On Fri, Apr 27, 2018 at 08:43:03AM -0600, Scott Bauer wrote:
>
>
> On 04/27/2018 06:41 AM, Dan Carpenter wrote:
> > I sent you an email to send this patch, but reviewing it now it's not
> > actually a run time bug. The cdrom_slot_status() function takes an
> > integer argument so it works.
>
> It's still runtime bug... I should reword the commit a bit to reflect that it's not
> like the upper 32 bit issue that you had found. Look at it this way, ints can be negative, right?
>
Oh. Yeah. Duh...
> The check is as follows:
>
> 2545: if (((int)arg >= cdi->capacity))
> return -EINVAL <https://elixir.bootlin.com/linux/v4.17-rc2/ident/EINVAL>;
> return cdrom_slot_status <https://elixir.bootlin.com/linux/v4.17-rc2/ident/cdrom_slot_status>(cdi, arg); so if (-65536 >= cdi->capacity) it's not so we don't return -einval. And we pass a negative index into cdrom_slot_status.
>
>
> where we do the following (https://elixir.bootlin.com/linux/v4.17-rc2/source/drivers/cdrom/cdrom.c#L13…):
>
> 1336:
> if (info->slots <https://elixir.bootlin.com/linux/v4.17-rc2/ident/slots>[slot <https://elixir.bootlin.com/linux/v4.17-rc2/ident/slot>].disc_present)
> ret = CDS_DISC_OK <https://elixir.bootlin.com/linux/v4.17-rc2/ident/CDS_DISC_OK>;
>
>
>
> >
> > I'm working on a static checker warning for these kinds of bugs:
> >
> > drivers/cdrom/cdrom.c:2444 cdrom_ioctl_select_disc() warn: truncated comparison 'arg' 'u64max' to 's32max'
> >
> > drivers/cdrom/cdrom.c
> > 2435 static int cdrom_ioctl_select_disc(struct cdrom_device_info *cdi,
> > 2436 unsigned long arg)
> > 2437 {
> > 2438 cd_dbg(CD_DO_IOCTL, "entering CDROM_SELECT_DISC\n");
> > 2439
> > 2440 if (!CDROM_CAN(CDC_SELECT_DISC))
> > 2441 return -ENOSYS;
> > 2442
> > 2443 if (arg != CDSL_CURRENT && arg != CDSL_NONE) {
> > 2444 if ((int)arg >= cdi->capacity)
> > ^^^^^^^^^^^^^^^^^^^^^^^^^
> > 2445 return -EINVAL;
> > 2446 }
> > 2447
> > 2448 /*
> > 2449 * ->select_disc is a hook to allow a driver-specific way of
> > 2450 * seleting disc. However, since there is no equivalent hook for
> > 2451 * cdrom_slot_status this may not actually be useful...
> > 2452 */
> > 2453 if (cdi->ops->select_disc)
> > 2454 return cdi->ops->select_disc(cdi, arg);
> > ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> > ->select_disc() also take an int so it's fine (plus there is no such
> > function so it's dead code).
> >
> > 2455
> > 2456 cd_dbg(CD_CHANGER, "Using generic cdrom_select_disc()\n");
> > 2457 return cdrom_select_disc(cdi, arg);
> > ^^^
> > Also an int.
> >
> > 2458 }
> >
> > So I think it's a good idea to fix these just for cleanliness and to
> > silence the static checker warnings but it doesn't affect runtime.
>
> Yeah, this one was "fine" aside from being messy, that's why I didn't send a patch for it.
>
I'm not convinced any more. Could you patch it and resend? We could
end up sending invalid commands to the cdrom firmware when we do
cdrom_load_unload() at the end of the cdrom_select_disc() function.
Proably there is no impact but we may as well fix it. Here is my
analysis if you are curious:
1371 /* If SLOT < 0, unload the current slot. Otherwise, try to load SLOT. */
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
CDSL_CURRENT is INT_MAX and CDSL_NONE is "INT_MAX - 1" but
cdrom_select_disc() calls this with slot set to -1.
1372 static int cdrom_load_unload(struct cdrom_device_info *cdi, int slot)
1373 {
1374 struct packet_command cgc;
1375
1376 cd_dbg(CD_CHANGER, "entering cdrom_load_unload()\n");
1377 if (cdi->sanyo_slot && slot < 0)
1378 return 0;
1379
1380 init_cdrom_command(&cgc, NULL, 0, CGC_DATA_NONE);
1381 cgc.cmd[0] = GPCMD_LOAD_UNLOAD;
1382 cgc.cmd[4] = 2 + (slot >= 0);
^^^^^^^^^^
So cmd[4] is 2.
1383 cgc.cmd[8] = slot;
^^^^^^^^^^^^^^^^^
Here were setting cmd[8] to any u8 value we choose.
1384 cgc.timeout = 60 * HZ;
1385
1386 /* The Sanyo 3 CD changer uses byte 7 of the
1387 GPCMD_TEST_UNIT_READY to command to switch CDs instead of
1388 using the GPCMD_LOAD_UNLOAD opcode. */
1389 if (cdi->sanyo_slot && -1 < slot) {
1390 cgc.cmd[0] = GPCMD_TEST_UNIT_READY;
1391 cgc.cmd[7] = slot;
1392 cgc.cmd[4] = cgc.cmd[8] = 0;
1393 cdi->sanyo_slot = slot ? slot : 3;
1394 }
1395
1396 return cdi->ops->generic_packet(cdi, &cgc);
1397 }
> P.S. Is your static analysis tooling available for the general public to look at?
Sure. I've been dorking with it for a couple days and I haven't tested
the latest version except on drivers/cdrom/cdrom.c so let me do some
more testing and then I'll post it.
regards,
dan carpenter
Hi,
This 2nd version of the series which fixes %p uses in kprobes.
Some by replacing with %pS, some by replacing with %px but
masking with kallsyms_show_value().
V1 series is here:
https://lkml.org/lkml/2018/1/25/1
I've read the thread about %pK and if I understand correctly
we shouldn't print kernel addresses. However, kprobes debugfs
interface can not stop to show the actual probe address because
it should be compared with addresses in kallsyms for debugging.
So, it depends on that kallsyms_show_value() allows to show
address to user, because if it returns true, anyway that user
can dump /proc/kallsyms.
Other error messages are replaced it with %pS, and one critical
function uses %px which is called right before BUG().
Also, I tried to fix this issue on each arch port. I searched
it by
# find arch/* | grep -e 'kprobe.*c' | xargs grep -w %p
And fixed all %p uses in those files.
Changes in this version;
- [1/7] is newly added.
- patches for MN10300(deleted) and s390(merged) are removed.
Thank you,
---
Masami Hiramatsu (7):
kprobes: Make blacklist root user read only
kprobes: Show blacklist addresses as same as kallsyms does
kprobes: Show address of kprobes if kallsyms does
kprobes: Replace %p with other pointer types
kprobes/x86: Fix %p uses in error messages
kprobes/arm: Fix %p uses in error messages
kprobes/arm64: Fix %p uses in error messages
arch/arm/probes/kprobes/core.c | 10 ++++----
arch/arm/probes/kprobes/test-core.c | 1 -
arch/arm64/kernel/probes/kprobes.c | 4 ++-
arch/x86/kernel/kprobes/core.c | 12 +++------
kernel/kprobes.c | 46 ++++++++++++++++++++++-------------
5 files changed, 40 insertions(+), 33 deletions(-)
--
Masami Hiramatsu (Linaro) <mhiramat(a)kernel.org>
The patch below does not apply to the 4.16-stable tree.
If someone wants it applied there, or to any other stable or longterm
tree, then please email the backport, including the original git commit
id to <stable(a)vger.kernel.org>.
thanks,
greg k-h
------------------ original commit in Linus's tree ------------------
>From 6cf09958f32b9667bb3ebadf74367c791112771b Mon Sep 17 00:00:00 2001
From: Martin Schwidefsky <schwidefsky(a)de.ibm.com>
Date: Fri, 20 Apr 2018 12:48:52 +0200
Subject: [PATCH] s390: correct module section names for expoline code revert
The main linker script vmlinux.lds.S for the kernel image merges
the expoline code patch tables into two section ".nospec_call_table"
and ".nospec_return_table". This is *not* done for the modules,
there the sections retain their original names as generated by gcc:
".s390_indirect_call", ".s390_return_mem" and ".s390_return_reg".
The module_finalize code has to check for the compiler generated
section names, otherwise no code patching is done. This slows down
the module code in case of "spectre_v2=off".
Cc: stable(a)vger.kernel.org # 4.16
Fixes: f19fbd5ed6 ("s390: introduce execute-trampolines for branches")
Signed-off-by: Martin Schwidefsky <schwidefsky(a)de.ibm.com>
diff --git a/arch/s390/kernel/module.c b/arch/s390/kernel/module.c
index 5a83be955c70..0dc8ac8548ee 100644
--- a/arch/s390/kernel/module.c
+++ b/arch/s390/kernel/module.c
@@ -465,11 +465,11 @@ int module_finalize(const Elf_Ehdr *hdr,
apply_alternatives(aseg, aseg + s->sh_size);
if (IS_ENABLED(CONFIG_EXPOLINE) &&
- (!strcmp(".nospec_call_table", secname)))
+ (!strncmp(".s390_indirect", secname, 14)))
nospec_revert(aseg, aseg + s->sh_size);
if (IS_ENABLED(CONFIG_EXPOLINE) &&
- (!strcmp(".nospec_return_table", secname)))
+ (!strncmp(".s390_return", secname, 12)))
nospec_revert(aseg, aseg + s->sh_size);
}
The patch
ASoC: mediatek: preallocate pages use platform device
has been applied to the asoc tree at
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git
All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.
You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.
If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.
Please add any relevant lists and maintainers to the CCs when replying
to this mail.
Thanks,
Mark
>From 5845e6155d8f4a4a9bae2d4c1d1bb4a4d9a925c2 Mon Sep 17 00:00:00 2001
From: Kai Chieh Chuang <kaichieh.chuang(a)mediatek.com>
Date: Fri, 27 Apr 2018 10:11:35 +0800
Subject: [PATCH] ASoC: mediatek: preallocate pages use platform device
preallocate pages should use platform device,
since we set dma mask for platform device.
Signed-off-by: KaiChieh Chuang <kaichieh.chuang(a)mediatek.com>
Signed-off-by: Mark Brown <broonie(a)kernel.org>
Cc: stable(a)vger.kernel.org
---
sound/soc/mediatek/common/mtk-afe-platform-driver.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/sound/soc/mediatek/common/mtk-afe-platform-driver.c b/sound/soc/mediatek/common/mtk-afe-platform-driver.c
index 53215b52e4f2..f8a06709f76d 100644
--- a/sound/soc/mediatek/common/mtk-afe-platform-driver.c
+++ b/sound/soc/mediatek/common/mtk-afe-platform-driver.c
@@ -64,14 +64,14 @@ static const struct snd_pcm_ops mtk_afe_pcm_ops = {
static int mtk_afe_pcm_new(struct snd_soc_pcm_runtime *rtd)
{
size_t size;
- struct snd_card *card = rtd->card->snd_card;
struct snd_pcm *pcm = rtd->pcm;
struct snd_soc_component *component = snd_soc_rtdcom_lookup(rtd, AFE_PCM_NAME);
struct mtk_base_afe *afe = snd_soc_component_get_drvdata(component);
size = afe->mtk_afe_hardware->buffer_bytes_max;
return snd_pcm_lib_preallocate_pages_for_all(pcm, SNDRV_DMA_TYPE_DEV,
- card->dev, size, size);
+ rtd->platform->dev,
+ size, size);
}
static void mtk_afe_pcm_free(struct snd_pcm *pcm)
--
2.17.0
On Wed, Feb 28, 2018 at 1:12 PM, Olof's autobuilder <build(a)lixom.net> wrote:
> Here are the build results from automated periodic testing.
> Warnings:
>
> arm64.allmodconfig:
> WARNING: modpost: missing MODULE_LICENSE() in drivers/phy/qualcomm/phy-qcom-ufs.o
Hi Greg,
It seems we're still missing one backport for a clean allmodconfig build:
59fba0869aca phy: qcom-ufs: add MODULE_LICENSE tag
Arnd
Dear Greg,
a fix for a voltage instability on our rk3399-puma board went into 4.15
(commit 87eba0716011, quoted below). As we have users who prefer to stay
on longterm 4.14, would you consider cherry-picking the commit into
4.14.y? It applies cleanly and has no effect outside of our rk3399-puma
board, where it fixes the instability.
Thank you and best regards,
Jakob
> commit 87eba0716011e528f7841026f2cc65683219d0ad
> Author: Klaus Goger <klaus.goger(a)theobroma-systems.com>
> Date: Tue Dec 5 08:11:58 2017 +0100
>
> arm64: dts: rockchip: remove vdd_log from rk3399-puma
>
> vdd_log has no consumer and therefore will not be set to a specific
> voltage. Still the PWM output pin gets configured and thence the vdd_log
> output voltage will changed from it's default. Depending on the idle
> state of the PWM this will slightly over or undervoltage the logic supply
> of the RK3399 and cause instability with GbE (undervoltage) and PCIe
> (overvoltage). Since the default value set by a voltage divider is the
> correct supply voltage and we don't need to change it during runtime we
> remove the rail from the devicetree completely so the PWM pin will not
> be configured.
>
> Signed-off-by: Klaus Goger <klaus.goger(a)theobroma-systems.com>
> Signed-off-by: Heiko Stuebner <heiko(a)sntech.de>
>
> arch/arm64/boot/dts/rockchip/rk3399-puma.dtsi | 11 -----------
> 1 file changed, 11 deletions(-)