VIA velocity skb leak.

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>

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)

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

From: Eric Dumazet <dada1@cosmosbay.com>
Date: Thu, 12 Mar 2009 06:14:27 +0100

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

Right, I'll add Dave's patch, but using max_t() here.

Thanks everyone.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html