August 2024 - Linux-stable-mirror

[tip: x86/urgent] x86/tdx: Fix data leak in mmio_read()

by tip-bot2 for Kirill A. Shutemov

The following commit has been merged into the x86/urgent branch of tip: Commit-ID: b6fb565a2d15277896583d471b21bc14a0c99661 Gitweb: https://git.kernel.org/tip/b6fb565a2d15277896583d471b21bc14a0c99661 Author: Kirill A. Shutemov <kirill.shutemov(a)linux.intel.com> AuthorDate: Mon, 26 Aug 2024 15:53:04 +03:00 Committer: Dave Hansen <dave.hansen(a)linux.intel.com> CommitterDate: Mon, 26 Aug 2024 12:45:19 -07:00 x86/tdx: Fix data leak in mmio_read() The mmio_read() function makes a TDVMCALL to retrieve MMIO data for an address from the VMM. Sean noticed that mmio_read() unintentionally exposes the value of an initialized variable (val) on the stack to the VMM. This variable is only needed as an output value. It did not need to be passed to the VMM in the first place. Do not send the original value of *val to the VMM. [ dhansen: clarify what 'val' is used for. ] Fixes: 31d58c4e557d ("x86/tdx: Handle in-kernel MMIO") Reported-by: Sean Christopherson <seanjc(a)google.com> Signed-off-by: Kirill A. Shutemov <kirill.shutemov(a)linux.intel.com> Signed-off-by: Dave Hansen <dave.hansen(a)linux.intel.com> Cc:stable@vger.kernel.org Link: https://lore.kernel.org/all/20240826125304.1566719-1-kirill.shutemov%40linu… --- arch/x86/coco/tdx/tdx.c | 1 - 1 file changed, 1 deletion(-) diff --git a/arch/x86/coco/tdx/tdx.c b/arch/x86/coco/tdx/tdx.c index 078e2ba..da8b66d 100644 --- a/arch/x86/coco/tdx/tdx.c +++ b/arch/x86/coco/tdx/tdx.c @@ -389,7 +389,6 @@ static bool mmio_read(int size, unsigned long addr, unsigned long *val) .r12 = size, .r13 = EPT_READ, .r14 = addr, - .r15 = *val, }; if (__tdx_hypercall(&args))

8 months

1
0
0 0

[PATCH] x86/tdx: Fix data leak in mmio_read()

by Kirill A. Shutemov

The mmio_read() function makes a TDVMCALL to retrieve MMIO data for an address from the VMM. Sean noticed that mmio_read() unintentionally exposes the value of an initialized variable on the stack to the VMM. Do not send the original value of *val to the VMM. Signed-off-by: Kirill A. Shutemov <kirill.shutemov(a)linux.intel.com> Reported-by: Sean Christopherson <seanjc(a)google.com> Fixes: 31d58c4e557d ("x86/tdx: Handle in-kernel MMIO") Cc: stable(a)vger.kernel.org # v5.19+ --- arch/x86/coco/tdx/tdx.c | 1 - 1 file changed, 1 deletion(-) diff --git a/arch/x86/coco/tdx/tdx.c b/arch/x86/coco/tdx/tdx.c index 078e2bac2553..da8b66dce0da 100644 --- a/arch/x86/coco/tdx/tdx.c +++ b/arch/x86/coco/tdx/tdx.c @@ -389,7 +389,6 @@ static bool mmio_read(int size, unsigned long addr, unsigned long *val) .r12 = size, .r13 = EPT_READ, .r14 = addr, - .r15 = *val, }; if (__tdx_hypercall(&args)) -- 2.43.0

8 months

2
2
0 0

[PATCH] codetag: debug: mark codetags for pages which transitioned from being poison to unpoison as empty

by Hao Ge

From: Hao Ge <gehao(a)kylinos.cn> The PG_hwpoison page will be caught and isolated on the entrance to the free buddy page pool. so,when we clear this flag and return it to the buddy system,mark codetags for pages as empty. It was detected by [1] and the following WARN occurred: [ 113.930443][ T3282] ------------[ cut here ]------------ [ 113.931105][ T3282] alloc_tag was not set [ 113.931576][ T3282] WARNING: CPU: 2 PID: 3282 at ./include/linux/alloc_tag.h:130 pgalloc_tag_sub.part.66+0x154/0x164 [ 113.932866][ T3282] Modules linked in: hwpoison_inject fuse ip6t_rpfilter ip6t_REJECT nf_reject_ipv6 ipt_REJECT nf_reject_ipv4 xt_conntrack ebtable_nat ebtable_broute ip6table_nat ip6table_man4 [ 113.941638][ T3282] CPU: 2 UID: 0 PID: 3282 Comm: madvise11 Kdump: loaded Tainted: G W 6.11.0-rc4-dirty #18 [ 113.943003][ T3282] Tainted: [W]=WARN [ 113.943453][ T3282] Hardware name: QEMU KVM Virtual Machine, BIOS unknown 2/2/2022 [ 113.944378][ T3282] pstate: 40400005 (nZcv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--) [ 113.945319][ T3282] pc : pgalloc_tag_sub.part.66+0x154/0x164 [ 113.946016][ T3282] lr : pgalloc_tag_sub.part.66+0x154/0x164 [ 113.946706][ T3282] sp : ffff800087093a10 [ 113.947197][ T3282] x29: ffff800087093a10 x28: ffff0000d7a9d400 x27: ffff80008249f0a0 [ 113.948165][ T3282] x26: 0000000000000000 x25: ffff80008249f2b0 x24: 0000000000000000 [ 113.949134][ T3282] x23: 0000000000000001 x22: 0000000000000001 x21: 0000000000000000 [ 113.950597][ T3282] x20: ffff0000c08fcad8 x19: ffff80008251e000 x18: ffffffffffffffff [ 113.952207][ T3282] x17: 0000000000000000 x16: 0000000000000000 x15: ffff800081746210 [ 113.953161][ T3282] x14: 0000000000000000 x13: 205d323832335420 x12: 5b5d353031313339 [ 113.954120][ T3282] x11: ffff800087093500 x10: 000000000000005d x9 : 00000000ffffffd0 [ 113.955078][ T3282] x8 : 7f7f7f7f7f7f7f7f x7 : ffff80008236ba90 x6 : c0000000ffff7fff [ 113.956036][ T3282] x5 : ffff000b34bf4dc8 x4 : ffff8000820aba90 x3 : 0000000000000001 [ 113.956994][ T3282] x2 : ffff800ab320f000 x1 : 841d1e35ac932e00 x0 : 0000000000000000 [ 113.957962][ T3282] Call trace: [ 113.958350][ T3282] pgalloc_tag_sub.part.66+0x154/0x164 [ 113.959000][ T3282] pgalloc_tag_sub+0x14/0x1c [ 113.959539][ T3282] free_unref_page+0xf4/0x4b8 [ 113.960096][ T3282] __folio_put+0xd4/0x120 [ 113.960614][ T3282] folio_put+0x24/0x50 [ 113.961103][ T3282] unpoison_memory+0x4f0/0x5b0 [ 113.961678][ T3282] hwpoison_unpoison+0x30/0x48 [hwpoison_inject] [ 113.962436][ T3282] simple_attr_write_xsigned.isra.34+0xec/0x1cc [ 113.963183][ T3282] simple_attr_write+0x38/0x48 [ 113.963750][ T3282] debugfs_attr_write+0x54/0x80 [ 113.964330][ T3282] full_proxy_write+0x68/0x98 [ 113.964880][ T3282] vfs_write+0xdc/0x4d0 [ 113.965372][ T3282] ksys_write+0x78/0x100 [ 113.965875][ T3282] __arm64_sys_write+0x24/0x30 [ 113.966440][ T3282] invoke_syscall+0x7c/0x104 [ 113.966984][ T3282] el0_svc_common.constprop.1+0x88/0x104 [ 113.967652][ T3282] do_el0_svc+0x2c/0x38 [ 113.968893][ T3282] el0_svc+0x3c/0x1b8 [ 113.969379][ T3282] el0t_64_sync_handler+0x98/0xbc [ 113.969980][ T3282] el0t_64_sync+0x19c/0x1a0 [ 113.970511][ T3282] ---[ end trace 0000000000000000 ]--- Link [1]: https://github.com/linux-test-project/ltp/blob/master/testcases/kernel/sysc… Fixes: a8fc28dad6d5 ("alloc_tag: introduce clear_page_tag_ref() helper function") Cc: stable(a)vger.kernel.org # v6.10 Signed-off-by: Hao Ge <gehao(a)kylinos.cn> --- mm/memory-failure.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/mm/memory-failure.c b/mm/memory-failure.c index 7066fc84f351..570388c41532 100644 --- a/mm/memory-failure.c +++ b/mm/memory-failure.c @@ -2623,6 +2623,12 @@ int unpoison_memory(unsigned long pfn) folio_put(folio); if (TestClearPageHWPoison(p)) { + /* the PG_hwpoison page will be caught and isolated + * on the entrance to the free buddy page pool. + * so,when we clear this flag and return it to the buddy system, + * clear it's codetag + */ + clear_page_tag_ref(p); folio_put(folio); ret = 0; } -- 2.25.1

8 months

3
14
0 0

[PATCH] Fixes: 496d0a648509 ("cpuidle: Fix guest_halt_poll_ns failed to take effect when setting guest_halt_poll_allow_shrink=N")

by Yanhao Dong

From: ysay <ysaydong(a)gmail.com> When guest_halt_poll_allow_shrink=N,setting guest_halt_poll_ns from a large value to 0 does not reset the CPU polling time, despite guest_halt_poll_ns being intended as a mandatory maximum time limit. The problem was situated in the adjust_poll_limit() within drivers/cpuidle/governors/haltpoll.c:79. Specifically, when guest_halt_poll_allow_shrink was set to N, resetting guest_halt_poll_ns to zero did not lead to executing any section of code that adjusts dev->poll_limit_ns. The issue has been resolved by relocating the check and assignment for dev->poll_limit_ns outside of the conditional block. This ensures that every modification to guest_halt_poll_ns properly influences the CPU polling time. Signed-off-by: ysay <ysaydong(a)gmail.com> --- drivers/cpuidle/governors/haltpoll.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/drivers/cpuidle/governors/haltpoll.c b/drivers/cpuidle/governors/haltpoll.c index 663b7f164..99c6260d7 100644 --- a/drivers/cpuidle/governors/haltpoll.c +++ b/drivers/cpuidle/governors/haltpoll.c @@ -78,26 +78,22 @@ static int haltpoll_select(struct cpuidle_driver *drv, static void adjust_poll_limit(struct cpuidle_device *dev, u64 block_ns) { - unsigned int val; + unsigned int val = dev->poll_limit_ns; /* Grow cpu_halt_poll_us if * cpu_halt_poll_us < block_ns < guest_halt_poll_us */ if (block_ns > dev->poll_limit_ns && block_ns <= guest_halt_poll_ns) { - val = dev->poll_limit_ns * guest_halt_poll_grow; + val *= guest_halt_poll_grow; if (val < guest_halt_poll_grow_start) val = guest_halt_poll_grow_start; - if (val > guest_halt_poll_ns) - val = guest_halt_poll_ns; trace_guest_halt_poll_ns_grow(val, dev->poll_limit_ns); - dev->poll_limit_ns = val; } else if (block_ns > guest_halt_poll_ns && guest_halt_poll_allow_shrink) { unsigned int shrink = guest_halt_poll_shrink; - val = dev->poll_limit_ns; if (shrink == 0) { val = 0; } else { @@ -108,8 +104,12 @@ static void adjust_poll_limit(struct cpuidle_device *dev, u64 block_ns) } trace_guest_halt_poll_ns_shrink(val, dev->poll_limit_ns); - dev->poll_limit_ns = val; } + + if (val > guest_halt_poll_ns) + val = guest_halt_poll_ns; + + dev->poll_limit_ns = val; } /** -- 2.43.5

8 months

3
2
0 0

[tip: x86/urgent] x86/tdx: Fix data leak in mmio_read()

by tip-bot2 for Kirill A. Shutemov

The following commit has been merged into the x86/urgent branch of tip: Commit-ID: eb786ee1390b8fbba633c01a971709c6906fd8bf Gitweb: https://git.kernel.org/tip/eb786ee1390b8fbba633c01a971709c6906fd8bf Author: Kirill A. Shutemov <kirill.shutemov(a)linux.intel.com> AuthorDate: Mon, 26 Aug 2024 15:53:04 +03:00 Committer: Dave Hansen <dave.hansen(a)linux.intel.com> CommitterDate: Mon, 26 Aug 2024 07:04:09 -07:00 x86/tdx: Fix data leak in mmio_read() The mmio_read() function makes a TDVMCALL to retrieve MMIO data for an address from the VMM. Sean noticed that mmio_read() unintentionally exposes the value of an initialized variable on the stack to the VMM. Do not send the original value of *val to the VMM. Fixes: 31d58c4e557d ("x86/tdx: Handle in-kernel MMIO") Reported-by: Sean Christopherson <seanjc(a)google.com> Signed-off-by: Kirill A. Shutemov <kirill.shutemov(a)linux.intel.com> Signed-off-by: Dave Hansen <dave.hansen(a)linux.intel.com> Cc:stable@vger.kernel.org Link: https://lore.kernel.org/all/20240826125304.1566719-1-kirill.shutemov%40linu… --- arch/x86/coco/tdx/tdx.c | 1 - 1 file changed, 1 deletion(-) diff --git a/arch/x86/coco/tdx/tdx.c b/arch/x86/coco/tdx/tdx.c index 078e2ba..da8b66d 100644 --- a/arch/x86/coco/tdx/tdx.c +++ b/arch/x86/coco/tdx/tdx.c @@ -389,7 +389,6 @@ static bool mmio_read(int size, unsigned long addr, unsigned long *val) .r12 = size, .r13 = EPT_READ, .r14 = addr, - .r15 = *val, }; if (__tdx_hypercall(&args))

8 months

1
0
0 0

[proposal] binfmt_misc: pass binfmt_misc flags to the interpreter

by Thorsten Glaser

commit 2347961b11d4079deace3c81dceed460c08a8fc1 I would like to propose this commit from 5.12 for stable, or rather, ask whether it’s a candidate and leave the exact picking/backporting to the experts (if it has other commits as prerequisites and/or later fixups). This is because qemu-user needs it, and it arrived just too late for the current Debian LTS kernel (5.10), and qemu-user in Debian until yesterday had a workaround, but now doesn’t because it’s in the stable kernel (6.1), so qemu-user-static cannot be just used as-is on Debian LTS any more. I’d like it to be applied to 5.10 (obviously), but perhaps others would appreciate more broad coverage. Thanks in advance, //mirabilos -- „Cool, /usr/share/doc/mksh/examples/uhr.gz ist ja ein Grund, mksh auf jedem System zu installieren.“ -- XTaran auf der OpenRheinRuhr, ganz begeistert (EN: “[…]uhr.gz is a reason to install mksh on every system.”)

8 months

2
2
0 0

[PATCH] btrfs: fix the race between umount and btrfs-cleaner

by Julian Sun

There is a race condition generic_shutdown_super() and __btrfs_run_defrag_inode(). Consider the following scenario: umount thread: btrfs-cleaner thread: btrfs_run_delayed_iputs() ->run_delayed_iput_locked() ->iput(inode) // Here the inode (ie ino 261) will be cleared and freed btrfs_kill_super() ->generic_shutdown_super() btrfs_run_defrag_inodes() ->__btrfs_run_defrag_inode() ->btrfs_iget(ino) // The inode 261 was recreated with i_count=1 // and added to the sb list ->evict_inodes(sb) // After some work // inode 261 was added ->iput(inode) // to the dispose list ->iput_funal() ->evict(inode) ->evict(inode) Now, we have two threads simultaneously evicting the same inode, which led to a bug. The above behavior can be confirmed by the log I added for debugging and the log printed when BUG was triggered. Due to space limitations, I cannot paste the full diff and here is a brief describtion. First, within __btrfs_run_defrag_inode(), set inode->i_state |= (1<<19) just before calling iput(). Within the dispose_list(), check the flag, if the flag was set, then pr_info("bug! double evict! crash will happen! state is 0x%lx\n", inode->i_state); Here is the printed log when the BUG was triggered: [ 190.686726][ T2336] bug! double evict! crash will happen! state is 0x80020 [ 190.687647][ T2336] ------------[ cut here ]------------ [ 190.688294][ T2336] kernel BUG at fs/inode.c:626! [ 190.688939][ T2336] Oops: invalid opcode: 0000 [#1] PREEMPT SMP KASAN NOPTI [ 190.689792][ T2336] CPU: 1 PID: 2336 Comm: a.out Not tainted 6.10.0-rc2-00223-g0c529ab65ef8-dirty #109 [ 190.690894][ T2336] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.16.2-debian-1.16.2-1 04/01/2014 [ 190.692111][ T2336] RIP: 0010:clear_inode+0x15b/0x190 // some logs... [ 190.704501][ T2336] btrfs_evict_inode+0x529/0xe80 [ 190.706966][ T2336] evict+0x2ed/0x6c0 [ 190.707209][ T2336] dispose_list+0x62/0x260 [ 190.707490][ T2336] evict_inodes+0x34e/0x450 To prevent this behavior, we need to set BTRFS_FS_CLOSING_START before kill_anon_super() to ensure that btrfs_run_defrag_inodes() doesn't continue working after unmount. Reported-and-tested-by: syzbot+67ba3c42bcbb4665d3ad(a)syzkaller.appspotmail.com Closes: https://syzkaller.appspot.com/bug?extid=67ba3c42bcbb4665d3ad CC: stable(a)vger.kernel.org Fixes: c146afad2c7f ("Btrfs: mount ro and remount support") Signed-off-by: Julian Sun <sunjunchao2870(a)gmail.com> --- fs/btrfs/super.c | 1 + 1 file changed, 1 insertion(+) diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c index f05cce7c8b8d..f7e87fe583ab 100644 --- a/fs/btrfs/super.c +++ b/fs/btrfs/super.c @@ -2093,6 +2093,7 @@ static int btrfs_get_tree(struct fs_context *fc) static void btrfs_kill_super(struct super_block *sb) { struct btrfs_fs_info *fs_info = btrfs_sb(sb); + set_bit(BTRFS_FS_CLOSING_START, &fs_info->flags); kill_anon_super(sb); btrfs_free_fs_info(fs_info); } -- 2.39.2

8 months

3
5
0 0

FAILED: patch "[PATCH] thermal: of: Fix OF node leak in of_thermal_zone_find() error" failed to apply to 6.6-stable tree

by gregkh＠linuxfoundation.org

The patch below does not apply to the 6.6-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>. To reproduce the conflict and resubmit, you may use the following commands: git fetch https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/ linux-6.6.y git checkout FETCH_HEAD git cherry-pick -x c0a1ef9c5be72ff28a5413deb1b3e1a066593c13 # <resolve conflicts, build, test, etc.> git commit -s git send-email --to '<stable(a)vger.kernel.org>' --in-reply-to '2024082600-trodden-majesty-7be0@gregkh' --subject-prefix 'PATCH 6.6.y' HEAD^.. Possible dependencies: c0a1ef9c5be7 ("thermal: of: Fix OF node leak in of_thermal_zone_find() error paths") thanks, greg k-h ------------------ original commit in Linus's tree ------------------ From c0a1ef9c5be72ff28a5413deb1b3e1a066593c13 Mon Sep 17 00:00:00 2001 From: Krzysztof Kozlowski <krzysztof.kozlowski(a)linaro.org> Date: Wed, 14 Aug 2024 21:58:23 +0200 Subject: [PATCH] thermal: of: Fix OF node leak in of_thermal_zone_find() error paths Terminating for_each_available_child_of_node() loop requires dropping OF node reference, so bailing out on errors misses this. Solve the OF node reference leak with scoped for_each_available_child_of_node_scoped(). Fixes: 3fd6d6e2b4e8 ("thermal/of: Rework the thermal device tree initialization") Cc: <stable(a)vger.kernel.org> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski(a)linaro.org> Reviewed-by: Chen-Yu Tsai <wenst(a)chromium.org> Reviewed-by: Daniel Lezcano <daniel.lezcano(a)linaro.org> Link: https://patch.msgid.link/20240814195823.437597-3-krzysztof.kozlowski@linaro… Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki(a)intel.com> diff --git a/drivers/thermal/thermal_of.c b/drivers/thermal/thermal_of.c index b08a9b64718d..1f252692815a 100644 --- a/drivers/thermal/thermal_of.c +++ b/drivers/thermal/thermal_of.c @@ -184,14 +184,14 @@ static struct device_node *of_thermal_zone_find(struct device_node *sensor, int * Search for each thermal zone, a defined sensor * corresponding to the one passed as parameter */ - for_each_available_child_of_node(np, tz) { + for_each_available_child_of_node_scoped(np, child) { int count, i; - count = of_count_phandle_with_args(tz, "thermal-sensors", + count = of_count_phandle_with_args(child, "thermal-sensors", "#thermal-sensor-cells"); if (count <= 0) { - pr_err("%pOFn: missing thermal sensor\n", tz); + pr_err("%pOFn: missing thermal sensor\n", child); tz = ERR_PTR(-EINVAL); goto out; } @@ -200,18 +200,19 @@ static struct device_node *of_thermal_zone_find(struct device_node *sensor, int int ret; - ret = of_parse_phandle_with_args(tz, "thermal-sensors", + ret = of_parse_phandle_with_args(child, "thermal-sensors", "#thermal-sensor-cells", i, &sensor_specs); if (ret < 0) { - pr_err("%pOFn: Failed to read thermal-sensors cells: %d\n", tz, ret); + pr_err("%pOFn: Failed to read thermal-sensors cells: %d\n", child, ret); tz = ERR_PTR(ret); goto out; } if ((sensor == sensor_specs.np) && id == (sensor_specs.args_count ? sensor_specs.args[0] : 0)) { - pr_debug("sensor %pOFn id=%d belongs to %pOFn\n", sensor, id, tz); + pr_debug("sensor %pOFn id=%d belongs to %pOFn\n", sensor, id, child); + tz = no_free_ptr(child); goto out; } }

8 months

1
0
0 0

FAILED: patch "[PATCH] thermal: of: Fix OF node leak in of_thermal_zone_find() error" failed to apply to 6.1-stable tree

by gregkh＠linuxfoundation.org

The patch below does not apply to the 6.1-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>. To reproduce the conflict and resubmit, you may use the following commands: git fetch https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/ linux-6.1.y git checkout FETCH_HEAD git cherry-pick -x c0a1ef9c5be72ff28a5413deb1b3e1a066593c13 # <resolve conflicts, build, test, etc.> git commit -s git send-email --to '<stable(a)vger.kernel.org>' --in-reply-to '2024082659-veto-ladies-d1b3@gregkh' --subject-prefix 'PATCH 6.1.y' HEAD^.. Possible dependencies: c0a1ef9c5be7 ("thermal: of: Fix OF node leak in of_thermal_zone_find() error paths") thanks, greg k-h ------------------ original commit in Linus's tree ------------------ From c0a1ef9c5be72ff28a5413deb1b3e1a066593c13 Mon Sep 17 00:00:00 2001 From: Krzysztof Kozlowski <krzysztof.kozlowski(a)linaro.org> Date: Wed, 14 Aug 2024 21:58:23 +0200 Subject: [PATCH] thermal: of: Fix OF node leak in of_thermal_zone_find() error paths Terminating for_each_available_child_of_node() loop requires dropping OF node reference, so bailing out on errors misses this. Solve the OF node reference leak with scoped for_each_available_child_of_node_scoped(). Fixes: 3fd6d6e2b4e8 ("thermal/of: Rework the thermal device tree initialization") Cc: <stable(a)vger.kernel.org> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski(a)linaro.org> Reviewed-by: Chen-Yu Tsai <wenst(a)chromium.org> Reviewed-by: Daniel Lezcano <daniel.lezcano(a)linaro.org> Link: https://patch.msgid.link/20240814195823.437597-3-krzysztof.kozlowski@linaro… Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki(a)intel.com> diff --git a/drivers/thermal/thermal_of.c b/drivers/thermal/thermal_of.c index b08a9b64718d..1f252692815a 100644 --- a/drivers/thermal/thermal_of.c +++ b/drivers/thermal/thermal_of.c @@ -184,14 +184,14 @@ static struct device_node *of_thermal_zone_find(struct device_node *sensor, int * Search for each thermal zone, a defined sensor * corresponding to the one passed as parameter */ - for_each_available_child_of_node(np, tz) { + for_each_available_child_of_node_scoped(np, child) { int count, i; - count = of_count_phandle_with_args(tz, "thermal-sensors", + count = of_count_phandle_with_args(child, "thermal-sensors", "#thermal-sensor-cells"); if (count <= 0) { - pr_err("%pOFn: missing thermal sensor\n", tz); + pr_err("%pOFn: missing thermal sensor\n", child); tz = ERR_PTR(-EINVAL); goto out; } @@ -200,18 +200,19 @@ static struct device_node *of_thermal_zone_find(struct device_node *sensor, int int ret; - ret = of_parse_phandle_with_args(tz, "thermal-sensors", + ret = of_parse_phandle_with_args(child, "thermal-sensors", "#thermal-sensor-cells", i, &sensor_specs); if (ret < 0) { - pr_err("%pOFn: Failed to read thermal-sensors cells: %d\n", tz, ret); + pr_err("%pOFn: Failed to read thermal-sensors cells: %d\n", child, ret); tz = ERR_PTR(ret); goto out; } if ((sensor == sensor_specs.np) && id == (sensor_specs.args_count ? sensor_specs.args[0] : 0)) { - pr_debug("sensor %pOFn id=%d belongs to %pOFn\n", sensor, id, tz); + pr_debug("sensor %pOFn id=%d belongs to %pOFn\n", sensor, id, child); + tz = no_free_ptr(child); goto out; } }

8 months

1
0
0 0

[PATCH v2] firmware_loader: Block path traversal

by Jann Horn

Most firmware names are hardcoded strings, or are constructed from fairly constrained format strings where the dynamic parts are just some hex numbers or such. However, there are a couple codepaths in the kernel where firmware file names contain string components that are passed through from a device or semi-privileged userspace; the ones I could find (not counting interfaces that require root privileges) are: - lpfc_sli4_request_firmware_update() seems to construct the firmware filename from "ModelName", a string that was previously parsed out of some descriptor ("Vital Product Data") in lpfc_fill_vpd() - nfp_net_fw_find() seems to construct a firmware filename from a model name coming from nfp_hwinfo_lookup(pf->hwinfo, "nffw.partno"), which I think parses some descriptor that was read from the device. (But this case likely isn't exploitable because the format string looks like "netronome/nic_%s", and there shouldn't be any *folders* starting with "netronome/nic_". The previous case was different because there, the "%s" is *at the start* of the format string.) - module_flash_fw_schedule() is reachable from the ETHTOOL_MSG_MODULE_FW_FLASH_ACT netlink command, which is marked as GENL_UNS_ADMIN_PERM (meaning CAP_NET_ADMIN inside a user namespace is enough to pass the privilege check), and takes a userspace-provided firmware name. (But I think to reach this case, you need to have CAP_NET_ADMIN over a network namespace that a special kind of ethernet device is mapped into, so I think this is not a viable attack path in practice.) Fix it by rejecting any firmware names containing ".." path components. For what it's worth, I went looking and haven't found any USB device drivers that use the firmware loader dangerously. Cc: stable(a)vger.kernel.org Fixes: abb139e75c2c ("firmware: teach the kernel to load firmware files directly from the filesystem") Signed-off-by: Jann Horn <jannh(a)google.com> --- Changes in v2: - describe fix in commit message (dakr) - write check more clearly and with comment in separate helper (dakr) - document new restriction in comment above request_firmware() (dakr) - warn when new restriction is triggered - Link to v1: https://lore.kernel.org/r/20240820-firmware-traversal-v1-1-8699ffaa9276@goo… --- drivers/base/firmware_loader/main.c | 41 +++++++++++++++++++++++++++++++++++++ 1 file changed, 41 insertions(+) diff --git a/drivers/base/firmware_loader/main.c b/drivers/base/firmware_loader/main.c index a03ee4b11134..dd47ce9a761f 100644 --- a/drivers/base/firmware_loader/main.c +++ b/drivers/base/firmware_loader/main.c @@ -849,6 +849,37 @@ static void fw_log_firmware_info(const struct firmware *fw, const char *name, {} #endif +/* + * Reject firmware file names with ".." path components. + * There are drivers that construct firmware file names from device-supplied + * strings, and we don't want some device to be able to tell us "I would like to + * be sent my firmware from ../../../etc/shadow, please". + * + * Search for ".." surrounded by either '/' or start/end of string. + * + * This intentionally only looks at the firmware name, not at the firmware base + * directory or at symlink contents. + */ +static bool name_contains_dotdot(const char *name) +{ + size_t name_len = strlen(name); + size_t i; + + if (name_len < 2) + return false; + for (i = 0; i < name_len - 1; i++) { + /* do we see a ".." sequence? */ + if (name[i] != '.' || name[i+1] != '.') + continue; + + /* is it a path component? */ + if ((i == 0 || name[i-1] == '/') && + (i == name_len - 2 || name[i+2] == '/')) + return true; + } + return false; +} + /* called from request_firmware() and request_firmware_work_func() */ static int _request_firmware(const struct firmware **firmware_p, const char *name, @@ -869,6 +900,14 @@ _request_firmware(const struct firmware **firmware_p, const char *name, goto out; } + if (name_contains_dotdot(name)) { + dev_warn(device, + "Firmware load for '%s' refused, path contains '..' component", + name); + ret = -EINVAL; + goto out; + } + ret = _request_firmware_prepare(&fw, name, device, buf, size, offset, opt_flags); if (ret <= 0) /* error or already assigned */ @@ -946,6 +985,8 @@ _request_firmware(const struct firmware **firmware_p, const char *name, * @name will be used as $FIRMWARE in the uevent environment and * should be distinctive enough not to be confused with any other * firmware image for this or any other device. + * It must not contain any ".." path components - "foo/bar..bin" is + * allowed, but "foo/../bar.bin" is not. * * Caller must hold the reference count of @device. * --- base-commit: b0da640826ba3b6506b4996a6b23a429235e6923 change-id: 20240820-firmware-traversal-6df8501b0fe4 -- Jann Horn <jannh(a)google.com>

8 months

6
11
0 0

2025

2024

2023

2022

2021

2020

2019

2018

2017

Linux-stable-mirror August 2024