Message ID | CALYGNiP-0MZ-FExV2HutTvE9U-QQtkKSoE--KN=JQE5STYsjAA@mail.gmail.com |
---|---|
State | RFC, archived |
Delegated to: | David Miller |
Headers | show |
On Wed, Jan 6, 2016 at 11:15 AM, Konstantin Khlebnikov <koct9i@gmail.com> wrote: > Looks like this happens because ip_options_fragment() relies on > correct ip options length in ip control block in skb. But in > ip_finish_output_gso() control block in segments is reused by > skb_gso_segment(). following ip_fragment() sees some garbage. > > In my case there was no ip options but length becomes non-zero and > ip_options_fragment() picked some bytes from payload and decides to > fill huge range with IPOPT_NOOP (1). One of that ones flipped nr_frags > in skb_shared_info at the end of data =) > Hmm, it looks like SKB_GSO_CB should be cleared after skb_gso_segment() since all the gso information should be saved in shared_info after it finishes. Does a memset(0) on SKB_GSO_CB after skb_gso_segment() work as well? -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Jan 6, 2016 at 10:59 PM, Cong Wang <xiyou.wangcong@gmail.com> wrote: > On Wed, Jan 6, 2016 at 11:15 AM, Konstantin Khlebnikov <koct9i@gmail.com> wrote: >> Looks like this happens because ip_options_fragment() relies on >> correct ip options length in ip control block in skb. But in >> ip_finish_output_gso() control block in segments is reused by >> skb_gso_segment(). following ip_fragment() sees some garbage. >> >> In my case there was no ip options but length becomes non-zero and >> ip_options_fragment() picked some bytes from payload and decides to >> fill huge range with IPOPT_NOOP (1). One of that ones flipped nr_frags >> in skb_shared_info at the end of data =) >> > > Hmm, it looks like SKB_GSO_CB should be cleared after skb_gso_segment() > since all the gso information should be saved in shared_info after it finishes. > > Does a memset(0) on SKB_GSO_CB after skb_gso_segment() work as well? This will break present logic around ip_options_fragment() - it clears options from second and following fragments. With zeroed cb it will do nothing. ip_options_fragment() can get required information directly from ip header but it also resets fields in IPCB -- probably it should stay valid here and somebody else will use it later. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
--- a/include/linux/skbuff.h +++ b/include/linux/skbuff.h @@ -3316,6 +3316,7 @@ static inline struct sec_path *skb_sec_path(struct sk_buff *skb) * Keeps track of level of encapsulation of network headers. */ struct skb_gso_cb { + char pad[32]; /* inet_skb_parm lives here */ int mac_offset; int encap_level; __u16 csum_start; And debug which prevents kernel crash too. --- a/net/ipv4/ip_options.c +++ b/net/ipv4/ip_options.c @@ -215,6 +215,10 @@ void ip_options_fragment(struct sk_buff *skb) int l = opt->optlen; int optlen; + const struct iphdr *iph = ip_hdr(skb); + l = iph->ihl * 4 - sizeof(struct iphdr); + WARN(opt->optlen != l, "%s %d != %d\n", __func__, opt->optlen, l); + while (l > 0) { switch (*optptr) { case IPOPT_END: