kernel/resource.c | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-)
From: King Dix <kingdix10@qq.com>
When a stack string variable is passed during the request resource
operation, it causes an oops problem when executing cat /proc/iomem.
In the original code, in functions like __request_region_locked, the name
member of the resource structure was directly assigned the stack string
pointer without proper memory management.
This fix changes the assignment of res->name to use kstrdup_const for
string copying, ensuring the correct storage and release of the string
and thus avoiding potential memory errors and oops issues.
Signed-off-by: King Dix <kingdix10@qq.com>
---
kernel/resource.c | 14 ++++++++------
1 file changed, 8 insertions(+), 6 deletions(-)
diff --git a/kernel/resource.c b/kernel/resource.c
index b7c0e24d9398..87d22741c066 100644
--- a/kernel/resource.c
+++ b/kernel/resource.c
@@ -168,8 +168,10 @@ static void free_resource(struct resource *res)
* buddy and trying to be smart and reusing them eventually in
* alloc_resource() overcomplicates resource handling.
*/
- if (res && PageSlab(virt_to_head_page(res)))
+ if (res && PageSlab(virt_to_head_page(res))) {
+ kfree_const(res->name);
kfree(res);
+ }
}
static struct resource *alloc_resource(gfp_t flags)
@@ -1098,7 +1100,7 @@ __reserve_region_with_split(struct resource *root, resource_size_t start,
if (!res)
return;
- res->name = name;
+ res->name = kstrdup_const(name, GFP_ATOMIC);
res->start = start;
res->end = end;
res->flags = type | IORESOURCE_BUSY;
@@ -1133,7 +1135,7 @@ __reserve_region_with_split(struct resource *root, resource_size_t start,
free_resource(res);
break;
}
- next_res->name = name;
+ next_res->name = kstrdup_const(name, GFP_ATOMIC);
next_res->start = conflict->end + 1;
next_res->end = end;
next_res->flags = type | IORESOURCE_BUSY;
@@ -1261,7 +1263,7 @@ static int __request_region_locked(struct resource *res, struct resource *parent
{
DECLARE_WAITQUEUE(wait, current);
- res->name = name;
+ res->name = kstrdup_const(name, GFP_KERNEL);
res->start = start;
res->end = start + n - 1;
@@ -1474,7 +1476,7 @@ void release_mem_region_adjustable(resource_size_t start, resource_size_t size)
goto retry;
}
}
- new_res->name = res->name;
+ new_res->name = kstrdup_const(res->name, GFP_ATOMIC);
new_res->start = end + 1;
new_res->end = res->end;
new_res->flags = res->flags;
@@ -1978,7 +1980,7 @@ get_free_mem_region(struct device *dev, struct resource *base,
} else {
res->start = addr;
res->end = addr + size - 1;
- res->name = name;
+ res->name = kstrdup_const(name, GFP_KERNEL);
res->desc = desc;
res->flags = IORESOURCE_MEM;
--
2.43.0
Hello,
kernel test robot noticed "BUG:sleeping_function_called_from_invalid_context_at_include/linux/sched/mm.h" on:
commit: 85f3f9e1bca4488dcf0032c11c2d0b59352971a0 ("[PATCH] resource: use kstrdup_const to prevent wild pointer issues")
url: https://github.com/intel-lab-lkp/linux/commits/kingdix10-qq-com/resource-use-kstrdup_const-to-prevent-wild-pointer-issues/20250101-215639
base: https://git.kernel.org/cgit/linux/kernel/git/akpm/mm.git mm-everything
patch link: https://lore.kernel.org/all/tencent_3BD5C192A0E62121DE44D1A05A9C9ECB7C06@qq.com/
patch subject: [PATCH] resource: use kstrdup_const to prevent wild pointer issues
in testcase: boot
config: x86_64-randconfig-161-20250102
compiler: gcc-12
test machine: qemu-system-x86_64 -enable-kvm -cpu SandyBridge -smp 2 -m 16G
(please refer to attached dmesg/kmsg for entire log/backtrace)
+-------------------------------------------------------------------------------+------------+------------+
| | d77847ea04 | 85f3f9e1bc |
+-------------------------------------------------------------------------------+------------+------------+
| BUG:sleeping_function_called_from_invalid_context_at_include/linux/sched/mm.h | 0 | 18 |
+-------------------------------------------------------------------------------+------------+------------+
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <oliver.sang@intel.com>
| Closes: https://lore.kernel.org/oe-lkp/202501031658.51df654c-lkp@intel.com
[ 24.398603][ T1] BUG: sleeping function called from invalid context at include/linux/sched/mm.h:321
[ 24.399830][ T1] in_atomic(): 1, irqs_disabled(): 0, non_block: 0, pid: 1, name: swapper/0
[ 24.400487][ T1] preempt_count: 1, expected: 0
[ 24.400919][ T1] RCU nest depth: 0, expected: 0
[ 24.401295][ T1] 2 locks held by swapper/0/1:
[ 24.401659][ T1] #0: ffff8881003600f0 (&dev->mutex){....}-{4:4}, at: __driver_attach (drivers/base/dd.c:1216 drivers/base/dd.c:1156)
[ 24.402502][ T1] #1: ffffffff95689c58 (resource_lock){++++}-{3:3}, at: __request_region (kernel/resource.c:1264 kernel/resource.c:1330)
[ 24.403255][ T1] CPU: 0 UID: 0 PID: 1 Comm: swapper/0 Tainted: G W T 6.13.0-rc4-00416-g85f3f9e1bca4 #2
[ 24.404161][ T1] Tainted: [W]=WARN, [T]=RANDSTRUCT
[ 24.404599][ T1] Call Trace:
[ 24.404862][ T1] <TASK>
[ 24.404862][ T1] dump_stack_lvl (lib/dump_stack.c:123)
[ 24.404862][ T1] dump_stack (lib/dump_stack.c:130)
[ 24.404862][ T1] __might_resched (kernel/sched/core.c:8767)
[ 24.404862][ T1] slab_alloc_node+0x12d/0x1b0
[ 24.404862][ T1] ? kstrdup_const (mm/util.c:101)
[ 24.404862][ T1] __kmalloc_node_track_caller_noprof (mm/slub.c:4293 mm/slub.c:4313)
[ 24.404862][ T1] kstrdup (mm/util.c:61 mm/util.c:81)
[ 24.404862][ T1] kstrdup_const (mm/util.c:101)
[ 24.404862][ T1] __request_region (kernel/resource.c:1266 kernel/resource.c:1330)
[ 24.404862][ T1] ? device_register (drivers/base/core.c:3748)
[ 24.404862][ T1] ? wake_up_q (kernel/sched/core.c:7088)
[ 24.404862][ T1] __parport_pc_probe_port (drivers/parport/parport_pc.c:2073)
[ 24.404862][ T1] ? __dev_printk (drivers/base/core.c:4962)
[ 24.404862][ T1] ? _dev_info (drivers/base/core.c:5004)
[ 24.404862][ T1] ? pnp_device_probe (drivers/pnp/driver.c:95)
[ 24.404862][ T1] parport_pc_pnp_probe (drivers/parport/parport_pc.c:2917)
[ 24.404862][ T1] parport_pc_pnp_probe (drivers/parport/parport_pc.c:3039)
[ 24.404862][ T1] pnp_device_probe (drivers/pnp/driver.c:111)
[ 24.404862][ T1] really_probe (drivers/base/dd.c:579 drivers/base/dd.c:658)
[ 24.404862][ T1] __driver_probe_device (drivers/base/dd.c:800)
[ 24.404862][ T1] driver_probe_device (drivers/base/dd.c:830)
[ 24.404862][ T1] __driver_attach (drivers/base/dd.c:1217 drivers/base/dd.c:1156)
[ 24.404862][ T1] ? __device_attach_driver (drivers/base/dd.c:1157)
[ 24.404862][ T1] bus_for_each_dev (drivers/base/bus.c:369)
[ 24.404862][ T1] driver_attach (drivers/base/dd.c:1235)
[ 24.404862][ T1] bus_add_driver (drivers/base/bus.c:675)
[ 24.404862][ T1] driver_register (drivers/base/driver.c:246)
[ 24.404862][ T1] pnp_register_driver (drivers/pnp/driver.c:281)
[ 24.404862][ T1] parport_pc_find_ports (drivers/parport/parport_pc.c:3118)
[ 24.404862][ T1] ? driver_register (drivers/base/driver.c:258)
[ 24.404862][ T1] parport_pc_init (drivers/parport/parport_pc.c:3380)
[ 24.404862][ T1] ? parport_pc_find_ports (drivers/parport/parport_pc.c:3354)
[ 24.404862][ T1] do_one_initcall (init/main.c:1266)
[ 24.404862][ T1] ? rcu_is_watching (arch/x86/include/asm/atomic.h:23 include/linux/atomic/atomic-arch-fallback.h:457 include/linux/context_tracking.h:128 kernel/rcu/tree.c:737)
[ 24.404862][ T1] do_initcalls (init/main.c:1327 init/main.c:1344)
[ 24.404862][ T1] ? rdinit_setup (init/main.c:1314)
[ 24.404862][ T1] ? rest_init (init/main.c:1458)
[ 24.404862][ T1] kernel_init_freeable (init/main.c:1581)
[ 24.404862][ T1] kernel_init (init/main.c:1468)
[ 24.404862][ T1] ret_from_fork (arch/x86/kernel/process.c:153)
[ 24.404862][ T1] ? rest_init (init/main.c:1458)
[ 24.404862][ T1] ret_from_fork_asm (arch/x86/entry/entry_64.S:254)
[ 24.404862][ T1] </TASK>
The kernel config and materials to reproduce are available at:
https://download.01.org/0day-ci/archive/20250103/202501031658.51df654c-lkp@intel.com
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
kingdix10@qq.com writes: > From: King Dix <kingdix10@qq.com> > > When a stack string variable is passed during the request resource > operation, it causes an oops problem when executing cat /proc/iomem. > > In the original code, in functions like __request_region_locked, the name > member of the resource structure was directly assigned the stack string > pointer without proper memory management. > > This fix changes the assignment of res->name to use kstrdup_const for > string copying, ensuring the correct storage and release of the string > and thus avoiding potential memory errors and oops issues. > > Signed-off-by: King Dix <kingdix10@qq.com> In general, I think that it's good to improve the resource requesting API. However, it's not good to use so many GFP_ATOMIC too. Why do you need to call resource requesting API with stack variable? If it's just some programming bugs, we should add more checks instead of hiding the bugs. For example, if we only allows kernel rodata and slab memory to be used in resource requesting. We can add a VM_WARN_ON() to check that. [snip] --- Best Regards, Huang, Ying
On Thu, 02 Jan 2025 09:59:26 +0800 "Huang, Ying" <ying.huang@linux.alibaba.com> wrote: > > From: King Dix <kingdix10@qq.com> > > > > When a stack string variable is passed during the request resource > > operation, it causes an oops problem when executing cat /proc/iomem. > > > > In the original code, in functions like __request_region_locked, the name > > member of the resource structure was directly assigned the stack string > > pointer without proper memory management. > > > > This fix changes the assignment of res->name to use kstrdup_const for > > string copying, ensuring the correct storage and release of the string > > and thus avoiding potential memory errors and oops issues. > > > > Signed-off-by: King Dix <kingdix10@qq.com> > > In general, I think that it's good to improve the resource requesting > API. However, it's not good to use so many GFP_ATOMIC too. Why do you > need to call resource requesting API with stack variable? If it's just > some programming bugs, we should add more checks instead of hiding the > bugs. For example, if we only allows kernel rodata and slab memory to be > used in resource requesting. We can add a VM_WARN_ON() to check that. I agree. It may not be a very good idea, but request_region() requires that the caller pass in a `name' string which is permanently available. __request_region() kerneldoc doesn't document this, and it should. Because of this present interface design, calling request_region() with an on-stack string must be considered a bug in the calling code.
© 2016 - 2026 Red Hat, Inc.