netdev - Re: [PATCH ethtool v1 07/13] ethtool: avoid null pointer dereference

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <0e9cc830-419d-afe2-cabd-991463b1d4bb@intel.com>
Date:   Wed, 7 Dec 2022 10:10:20 -0800
From:   Jesse Brandeburg <jesse.brandeburg@...el.com>
To:     Michal Kubecek <mkubecek@...e.cz>
CC:     <netdev@...r.kernel.org>
Subject: Re: [PATCH ethtool v1 07/13] ethtool: avoid null pointer dereference

On 12/7/2022 2:52 AM, Michal Kubecek wrote:
> On Tue, Dec 06, 2022 at 05:03:47PM -0800, Jesse Brandeburg wrote:
>> '$ scan-build make' reports:
>>
>> Description: Array access (from variable 'arg') results in a null
>> pointer dereference
>> File: /git/ethtool/netlink/parser.c
>> Line: 782
>>
>> Description: Dereference of null pointer (loaded from variable 'p')
>> File: /git/ethtool/netlink/parser.c
>> Line: 794
>>
>> Both of these bugs are prevented by checking the input from netlink
>> which was allowing nlctxt->argp to be NULL.
> 
> It is not input from netlink, nlctx->argp is always one of the members
> in argv[] array and as argc is calculated by kernel (execve() only gets
> argv, not argc), a null pointer could only be a result of a kernel bug.
> 
> If we wanted to check for null argv[] members, it would rather make
> sense to do it somewhere on the global level than in each of the
> handlers separately. But I'm not convinced it would help much, while we
> could catch a null pointer, I'm not sure we could catch an invalid one.

I guess the question here is the value of cleaning up these "marginal" 
possibility issues so we can run the tool with "known clean" code, to 
detect issues added in the future.


>> CC: Michal Kubecek <mkubecek@...e.cz>
>> Fixes: 81a30f416ec7 ("netlink: add bitset command line parser handlers")
>> Signed-off-by: Jesse Brandeburg <jesse.brandeburg@...el.com>
> 
> This patch is marked as "07/13" but I did not receive the rest of the
> series. And even this one was not sent to netdev mailing list.

This one was sent by accident straight to you via git-auto-cc while we 
did internal review. My bad. :-(

My apologies, the full series should be on the list soon, I'll address 
your comments before I send.