linux-kernel - Re: [PATCH v3] staging: ks7010: Add null pointer check for skb

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: <20180927180443.yjqycqa7jir6rb57@localhost.localdomain>
Date:   Thu, 27 Sep 2018 19:04:43 +0100
From:   Aymen Qader <qader.aymen@...il.com>
To:     Wolfram Sang <wsa@...-dreams.de>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        devel@...verdev.osuosl.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3] staging: ks7010: Add null pointer check for skb

Retraction: in hindsight I see that with the current usage of this
function, there is already a check for the socket buffer so this check
is unnecessary. However, I'm not sure if it's considered good practice
to keep this check anyway--in any case, ENOMEM isn't the right error
to return.

On Thu, Sep 27, 2018 at 04:16:13PM +0100, Aymen Qader wrote:
> Add a null pointer check for the socket buffer in ks_hostif.c to avoid a
> possible null pointer deference, and remove a later now-redundant null
> pointer check.
> 
> Signed-off-by: Aymen Qader <qader.aymen@...il.com>
> ---
> v2: Remove redundant pointer check
> v3: Style fix
> 
>  drivers/staging/ks7010/ks_hostif.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/staging/ks7010/ks_hostif.c b/drivers/staging/ks7010/ks_hostif.c
> index 0e554e3359b5..95b6c7557e84 100644
> --- a/drivers/staging/ks7010/ks_hostif.c
> +++ b/drivers/staging/ks7010/ks_hostif.c
> @@ -1011,6 +1011,11 @@ int hostif_data_request(struct ks_wlan_private *priv, struct sk_buff *skb)
>  	size_t size;
>  	int ret;
>  
> +	if (!skb) {
> +		ret = -ENOMEM;
> +		goto err_kfree;
> +	}
> +
>  	skb_len = skb->len;
>  	if (skb_len > ETH_FRAME_LEN) {
>  		netdev_err(priv->net_dev, "bad length skb_len=%d\n", skb_len);
> @@ -1023,7 +1028,6 @@ int hostif_data_request(struct ks_wlan_private *priv, struct sk_buff *skb)
>  	    priv->wpa.mic_failure.stop) {
>  		if (netif_queue_stopped(priv->net_dev))
>  			netif_wake_queue(priv->net_dev);
> -		if (skb)
>  			dev_kfree_skb(skb);
>  
>  		return 0;
> -- 
> 2.17.1
>