From patchwork Wed Jan 25 15:06:53 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Paul Durrant X-Patchwork-Id: 719701 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.180.67]) by ozlabs.org (Postfix) with ESMTP id 3v7pLv2xR0z9sD6 for ; Thu, 26 Jan 2017 02:07:35 +1100 (AEDT) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1750907AbdAYPHd convert rfc822-to-8bit (ORCPT ); Wed, 25 Jan 2017 10:07:33 -0500 Received: from smtp.eu.citrix.com ([185.25.65.24]:32793 "EHLO SMTP.EU.CITRIX.COM" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750723AbdAYPHc (ORCPT ); Wed, 25 Jan 2017 10:07:32 -0500 X-IronPort-AV: E=Sophos;i="5.33,284,1477958400"; d="scan'208";a="39366517" From: Paul Durrant To: Sowmini Varadhan CC: Konrad Rzeszutek Wilk , Wei Liu , "netdev@vger.kernel.org" , "xen-devel@lists.xenproject.org" Subject: RE: [Xen-devel] xennet_start_xmit assumptions Thread-Topic: [Xen-devel] xennet_start_xmit assumptions Thread-Index: AQHScaACDoaltUjDfE6xVNoHThvJYKE+jTwAgAD85DCAAAw/AIAJvsBg Date: Wed, 25 Jan 2017 15:06:53 +0000 Message-ID: References: <20170118153132.GB9258@oracle.com> <20170118192528.GA6847@char.us.oracle.com> <20170119111426.GA22018@oracle.com> In-Reply-To: <20170119111426.GA22018@oracle.com> Accept-Language: en-GB, en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-ms-exchange-transport-fromentityheader: Hosted MIME-Version: 1.0 Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org > -----Original Message----- > From: Sowmini Varadhan [mailto:sowmini.varadhan@oracle.com] > Sent: 19 January 2017 11:14 > To: Paul Durrant > Cc: Konrad Rzeszutek Wilk ; Wei Liu > ; netdev@vger.kernel.org; xen- > devel@lists.xenproject.org > Subject: Re: [Xen-devel] xennet_start_xmit assumptions > > On (01/19/17 09:36), Paul Durrant wrote: > > > > Hi Sowmini, > > > > Sounds like a straightforward bug to me... netfront should be able > > to handle an empty skb and clearly, if it's relying on skb_headlen() > > being non-zero, that's not the case. > > > > Paul > > I see. Seems like there are 2 things broken here: recovering > from skb->len = 0, and recovering from the more complex > case of (skb->len > 0 && skb_headlen(skb) == 0) > > Do you folks want to take a shot at fixing this, > since you know the code better? If you are interested, > I can share my test program to help you reproduce the > simpler skb->len == 0 case, but it's the fully non-linear > skbs that may be more interesting to reproduce/fix. > > I'll probably work on fixing packet_snd to return -EINVAL > or similar when the len is zero this week. > Sowmini, I knocked together the following patch, which seems to work for me: ---8<--- ---8<--- Making netfront cope with a fully non-linear skb looks like it would be quite intrusive and probably not worth it so I opted for just doing the ETH_HLEN pull-tail if necessary. Can you check it works for you? Paul diff --git a/drivers/net/xen-netfront.c b/drivers/net/xen-netfront.c index 40f26b6..a957c89 100644 --- a/drivers/net/xen-netfront.c +++ b/drivers/net/xen-netfront.c @@ -567,6 +567,10 @@ static int xennet_start_xmit(struct sk_buff *skb, struct net_device *dev) u16 queue_index; struct sk_buff *nskb; + /* Drop packets that are not at least ETH_HLEN in length */ + if (skb->len < ETH_HLEN) + goto drop; + /* Drop the packet if no queues are set up */ if (num_queues < 1) goto drop; @@ -609,6 +613,8 @@ static int xennet_start_xmit(struct sk_buff *skb, struct net_device *dev) } len = skb_headlen(skb); + if ((len < ETH_HLEN) && !__pskb_pull_tail(skb, ETH_HLEN)) + goto drop; spin_lock_irqsave(&queue->tx_lock, flags);