linux-kernel - Re: [PATCH] usb: gadget: rndis: Avoid dereference before NULL check

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]

Date:   Fri, 9 Sep 2022 07:38:07 +0200
From:   Greg KH <gregkh@...uxfoundation.org>
To:     Jim Lin <jilin@...dia.com>
Cc:     balbi@...nel.org, linux-usb@...r.kernel.org,
        linux-kernel@...r.kernel.org, Aniruddha TVS Rao <anrao@...dia.com>
Subject: Re: [PATCH] usb: gadget: rndis: Avoid dereference before NULL check

On Fri, Sep 09, 2022 at 01:56:15AM +0800, Jim Lin wrote:
> NULL check is performed after params->dev is dereferenced in
> dev_get_stats.

I do not understand this statement.

> Fixed by adding a NULL check before dereferencing params->dev and
> removing subsequent NULL checks for it.
> 
> Signed-off-by: Aniruddha TVS Rao <anrao@...dia.com>
> Signed-off-by: Jim Lin <jilin@...dia.com>
> ---
>  drivers/usb/gadget/function/rndis.c | 37 ++++++++++++-----------------
>  1 file changed, 15 insertions(+), 22 deletions(-)
> 
> diff --git a/drivers/usb/gadget/function/rndis.c b/drivers/usb/gadget/function/rndis.c
> index 64de9f1b874c..d2f18f34c8e5 100644
> --- a/drivers/usb/gadget/function/rndis.c
> +++ b/drivers/usb/gadget/function/rndis.c
> @@ -198,6 +198,9 @@ static int gen_ndis_query_resp(struct rndis_params *params, u32 OID, u8 *buf,
>  	outbuf = (__le32 *)&resp[1];
>  	resp->InformationBufferOffset = cpu_to_le32(16);
>  
> +	if (!params->dev)
> +		return -ENODEV;
> +

As Sergey points out, this check is useless and the ones below should
also be removed.

But, why make this check at all, how did you trigger a problem with the
current code?

Are you using this driver?  If so, why?  It is totally broken (as per
the specification) and we really really need to just delete it from the
tree to prevent anyone else from ever using it.

thanks,

greg k-h