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

[Aymen Qader]

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@xxxxxxxxx>
> ---
> 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
>