netdev - Re: [IPV6]: Fix IPsec datagram fragmentation

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: <20080215011313.7f5c035f.billfink@mindspring.com>
Date:	Fri, 15 Feb 2008 01:13:13 -0500
From:	Bill Fink <billfink@...dspring.com>
To:	Herbert Xu <herbert@...dor.apana.org.au>
Cc:	David Miller <davem@...emloft.net>, yoshfuji@...ux-ipv6.org,
	netdev@...r.kernel.org, dlstevens@...ibm.com, kazunori@...azawa.org
Subject: Re: [IPV6]: Fix IPsec datagram fragmentation

Hi Herbert,

On Fri, 15 Feb 2008, Herbert Xu wrote:

> On Tue, Feb 12, 2008 at 06:08:28PM -0800, David Miller wrote:
> > 
> > > [IPV6]: Fix IPsec datagram fragmentation
> >
> > Applied, and I'll queue this up to -stable as well.
> 
> Sorry, David Stevens just told me that it doesn't work as intended.
> 
> [IPV6]: Fix reversed local_df test in ip6_fragment
> 
> I managed to reverse the local_df test when forward-porting this
> patch so it actually makes things worse by never fragmenting at
> all.
> 
> Thanks to David Stevens for testing and reporting this bug.
> 
> Signed-off-by: Herbert Xu <herbert@...dor.apana.org.au>
> 
> --
> diff --git a/net/ipv6/ip6_output.c b/net/ipv6/ip6_output.c
> index 4e9a2fe..35ba693 100644
> --- a/net/ipv6/ip6_output.c
> +++ b/net/ipv6/ip6_output.c
> @@ -621,7 +621,7 @@ static int ip6_fragment(struct sk_buff *skb, int (*output)(struct sk_buff *))
>  	 * or if the skb it not generated by a local socket.  (This last
>  	 * check should be redundant, but it's free.)
>  	 */
> -	if (skb->local_df) {
> +	if (!skb->local_df) {
>  		skb->dev = skb->dst->dev;
>  		icmpv6_send(skb, ICMPV6_PKT_TOOBIG, 0, mtu, skb->dev);
>  		IP6_INC_STATS(ip6_dst_idev(skb->dst), IPSTATS_MIB_FRAGFAILS);

I think the setting of skb->local_def is still backwards in your
original patch:

> diff --git a/net/ipv6/ip6_output.c b/net/ipv6/ip6_output.c
> index 9ac6ca2..4e9a2fe 100644
> --- a/net/ipv6/ip6_output.c
> +++ b/net/ipv6/ip6_output.c

...

> @@ -1420,6 +1420,10 @@ int ip6_push_pending_frames(struct sock *sk)
> 		tmp_skb->sk = NULL;
> 	}
> 
> +	/* Allow local fragmentation. */
> +	if (np->pmtudisc >= IPV6_PMTUDISC_DO)
> +		skb->local_df = 1;
> +
> 	ipv6_addr_copy(final_dst, &fl->fl6_dst);
> 	__skb_pull(skb, skb_network_header_len(skb));
> 	if (opt && opt->opt_flen)

I think the test should be:

	if (np->pmtudisc < IPV6_PMTUDISC_DO)

as it is in net/ipv4/ip_output.c:

/* Unless user demanded real pmtu discovery (IP_PMTUDISC_DO), we allow
 * to fragment the frame generated here. No matter, what transforms
 * how transforms change size of the packet, it will come out.
 */
	if (inet->pmtudisc < IP_PMTUDISC_DO)
		skb->local_df = 1;

Or perhaps I'm just missing something obvious.

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