From patchwork Thu Mar 12 04:56:57 2009 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Dave Jones X-Patchwork-Id: 24334 X-Patchwork-Delegate: davem@davemloft.net Return-Path: X-Original-To: patchwork-incoming@ozlabs.org Delivered-To: patchwork-incoming@ozlabs.org Received: from vger.kernel.org (vger.kernel.org [209.132.176.167]) by ozlabs.org (Postfix) with ESMTP id 7A9BEDE226 for ; Thu, 12 Mar 2009 15:57:16 +1100 (EST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751219AbZCLE5L (ORCPT ); Thu, 12 Mar 2009 00:57:11 -0400 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S1751002AbZCLE5L (ORCPT ); Thu, 12 Mar 2009 00:57:11 -0400 Received: from mx2.redhat.com ([66.187.237.31]:41516 "EHLO mx2.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750835AbZCLE5K (ORCPT ); Thu, 12 Mar 2009 00:57:10 -0400 Received: from int-mx2.corp.redhat.com (int-mx2.corp.redhat.com [172.16.27.26]) by mx2.redhat.com (8.13.8/8.13.8) with ESMTP id n2C4uxje006068; Thu, 12 Mar 2009 00:56:59 -0400 Received: from ns3.rdu.redhat.com (ns3.rdu.redhat.com [10.11.255.199]) by int-mx2.corp.redhat.com (8.13.1/8.13.1) with ESMTP id n2C4ux38025656; Thu, 12 Mar 2009 00:56:59 -0400 Received: from gelk.kernelslacker.org (vpn-12-95.rdu.redhat.com [10.11.12.95]) by ns3.rdu.redhat.com (8.13.8/8.13.8) with ESMTP id n2C4uwf1019226; Thu, 12 Mar 2009 00:56:58 -0400 Received: from gelk.kernelslacker.org (localhost.localdomain [127.0.0.1]) by gelk.kernelslacker.org (8.14.3/8.14.3) with ESMTP id n2C4uvxD011486; Thu, 12 Mar 2009 00:56:57 -0400 Received: (from davej@localhost) by gelk.kernelslacker.org (8.14.3/8.14.3/Submit) id n2C4uv5P011482; Thu, 12 Mar 2009 00:56:57 -0400 X-Authentication-Warning: gelk.kernelslacker.org: davej set sender to davej@redhat.com using -f Date: Thu, 12 Mar 2009 00:56:57 -0400 From: Dave Jones To: Eric Dumazet Cc: David Miller , netdev@vger.kernel.org Subject: Re: VIA velocity skb leak. Message-ID: <20090312045657.GB7132@redhat.com> References: <20090312041352.GA6035@redhat.com> <20090311.211706.68465072.davem@davemloft.net> <20090311.212009.51140677.davem@davemloft.net> <20090312043954.GA7132@redhat.com> <49B89385.4090005@cosmosbay.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <49B89385.4090005@cosmosbay.com> User-Agent: Mutt/1.5.18 (2008-05-17) X-Scanned-By: MIMEDefang 2.58 on 172.16.27.26 Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org 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 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 */