From: Jacob Wen <jian.w.wen@oracle.com>
To: Guillaume Nault <gnault@redhat.com>
Cc: netdev@vger.kernel.org, eric.dumazet@gmail.com
Subject: Re: [PATCH net-next v2] net: l2tp: fix reading optional fields of L2TPv3
Date: Mon, 28 Jan 2019 14:15:14 +0800 [thread overview]
Message-ID: <b5f80142-236f-2f54-d17d-3a87451a04e3@oracle.com> (raw)
In-Reply-To: <20190124160107.GA6470@localhost.localdomain>
On 1/25/19 12:01 AM, Guillaume Nault wrote:
> On Thu, Jan 24, 2019 at 03:49:17PM +0800, Jacob Wen wrote:
>> Use pskb_may_pull() to make sure the optional fields are in skb linear
>> parts, so we can safely read them later.
>>
>> It's easy to reproduce the issue with a net driver that supports paged
>> skb data. Just create a L2TPv3 over IP tunnel and then generates some
>> network traffic.
>> Once reproduced, rx err in /sys/kernel/debug/l2tp/tunnels will increase.
>>
>> Signed-off-by: Jacob Wen <jian.w.wen@oracle.com>
>> ---
>> Changes in v2:
>> 1. Only fix L2TPv3 to make code simple.
>> To fix both L2TPv3 and L2TPv2, we'd better refactor l2tp_recv_common.
>> It's complicated to do so.
>>
> Yes, the L2TP data path definitely needs some care. But for a one-off
> patch like this, it'd probably make more sense to respect the current
> code structure instead of adding yet more special cases.
Agree. I didn't give l2tp_udp_recv_core enough attention.
> I mean, l2tp_recv_common() assumes that it can safely access the L2TP
> header: pskb_may_pull() is done in l2tp_udp_recv_core() (which probably
> should pull more bytes in case the length field is present BTW).
Yes. Two more bytes are required for L2TPv2.
> It's up to l2tp_ip (and l2tp_ip6) to respect this requirement, so that's
> where pskb_may_pull() should be done. Yes it'd be better to linearise
> data close to the place we access them, but that'd be long term
> refactoring. If we don't have the resources to do that, let's just, at
> least keep some consistency.
I will send a new patch.
Thanks.
--
Jacob
prev parent reply other threads:[~2019-01-28 6:15 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-01-24 7:49 [PATCH net-next v2] net: l2tp: fix reading optional fields of L2TPv3 Jacob Wen
2019-01-24 16:01 ` Guillaume Nault
2019-01-28 6:15 ` Jacob Wen [this message]
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=b5f80142-236f-2f54-d17d-3a87451a04e3@oracle.com \
--to=jian.w.wen@oracle.com \
--cc=eric.dumazet@gmail.com \
--cc=gnault@redhat.com \
--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).