From: Peng Fan <[email protected]>
When executing the following command, we met kernel dump.
dmesg -c > /dev/null; cd /sys;
for i in `ls /sys/kernel/debug/regmap/* -d`; do
echo "Checking regmap in $i";
cat $i/registers;
done && grep -ri "0x02d0" *;
------------[ cut here ]------------
WARNING: CPU: 0 PID: 6406 at mm/page_alloc.c:4738 __alloc_pages_nodemask+0x2c8/0xda4
Modules linked in: mx6s_capture ov5640_camera_v2 galcore(O) [last unloaded: snvs_ui]
CPU: 0 PID: 6406 Comm: grep Tainted: G O 5.4.3-lts-lf-5.4.y+g54cbaf43e23e #1
Hardware name: Freescale i.MX6 SoloX (Device Tree)
[<80110770>] (unwind_backtrace) from [<8010b650>] (show_stack+0x10/0x14)
[<8010b650>] (show_stack) from [<80b3770c>] (dump_stack+0x90/0xa4)
[<80b3770c>] (dump_stack) from [<80130310>] (__warn+0xbc/0xd8)
[<80130310>] (__warn) from [<80130390>] (warn_slowpath_fmt+0x64/0xc4)
[<80130390>] (warn_slowpath_fmt) from [<80214328>] (__alloc_pages_nodemask+0x2c8/0xda4)
[<80214328>] (__alloc_pages_nodemask) from [<801f4ba8>] (kmalloc_order+0x1c/0x4c)
[<801f4ba8>] (kmalloc_order) from [<8061e360>] (regmap_read_debugfs+0x64/0x39c)
[<8061e360>] (regmap_read_debugfs) from [<8061e6f0>] (regmap_map_read_file+0x28/0x30)
[<8061e6f0>] (regmap_map_read_file) from [<803c0614>] (full_proxy_read+0x54/0x70)
[<803c0614>] (full_proxy_read) from [<8022c68c>] (__vfs_read+0x30/0x1c4)
[<8022c68c>] (__vfs_read) from [<8022c8b4>] (vfs_read+0x94/0x110)
[<8022c8b4>] (vfs_read) from [<8022cb98>] (ksys_read+0x64/0xec)
[<8022cb98>] (ksys_read) from [<80101000>] (ret_fast_syscall+0x0/0x54)
Exception stack(0xa868bfa8 to 0xa868bff0)
bfa0: 03001000 03000000 00000006 70e0e000 03000000 00000000
bfc0: 03001000 03000000 76fa53d0 00000003 00000006 70e0d008 00040298 70e0e000
bfe0: 00000003 7ead45a0 76ea2cd7 76e2ed16
---[ end trace f0a1fccbc20d20ff ]---
It is because the count value is too big, and kmalloc fails.
So add a check to allow only max->max_register as max count.
Signed-off-by: Peng Fan <[email protected]>
---
drivers/base/regmap/regmap-debugfs.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/base/regmap/regmap-debugfs.c b/drivers/base/regmap/regmap-debugfs.c
index e72843fe41df..a375cb2ebac9 100644
--- a/drivers/base/regmap/regmap-debugfs.c
+++ b/drivers/base/regmap/regmap-debugfs.c
@@ -283,9 +283,10 @@ static ssize_t regmap_map_read_file(struct file *file, char __user *user_buf,
size_t count, loff_t *ppos)
{
struct regmap *map = file->private_data;
+ size_t num = count > map->max_register ? map->max_register : count;
return regmap_read_debugfs(map, 0, map->max_register, user_buf,
- count, ppos);
+ num, ppos);
}
#undef REGMAP_ALLOW_WRITE_DEBUGFS
@@ -371,6 +372,8 @@ static ssize_t regmap_reg_ranges_read_file(struct file *file,
if (*ppos < 0 || !count)
return -EINVAL;
+ count = (count + *ppos) > map->max_register ? map->max_register : count;
+
buf = kmalloc(count, GFP_KERNEL);
if (!buf)
return -ENOMEM;
--
2.16.4
On Sun, Mar 08, 2020 at 09:10:58PM +0800, [email protected] wrote:
> ------------[ cut here ]------------
> WARNING: CPU: 0 PID: 6406 at mm/page_alloc.c:4738 __alloc_pages_nodemask+0x2c8/0xda4
> Modules linked in: mx6s_capture ov5640_camera_v2 galcore(O) [last unloaded: snvs_ui]
> CPU: 0 PID: 6406 Comm: grep Tainted: G O 5.4.3-lts-lf-5.4.y+g54cbaf43e23e #1
Please think hard before including complete backtraces in upstream
reports, they are very large and contain almost no useful information
relative to their size so often obscure the relevant content in your
message. If part of the backtrace is usefully illustrative (it often is
for search engines if nothing else) then it's usually better to pull out
the relevant sections.
On Sun, Mar 08, 2020 at 09:10:58PM +0800, [email protected] wrote:
> From: Peng Fan <[email protected]>
> @@ -283,9 +283,10 @@ static ssize_t regmap_map_read_file(struct file *file, char __user *user_buf,
> size_t count, loff_t *ppos)
> {
> struct regmap *map = file->private_data;
> + size_t num = count > map->max_register ? map->max_register : count;
I can see that it might be useful to limit the read size (though our
error checking is doing the right thing here, it's just that kmalloc()
is very verbose) but this doesn't seem like a good limit, especially for
smaller register maps. Since it's limiting reads to the number of
registers it's going to result in it being impossible to dump the full
register map in a single read. This is fine from a filesystem API point
of view, reads can always return less data than was asked for, but it's
annoying from the point of view of anyone hacking together something
like a little program to monitor a specific register during testing or
whatever. If the register map is small enough you won't even be able to
read a single register in a read which is going to be annoying. Having
either a lower bound or a more generous upper bound would be better.
Please also write normal conditional statements, the ternery operator
isn't great for legibility.