netdev - Re: [PATCH ethtool v2 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] [thread-next>] [day] [month] [year] [list]

Message-ID: <20221209180610.b43ttow2qugq7ont@lion.mk-sys.cz>
Date:   Fri, 9 Dec 2022 19:06:10 +0100
From:   Michal Kubecek <mkubecek@...e.cz>
To:     Jesse Brandeburg <jesse.brandeburg@...el.com>
Cc:     netdev@...r.kernel.org
Subject: Re: [PATCH ethtool v2 07/13] ethtool: avoid null pointer dereference

On Fri, Dec 09, 2022 at 09:36:54AM -0800, Jesse Brandeburg wrote:
> Here is the callchain, for reference:
> This is from the command
> # ethtool -s eth0 wol pumbag
> 
> #0  nl_parse_char_bitset
> #1  in nl_parser at netlink/parser.c:1099
> #2  in nl_sset at netlink/settings.c:1247
> #3  in netlink_run_handler at netlink/netlink.c:493
> #4  in main at ethtool.c:6425
> 
> and in the #0 frame above, *nlctx->argp = "pumbag"
> in the callchain above, scan-build doesn't like us de-referencing argp
> because it doesn't have proof it's not null.
> 
> Further I tried putting the check in every element of the stack frame above
> and they all fail the scan-build check still, probably because the pointer
> is advanced to the "pumbag" argument later in the code.

This should be guaranteed by min_argc beeing set to 1 in the relevant
member of sset_params[] array in netlink/setings.c. The dispatcher code
in nl_parser() checks that there are at least ->min_argc parameters left
before calling specific ->handler(). So as long as argc and argv[] are
consistent, we should be safe.

But I suppose we cannot expect the static checker to be smart enough to
see through that logic.

Michal

Download attachment "signature.asc" of type "application/pgp-signature" (489 bytes)