On Thu, Feb 02, 2017 at 01:32:10PM +0100, Stanislaw Gruszka wrote:
> If ->get_regs_len() callback return 0, we allocate 0 bytes of memory,
> what print ugly warning in dmesg, which can be found further below.
>
> This happen on mac80211 devices where ieee80211_get_regs_len() just
> return 0 and driver only fills ethtool_regs structure and actually
> do not provide any dump. However I assume this can happen on other
> drivers i.e. when for some devices driver provide regs dump and for
> others do not. Hence preventing to to print warning in ethtool code
> seems to be reasonable.
>
> ethtool: vmalloc: allocation failure: 0 bytes,
> mode:0x24080c2(GFP_KERNEL|__GFP_HIGHMEM|__GFP_ZERO)
> <snip>
> Call Trace:
> [<ffffffff813bde47>] dump_stack+0x63/0x8c
> [<ffffffff811b0a1f>] warn_alloc+0x13f/0x170
> [<ffffffff811f0476>] __vmalloc_node_range+0x1e6/0x2c0
> [<ffffffff811f0874>] vzalloc+0x54/0x60
> [<ffffffff8169986c>] dev_ethtool+0xb4c/0x1b30
> [<ffffffff816adbb1>] dev_ioctl+0x181/0x520
> [<ffffffff816714d2>] sock_do_ioctl+0x42/0x50
> <snip>
> Mem-Info:
> active_anon:435809 inactive_anon:173951 isolated_anon:0
> active_file:835822 inactive_file:196932 isolated_file:0
> unevictable:0 dirty:8 writeback:0 unstable:0
> slab_reclaimable:157732 slab_unreclaimable:10022
> mapped:83042 shmem:306356 pagetables:9507 bounce:0
> free:130041 free_pcp:1080 free_cma:0
> Node 0 active_anon:1743236kB inactive_anon:695804kB active_file:3343288kB
> inactive_file:787728kB unevictable:0kB isolated(anon):0kB isolated(file):0kB
> mapped:332168kB dirty:32kB writeback:0kB shmem:0kB shmem_thp: 0kB
> shmem_pmdmapped: 0kB anon_thp: 1225424kB writeback_tmp:0kB unstable:0kB
> pages_scanned:0 all_unreclaimable? no
> Node 0 DMA free:15900kB min:136kB low:168kB high:200kB active_anon:0kB
> inactive_anon:0kB active_file:0kB inactive_file:0kB unevictable:0kB
> writepending:0kB present:15984kB managed:15900kB mlocked:0kB
> slab_reclaimable:0kB slab_unreclaimable:0kB kernel_stack:0kB pagetables:0kB
> bounce:0kB free_pcp:0kB local_pcp:0kB free_cma:0kB
> lowmem_reserve[]: 0 3187 7643 7643
> Node 0 DMA32 free:419732kB min:28124kB low:35152kB high:42180kB
> active_anon:541180kB inactive_anon:248988kB active_file:1466388kB
> inactive_file:389632kB unevictable:0kB writepending:0kB present:3370280kB
> managed:3290932kB mlocked:0kB slab_reclaimable:217184kB
> slab_unreclaimable:4180kB kernel_stack:160kB pagetables:984kB bounce:0kB
> free_pcp:2236kB local_pcp:660kB free_cma:0kB
> lowmem_reserve[]: 0 0 4456 4456
>
> Signed-off-by: Stanislaw Gruszka <[email protected]>
> ---
> v1 -> v2: nullify regbuf to avoid using uninitialized variable in line:
> if (regbuf && copy_to_user(useraddr, regbuf, regs.len))
>
> net/core/ethtool.c | 9 ++++++---
> 1 file changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/net/core/ethtool.c b/net/core/ethtool.c
> index 6b3eee0..4b6dc9b 100644
> --- a/net/core/ethtool.c
> +++ b/net/core/ethtool.c
> @@ -1405,9 +1405,12 @@ static int ethtool_get_regs(struct net_device *dev,
> char __user *useraddr)
> if (regs.len > reglen)
> regs.len = reglen;
>
> - regbuf = vzalloc(reglen);
> - if (reglen && !regbuf)
> - return -ENOMEM;
> + regbuf = NULL;
Any reason to prefer this over changing the declaration to include
the assignment?
void *regbuf = NULL;
> + if (reglen) {
> + regbuf = vzalloc(reglen);
> + if (!regbuf)
> + return -ENOMEM;
> + }
>
> ops->get_regs(dev, ®s, regbuf);
>
> --
> 1.8.3.1
>
>
--
John W. Linville Someday the world will need a hero, and you
[email protected] might be all we have. Be ready.
