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-next>] [day] [month] [year] [list]
Message-ID: <20221207105217.mw7qkm3l3go2dqri@lion.mk-sys.cz>
Date:   Wed, 7 Dec 2022 11:52:17 +0100
From:   Michal Kubecek <mkubecek@...e.cz>
To:     Jesse Brandeburg <jesse.brandeburg@...el.com>
Cc:     e1000-patches@...ists.intel.com, netdev@...r.kernel.org
Subject: Re: [PATCH ethtool v1 07/13] ethtool: avoid null pointer dereference

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.

> 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.

Michal

> ---
>  netlink/parser.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/netlink/parser.c b/netlink/parser.c
> index 70f451008eb4..c573a9598a9f 100644
> --- a/netlink/parser.c
> +++ b/netlink/parser.c
> @@ -874,7 +874,7 @@ int nl_parse_bitset(struct nl_context *nlctx, uint16_t type, const void *data,
>   * optionally followed by '/' and another numeric value (mask, unless no_mask
>   * is set), or a string consisting of characters corresponding to bit indices.
>   * The @data parameter points to struct char_bitset_parser_data. Generates
> - * biset nested attribute. Fails if type is zero or if @dest is not null.
> + * bitset nested attribute. Fails if type is zero or if @dest is not null.
>   */
>  int nl_parse_char_bitset(struct nl_context *nlctx, uint16_t type,
>  			 const void *data, struct nl_msg_buff *msgbuff,
> @@ -882,7 +882,7 @@ int nl_parse_char_bitset(struct nl_context *nlctx, uint16_t type,
>  {
>  	const struct char_bitset_parser_data *parser_data = data;
>  
> -	if (!type || dest) {
> +	if (!type || dest || !*nlctx->argp) {
>  		fprintf(stderr, "ethtool (%s): internal error parsing '%s'\n",
>  			nlctx->cmd, nlctx->param);
>  		return -EFAULT;
> -- 
> 2.31.1
> 

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

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ