Message ID | 1233914158.21135.11.camel@localhost.localdomain |
---|---|
State | Accepted, archived |
Delegated to: | David Miller |
Headers | show |
From: Jesper Dangaard Brouer <jdb@comx.dk> Date: Fri, 06 Feb 2009 10:55:58 +0100 > On Fri, 2009-02-06 at 01:08 -0800, David Miller wrote: > > Please respin this patch of your's with proper commit message > > and signoffs, thanks! > > Like the UDP header fix, pskb_may_pull() can potentially > alter the SKB buffer. Thus the saddr and daddr, pointers > may point to the old skb->data buffer. > > I haven't seen corruptions, as its only seen if the old > skb->data buffer were reallocated by another user and > written into very quickly (or poison'd by SLAB debugging). > > Signed-off-by: Jesper Dangaard Brouer <hawk@comx.dk> Applied, thanks! -- 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
Jesper Dangaard Brouer a écrit : > On Fri, 2009-02-06 at 01:08 -0800, David Miller wrote: >> Please respin this patch of your's with proper commit message >> and signoffs, thanks! > > Like the UDP header fix, pskb_may_pull() can potentially > alter the SKB buffer. Thus the saddr and daddr, pointers > may point to the old skb->data buffer. > I dont know... daddr and saddr are not pointers but integers. Patch makes sense as a cleanup, but ChangeLog seems wrong ? > I haven't seen corruptions, as its only seen if the old > skb->data buffer were reallocated by another user and > written into very quickly (or poison'd by SLAB debugging). > > Signed-off-by: Jesper Dangaard Brouer <hawk@comx.dk> > --- > > net/ipv4/udp.c | 6 ++++-- > 1 files changed, 4 insertions(+), 2 deletions(-) > > > diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c > index cc3a0a0..c47c989 100644 > --- a/net/ipv4/udp.c > +++ b/net/ipv4/udp.c > @@ -1234,8 +1234,7 @@ int __udp4_lib_rcv(struct sk_buff *skb, struct udp_table *udptable, > struct udphdr *uh; > unsigned short ulen; > struct rtable *rt = (struct rtable*)skb->dst; > - __be32 saddr = ip_hdr(skb)->saddr; > - __be32 daddr = ip_hdr(skb)->daddr; > + __be32 saddr, daddr; > struct net *net = dev_net(skb->dev); > > /* > @@ -1259,6 +1258,9 @@ int __udp4_lib_rcv(struct sk_buff *skb, struct udp_table *udptable, > if (udp4_csum_init(skb, uh, proto)) > goto csum_error; > > + saddr = ip_hdr(skb)->saddr; > + daddr = ip_hdr(skb)->daddr; > + > if (rt->rt_flags & (RTCF_BROADCAST|RTCF_MULTICAST)) > return __udp4_lib_mcast_deliver(net, skb, uh, > saddr, daddr, udptable); > > -- 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 Fri, 2009-02-06 at 11:04 +0100, Eric Dumazet wrote: > Jesper Dangaard Brouer a écrit : > > On Fri, 2009-02-06 at 01:08 -0800, David Miller wrote: > >> Please respin this patch of your's with proper commit message > >> and signoffs, thanks! > > > > Like the UDP header fix, pskb_may_pull() can potentially > > alter the SKB buffer. Thus the saddr and daddr, pointers > > may point to the old skb->data buffer. > > > > I dont know... daddr and saddr are not pointers but integers. Yes, you are right... its only in the ipv6 code these are pointers (which as DaveM mentioned handels it correctly). > Patch makes sense as a cleanup, but ChangeLog seems wrong ? Okay, lets view it as a cleanup... Its upto DaveM if he wants to fix the commit message (or ask me the correct it, revert and reapply...) > > I haven't seen corruptions, as its only seen if the old > > skb->data buffer were reallocated by another user and > > written into very quickly (or poison'd by SLAB debugging). > > > > Signed-off-by: Jesper Dangaard Brouer <hawk@comx.dk> > > --- > > > > net/ipv4/udp.c | 6 ++++-- > > 1 files changed, 4 insertions(+), 2 deletions(-) > > > > > > diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c > > index cc3a0a0..c47c989 100644 > > --- a/net/ipv4/udp.c > > +++ b/net/ipv4/udp.c > > @@ -1234,8 +1234,7 @@ int __udp4_lib_rcv(struct sk_buff *skb, struct udp_table *udptable, > > struct udphdr *uh; > > unsigned short ulen; > > struct rtable *rt = (struct rtable*)skb->dst; > > - __be32 saddr = ip_hdr(skb)->saddr; > > - __be32 daddr = ip_hdr(skb)->daddr; > > + __be32 saddr, daddr; > > struct net *net = dev_net(skb->dev); > > > > /* > > @@ -1259,6 +1258,9 @@ int __udp4_lib_rcv(struct sk_buff *skb, struct udp_table *udptable, > > if (udp4_csum_init(skb, uh, proto)) > > goto csum_error; > > > > + saddr = ip_hdr(skb)->saddr; > > + daddr = ip_hdr(skb)->daddr; > > + > > if (rt->rt_flags & (RTCF_BROADCAST|RTCF_MULTICAST)) > > return __udp4_lib_mcast_deliver(net, skb, uh, > > saddr, daddr, udptable); > > > > > >
From: Jesper Dangaard Brouer <jdb@comx.dk> Date: Fri, 06 Feb 2009 11:49:22 +0100 > On Fri, 2009-02-06 at 11:04 +0100, Eric Dumazet wrote: > > Jesper Dangaard Brouer a écrit : > > > On Fri, 2009-02-06 at 01:08 -0800, David Miller wrote: > > >> Please respin this patch of your's with proper commit message > > >> and signoffs, thanks! > > > > > > Like the UDP header fix, pskb_may_pull() can potentially > > > alter the SKB buffer. Thus the saddr and daddr, pointers > > > may point to the old skb->data buffer. > > > > > > > I dont know... daddr and saddr are not pointers but integers. > > Yes, you are right... its only in the ipv6 code these are pointers > (which as DaveM mentioned handels it correctly). > > > Patch makes sense as a cleanup, but ChangeLog seems wrong ? > > Okay, lets view it as a cleanup... Its upto DaveM if he wants to fix the > commit message (or ask me the correct it, revert and reapply...) I already pushed the commit out, rebasing the tree is not an option and a revert is super ugly so it's staying as-is. -- 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
diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c index cc3a0a0..c47c989 100644 --- a/net/ipv4/udp.c +++ b/net/ipv4/udp.c @@ -1234,8 +1234,7 @@ int __udp4_lib_rcv(struct sk_buff *skb, struct udp_table *udptable, struct udphdr *uh; unsigned short ulen; struct rtable *rt = (struct rtable*)skb->dst; - __be32 saddr = ip_hdr(skb)->saddr; - __be32 daddr = ip_hdr(skb)->daddr; + __be32 saddr, daddr; struct net *net = dev_net(skb->dev); /* @@ -1259,6 +1258,9 @@ int __udp4_lib_rcv(struct sk_buff *skb, struct udp_table *udptable, if (udp4_csum_init(skb, uh, proto)) goto csum_error; + saddr = ip_hdr(skb)->saddr; + daddr = ip_hdr(skb)->daddr; + if (rt->rt_flags & (RTCF_BROADCAST|RTCF_MULTICAST)) return __udp4_lib_mcast_deliver(net, skb, uh, saddr, daddr, udptable);