netdev - Re: [PATCH] ipv6: ip6_append_data_mtu do not handle the mtu of the second fragment properly

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: <20140315155519.GA12291@order.stressinduktion.org>
Date:	Sat, 15 Mar 2014 16:55:19 +0100
From:	Hannes Frederic Sowa <hannes@...essinduktion.org>
To:	lucien xin <lucien.xin@...il.com>
Cc:	network dev <netdev@...r.kernel.org>,
	Gao feng <gaofeng@...fujitsu.com>
Subject: Re: [PATCH] ipv6: ip6_append_data_mtu do not handle the mtu of the second fragment properly

On Thu, Mar 13, 2014 at 01:38:15PM +0800, lucien xin wrote:
> On Wed, Mar 12, 2014 at 6:26 PM, Hannes Frederic Sowa
> <hannes@...essinduktion.org> wrote:
> > On Wed, Mar 12, 2014 at 10:40:50AM +0800, lucien xin wrote:
> >
> > It would be possible if we are absolutely sure if we don't call
> > ip6_append_data_mtu a second time, which I have not yet reviewed.
> >
> > The line I proposed above may also suffer from this problem.
> >
> > Maybe you already checked that?
> >
> hmm... this problem do exist.  when it enter "the while( length>0 ){
> }" with skb != NULL first, the problem
> will happen, of course, perhaps there are also other cases that
> trigger that problem.  because  that code seems
> a little mess， I hope the following change can make it more clear and
> eliminate potential insecurity,
> pls help to check it

The diff is good, thanks!

> diff --git a/net/ipv6/ip6_output.c b/net/ipv6/ip6_output.c
> index 2bc1070..07ac8f9 100644
> --- a/net/ipv6/ip6_output.c
> +++ b/net/ipv6/ip6_output.c
> @@ -1101,21 +1101,19 @@ static void ip6_append_data_mtu(unsigned int
> *mtu,
>                                 unsigned int fragheaderlen,
>                                 struct sk_buff *skb,
>                                 struct rt6_info *rt,
> -                               bool pmtuprobe)
> +                               unsigned int orig_mtu)
>  {
>         if (!(rt->dst.flags & DST_XFRM_TUNNEL)) {
>                 if (skb == NULL) {
>                         /* first fragment, reserve header_len */
> -                       *mtu = *mtu - rt->dst.header_len;
> +                       *mtu = orig_mtu - rt->dst.header_len;
> 
>                 } else {
>                         /*
>                          * this fragment is not first, the headers
>                          * space is regarded as data space.
>                          */
> -                       *mtu = min(*mtu, pmtuprobe ?
> -                                  rt->dst.dev->mtu :
> -                                  dst_mtu(rt->dst.path));
> +                       *mtu = orig_mtu;
>                 }
>                 *maxfraglen = ((*mtu - fragheaderlen) & ~7)
>                               + fragheaderlen - sizeof(struct frag_hdr);
> @@ -1132,7 +1130,7 @@ int ip6_append_data(struct sock *sk, int
> getfrag(void *from, char *to,
>         struct ipv6_pinfo *np = inet6_sk(sk);
>         struct inet_cork *cork;
>         struct sk_buff *skb, *skb_prev = NULL;
> -       unsigned int maxfraglen, fragheaderlen, mtu;
> +       unsigned int maxfraglen, fragheaderlen, mtu, orig_mtu;
>         int exthdrlen;
>         int dst_exthdrlen;
>         int hh_len;
> @@ -1214,6 +1212,7 @@ int ip6_append_data(struct sock *sk, int
> getfrag(void *from, char *to,
>                 dst_exthdrlen = 0;
>                 mtu = cork->fragsize;
>         }
> +       orig_mtu = mtu;
> 
>         hh_len = LL_RESERVED_SPACE(rt->dst.dev);
> 
> @@ -1313,8 +1312,7 @@ alloc_new_skb:
>                         if (skb == NULL || skb_prev == NULL)
>                                 ip6_append_data_mtu(&mtu, &maxfraglen,
>                                                     fragheaderlen,
> skb, rt,
> -                                                   np->pmtudisc >=
> -                                                   IPV6_PMTUDISC_PROBE);
> +                                                   orig_mtu);
> 
>                         skb_prev = skb;

--
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