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 for Android: free password hash cracker in your pocket

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

Message-ID: <CADvbK_eZ5AO3g7uHRykspHouXnvtCTf0dUWOXf8hd3r0wV6Hnw@mail.gmail.com>
Date:	Thu, 13 Mar 2014 13:38:15 +0800
From:	lucien xin <lucien.xin@...il.com>
To:	lucien xin <lucien.xin@...il.com>,
	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 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

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