All of lore.kernel.org
 help / color / mirror / Atom feed
From: David Miller <davem@davemloft.net>
To: netdev@vger.kernel.org
Cc: vladislav.yasevich@hp.com, yjwei@cn.fujitsu.com, jchapman@katalix.com
Subject: ip_queue_xmit() used illegally
Date: Fri, 06 May 2011 12:26:56 -0700 (PDT)	[thread overview]
Message-ID: <20110506.122656.189696988.davem@davemloft.net> (raw)


Several users of ip_queue_xmit() use it illegally.

I've only audited L2TP and SCTP so far, and they both cannot use
ip_queue_xmit() with the way they operate currently.

The issue surrounds how the socket binding is maintained in
inet->inet_daddr, inet->inet_saddr etc.

TCP does things right, in that ip_queue_xmit() is only invoked with
inet->inet_daddr and inet->inet_saddr having fully resolved, final,
fully connected values.

This is an absolute requirement because if the socket's route
invalidates (which happens completely asynchronously) it's going to
lookup a new route using whatever is stored in
inet->inet_{daddr,saddr} and then use those addresses to build the
packet.  Even if ->inet_{saddr,daddr} are both zero this will still
emit a packet (bonus points if you know what addresses will be picked,
no peeking at route.c :-).

SCTP stores it's binding information using transports and assosciations
and does not fill in the ->inet_{daddr,saddr} values.

It tries to work around this route issue by checking dst->obsolete
directly in sctp_packet_transmit(), which just makes the race smaller
and does not eliminate it.  ip_queue_xmit() can still end up with
__sk_dst_check() returning NULL and then we end up emitting a
potentially bogus packet.

L2TP supports more of a datagram type socket semantic than a stream
one, it allows unconnected modes of operation.  And for this reason
it also cannot use ip_queue_xmit() legally.

After a quick cursory scan it seem like DCCP is OK.

I think SCTP could potentially be fixed by simply filling in the
inet->inet_{daddr,saddr} values when it makes an internal binding
of the transport via sctp_transport_route().

L2TP on the other hand will need to use another interface to send ipv4
packets because it allows disconnected operation.

             reply	other threads:[~2011-05-06 19:27 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-05-06 19:26 David Miller [this message]
2011-05-06 20:21 ` ip_queue_xmit() used illegally David Miller
2011-05-06 21:10   ` Vladislav Yasevich
2011-05-06 21:28     ` David Miller

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=20110506.122656.189696988.davem@davemloft.net \
    --to=davem@davemloft.net \
    --cc=jchapman@katalix.com \
    --cc=netdev@vger.kernel.org \
    --cc=vladislav.yasevich@hp.com \
    --cc=yjwei@cn.fujitsu.com \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.