Re: [PATCH 3/3] usb: gadget: u_ether: fix dereference after null check coverify warning

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi,

Peter Chen <hzpeterchen@xxxxxxxxx> writes:
> On Fri, Jul 01, 2016 at 03:33:30PM +0800, Peter Chen wrote:
>> skb is checked for null pointer at above code, so skb might be null.
>> eem_wrap uses it without checking null pointer, fix it by adding null
>> pointer check.
>> 
>> Signed-off-by: Peter Chen <peter.chen@xxxxxxx>
>> ---
>>  drivers/usb/gadget/function/u_ether.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>> 
>> diff --git a/drivers/usb/gadget/function/u_ether.c b/drivers/usb/gadget/function/u_ether.c
>> index 5f562c1..2bc8823 100644
>> --- a/drivers/usb/gadget/function/u_ether.c
>> +++ b/drivers/usb/gadget/function/u_ether.c
>> @@ -549,7 +549,7 @@ static netdev_tx_t eth_start_xmit(struct sk_buff *skb,
>>  		unsigned long	flags;
>>  
>>  		spin_lock_irqsave(&dev->lock, flags);
>> -		if (dev->port_usb)
>> +		if (dev->port_usb && skb)
>>  			skb = dev->wrap(dev->port_usb, skb);
>>  		spin_unlock_irqrestore(&dev->lock, flags);
>>  		if (!skb) {
>> -- 
>
> Felipe, please do not queue this patch, it will break the ncm function
> which the socket buffer may be NULL.
>
> If you are ok with below changes, I will submit patch:
>
> diff --git a/drivers/usb/gadget/function/f_eem.c b/drivers/usb/gadget/function/f_eem.c
> index d58bfc3..5e0db68 100644
> --- a/drivers/usb/gadget/function/f_eem.c
> +++ b/drivers/usb/gadget/function/f_eem.c
> @@ -341,11 +341,15 @@ static struct sk_buff *eem_wrap(struct gether *port, struct sk_buff *skb)
>  {
>  	struct sk_buff	*skb2 = NULL;
>  	struct usb_ep	*in = port->in_ep;
> -	int		padlen = 0;
> +	int 		headroom, tailroom, padlen = 0;
>  	u16		len = skb->len;
>  
> -	int headroom = skb_headroom(skb);
> -	int tailroom = skb_tailroom(skb);
> +	if (!skb)
> +		return NULL;
> +
> +	len = skb->len;
> +	headroom = skb_headroom(skb);
> +	tailroom = skb_tailroom(skb);
>  
>  	/* When (len + EEM_HLEN + ETH_FCS_LEN) % in->maxpacket) is 0,
>  	 * stick two bytes of zero-length EEM packet on the end.
> diff --git a/drivers/usb/gadget/function/f_rndis.c b/drivers/usb/gadget/function/f_rndis.c
> index c800582..16562e4 100644
> --- a/drivers/usb/gadget/function/f_rndis.c
> +++ b/drivers/usb/gadget/function/f_rndis.c
> @@ -374,6 +374,9 @@ static struct sk_buff *rndis_add_header(struct gether *port,
>  {
>  	struct sk_buff *skb2;
>  
> +	if (!skb)
> +		return NULL;
> +
>  	skb2 = skb_realloc_headroom(skb, sizeof(struct rndis_packet_msg_type));
>  	rndis_add_hdr(skb2);
>

looks okay to me. Patch 1/3 and 2/3 are still fine, right?

-- 
balbi

Attachment: signature.asc
Description: PGP signature


[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux