netdev - Re: [PATCH] datapath: 16bit inner_network_header field in struct ovs_gso

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAEP_g=_ri4kY+4aBZGjohZkrSJQkxHg7ybo+szFTiouY0AzZvA@mail.gmail.com>
Date:	Tue, 20 May 2014 17:14:25 -0700
From:	Jesse Gross <jesse@...ira.com>
To:	Simon Horman <horms@...ge.net.au>
Cc:	"dev@...nvswitch.org" <dev@...nvswitch.org>,
	netdev <netdev@...r.kernel.org>, Thomas Graf <tgraf@...g.ch>
Subject: Re: [PATCH] datapath: 16bit inner_network_header field in struct ovs_gso_cb

On Tue, May 20, 2014 at 4:31 PM, Simon Horman <horms@...ge.net.au> wrote:
> On Tue, May 20, 2014 at 12:54:32PM -0700, Jesse Gross wrote:
>> On Tue, May 20, 2014 at 3:41 AM, Simon Horman <horms@...ge.net.au> wrote:
>> > diff --git a/datapath/linux/compat/gso.h b/datapath/linux/compat/gso.h
>> > index b83a4c3..f459c18 100644
>> > --- a/datapath/linux/compat/gso.h
>> > +++ b/datapath/linux/compat/gso.h
>> >  #ifdef NET_SKBUFF_DATA_USES_OFFSET
>> > -#define skb_inner_network_header rpl_skb_inner_network_header
>> > -static inline unsigned char *skb_inner_network_header(const struct sk_buff *skb)
>> > -{
>> > -       return skb->head + OVS_GSO_CB(skb)->inner_network_header;
>> > -}
>> > -
>> >  #define skb_inner_mac_header rpl_skb_inner_mac_header
>> >  static inline unsigned char *skb_inner_mac_header(const struct sk_buff *skb)
>> >  {
>> >         return skb->head + OVS_GSO_CB(skb)->inner_mac_header;
>> >  }
>> >
>> > +#define skb_inner_network_header rpl_skb_inner_network_header
>> > +static inline unsigned char *skb_inner_network_header(const struct sk_buff *skb)
>> > +{
>> > +       return skb_inner_mac_header(skb) +
>> > +               OVS_GSO_CB(skb)->inner_network_header;
>> > +}
>> > +
>> >  #else
>>
>> This looks mostly fine to me but doesn't this need to be outside the
>> #ifdef NET_SKBUFF_DATA_USES_OFFSET block? Otherwise we will treat the
>> offset as a pointer on 32 bit kernels.
>
> Thanks, Somehow I completely missed that.
> How about this?
>
>
> From: Simon Horman <horms@...ge.net.au>
>
> [PATCH v1.1] datapath: 16bit inner_network_header field in struct ovs_gso_cb
>
> The motivation for this is to create a 16bit hole in struct ovs_gso_cb
> which may be used for the inner_protocol field which is needed
> for the proposed implementation of compatibility for MPLS GSO segmentation.
>
> This should be safe as inner_network_header is now an offset to
> the inner_mac_header rather than skb->head.
>
> As pointed out by Thomas Graf simply making both inner offsets 16bis is not
> safe as there have been cases of overflow with "with collapsed TCP frames
> on IB when the headroom grew beyond 64K. See commit 50bceae9bd ``tcp:
> Reallocate headroom if it would overflow csum_start'' for additional
> details."
>
> This patch is based on suggestions by Thomas Graf and Jesse Gross.
>
> Cc: Thomas Graf <tgraf@...g.ch>
> Cc: Jesse Gross <jesse@...ira.com>
> Signed-off-by: Simon Horman <horms@...ge.net.au>

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