netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jesper Dangaard Brouer <jdb@comx.dk>
To: Eric Dumazet <dada1@cosmosbay.com>
Cc: David Miller <davem@davemloft.net>, netdev@vger.kernel.org
Subject: Re: [PATCH] udp: Fix potential wrong ip_hdr(skb) pointers
Date: Fri, 06 Feb 2009 11:49:22 +0100	[thread overview]
Message-ID: <1233917362.21135.16.camel@localhost.localdomain> (raw)
In-Reply-To: <498C0B42.7080309@cosmosbay.com>

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);
> > 
> >
> 
> 
-- 
Med venlig hilsen / Best regards
  Jesper Brouer
  ComX Networks A/S
  Linux Network developer
  Cand. Scient Datalog / MSc.
  Author of http://adsl-optimizer.dk
  LinkedIn: http://www.linkedin.com/in/brouer

  reply	other threads:[~2009-02-06 10:50 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-02-03 13:38 BUG: NIU driver: strange issues with multicast "UDP: short packet" Jesper Dangaard Brouer
2009-02-03 23:38 ` David Miller
2009-02-04  8:55   ` Jesper Dangaard Brouer
2009-02-04  9:00     ` David Miller
2009-02-04  9:32       ` Jesper Dangaard Brouer
2009-02-04  9:34         ` David Miller
2009-02-05 12:44       ` Jesper Dangaard Brouer
2009-02-05 12:47         ` [PATCH] Fix UDP short packet false positive Jesper Dangaard Brouer
2009-02-05 23:06           ` David Miller
2009-02-06  9:00             ` [RFC] " Jesper Dangaard Brouer
2009-02-06  9:08               ` David Miller
2009-02-06  9:55                 ` [PATCH] udp: Fix potential wrong ip_hdr(skb) pointers Jesper Dangaard Brouer
2009-02-06  9:59                   ` David Miller
2009-02-06 10:04                   ` Eric Dumazet
2009-02-06 10:49                     ` Jesper Dangaard Brouer [this message]
2009-02-06 11:11                       ` David Miller
2009-02-04 11:52   ` BUG: NIU driver: strange issues with multicast "UDP: short packet" Herbert Xu

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1233917362.21135.16.camel@localhost.localdomain \
    --to=jdb@comx.dk \
    --cc=dada1@cosmosbay.com \
    --cc=davem@davemloft.net \
    --cc=netdev@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).