Re: VIA velocity skb leak.

[Eric Dumazet <dada1@cosmosbay.com>]

Dave Jones a écrit :
> On Thu, Mar 12, 2009 at 05:45:57AM +0100, Eric Dumazet wrote:
> 
>  > > @@ -1845,10 +1846,11 @@ static void velocity_free_tx_buf(struct velocity_info *vptr, struct velocity_td_
>  > >  	if (tdinfo->skb_dma) {
>  > >  
>  > > +		pktlen = (skb->len > ETH_ZLEN ? : ETH_ZLEN);
>  > 
>  > I personally find better to use max(skb->len, ETH_ZLEN) macro, but YMMV ;)
>  > 
>  > It actually can avoid you a bug ;)
>  
> I prefer that too, but it makes a warning.
> 
> drivers/net/via-velocity.c:2093: warning: comparison of distinct pointer types lacks a cast
> 
> We can fix this by either casting ETH_ZLEN to an unsigned int,
> or we could just do the diff below..
> 
> Or did I overlook something?
> 
> (if this looks ok, perhaps the other defines could use the same treatment?)
> 
> 	Dave
> 
> The minimum frame length is never signed, define it as
> such so we don't need excessive casts in comparisons.
> 
> Signed-off-by: Dave Jones <davej@redhat.com>
> 
> diff --git a/include/linux/if_ether.h b/include/linux/if_ether.h
> index 7f3c735..c41183e 100644
> --- a/include/linux/if_ether.h
> +++ b/include/linux/if_ether.h
> @@ -30,7 +30,7 @@
>  
>  #define ETH_ALEN	6		/* Octets in one ethernet addr	 */
>  #define ETH_HLEN	14		/* Total octets in header.	 */
> -#define ETH_ZLEN	60		/* Min. octets in frame sans FCS */
> +#define ETH_ZLEN	60U		/* Min. octets in frame sans FCS */
>  #define ETH_DATA_LEN	1500		/* Max. octets in payload	 */
>  #define ETH_FRAME_LEN	1514		/* Max. octets in frame sans FCS */
>  #define ETH_FCS_LEN	4		/* Octets in the FCS		 */
> 

or use max_t(unsigned, skb->len, ETH_ZLEN)