All of lore.kernel.org
 help / color / mirror / Atom feed
From: mark.rutland@arm.com (Mark Rutland)
To: linux-arm-kernel@lists.infradead.org
Subject: v4.14-rc2/arm64 kernel BUG at net/core/skbuff.c:2626
Date: Mon, 2 Oct 2017 16:01:57 +0100	[thread overview]
Message-ID: <20171002150156.GC21696@leverpostej> (raw)
In-Reply-To: <CANn89i+zQG=rjHRqzsvPzjg5tqW43Lcz-BJ9spLascP9Nt5z8Q@mail.gmail.com>

On Mon, Oct 02, 2017 at 07:42:17AM -0700, Eric Dumazet wrote:
> On Mon, Oct 2, 2017 at 7:21 AM, Mark Rutland <mark.rutland@arm.com> wrote:
> > Just to check I've understood correctly, are you suggesting that the
> > IPv4 code should also check the dev->mtu against a IP_MIN_MTU (which
> > doesn't seem to exist today)?
> 
> We have plenty of places this is checked.
> 
> For example, trying to set MTU < 68 usually removes IPv4 addresses and routes.
> 
> Problem is : these checks are not fool proof yet.
> 
> ( Only the admin was supposed to play these games )

Sorry, I meant that there was no constant called IP_MIN_MTU, and I was
just looking at the sites fixed up by c780a049f9bf4423.

I appreciate given that this requires admin privileges it's not exactly
high priority. I didn't mean for the above to sound like some kind of
accusation!

> > Otherwise, I do spot another potential issue. The writer side (e.g. most
> > net_device::ndo_change_mtu implementations and the __dev_set_mtu()
> > fallback) doesn't use WRITE_ONCE().
> 
> It does not matter how many strange values can be observed by the reader :
> We must be fool proof anyway from reader point of view, so the
> WRITE_ONCE() is not strictly needed.

Ok. If we expect to always check somewhere on the reader side I guess
that makes sense.

Thanks,
Mark.

WARNING: multiple messages have this Message-ID (diff)
From: Mark Rutland <mark.rutland@arm.com>
To: Eric Dumazet <edumazet@google.com>
Cc: LKML <linux-kernel@vger.kernel.org>,
	netdev <netdev@vger.kernel.org>,
	linux-arm-kernel@lists.infradead.org,
	syzkaller <syzkaller@googlegroups.com>,
	"David S. Miller" <davem@davemloft.net>,
	Willem de Bruijn <willemb@google.com>
Subject: Re: v4.14-rc2/arm64 kernel BUG at net/core/skbuff.c:2626
Date: Mon, 2 Oct 2017 16:01:57 +0100	[thread overview]
Message-ID: <20171002150156.GC21696@leverpostej> (raw)
In-Reply-To: <CANn89i+zQG=rjHRqzsvPzjg5tqW43Lcz-BJ9spLascP9Nt5z8Q@mail.gmail.com>

On Mon, Oct 02, 2017 at 07:42:17AM -0700, Eric Dumazet wrote:
> On Mon, Oct 2, 2017 at 7:21 AM, Mark Rutland <mark.rutland@arm.com> wrote:
> > Just to check I've understood correctly, are you suggesting that the
> > IPv4 code should also check the dev->mtu against a IP_MIN_MTU (which
> > doesn't seem to exist today)?
> 
> We have plenty of places this is checked.
> 
> For example, trying to set MTU < 68 usually removes IPv4 addresses and routes.
> 
> Problem is : these checks are not fool proof yet.
> 
> ( Only the admin was supposed to play these games )

Sorry, I meant that there was no constant called IP_MIN_MTU, and I was
just looking at the sites fixed up by c780a049f9bf4423.

I appreciate given that this requires admin privileges it's not exactly
high priority. I didn't mean for the above to sound like some kind of
accusation!

> > Otherwise, I do spot another potential issue. The writer side (e.g. most
> > net_device::ndo_change_mtu implementations and the __dev_set_mtu()
> > fallback) doesn't use WRITE_ONCE().
> 
> It does not matter how many strange values can be observed by the reader :
> We must be fool proof anyway from reader point of view, so the
> WRITE_ONCE() is not strictly needed.

Ok. If we expect to always check somewhere on the reader side I guess
that makes sense.

Thanks,
Mark.

  reply	other threads:[~2017-10-02 15:01 UTC|newest]

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-10-02 10:49 v4.14-rc2/arm64 kernel BUG at net/core/skbuff.c:2626 Mark Rutland
2017-10-02 10:49 ` Mark Rutland
2017-10-02 13:36 ` Eric Dumazet
2017-10-02 13:36   ` Eric Dumazet
2017-10-02 14:21   ` Mark Rutland
2017-10-02 14:21     ` Mark Rutland
2017-10-02 14:42     ` Eric Dumazet
2017-10-02 14:42       ` Eric Dumazet
2017-10-02 15:01       ` Mark Rutland [this message]
2017-10-02 15:01         ` Mark Rutland
2017-10-03 15:19       ` Dmitry Vyukov
2017-10-03 15:19         ` Dmitry Vyukov
2017-10-03 15:38         ` Eric Dumazet
2017-10-03 15:38           ` Eric Dumazet
2017-10-03 16:06           ` Dmitry Vyukov
2017-10-03 16:06             ` Dmitry Vyukov
2017-10-03 16:36             ` Eric Dumazet
2017-10-03 16:36               ` Eric Dumazet
2017-10-02 14:48     ` Eric Dumazet
2017-10-02 14:48       ` Eric Dumazet
2017-10-02 15:03       ` Mark Rutland
2017-10-02 15:03         ` Mark Rutland
2017-10-02 17:21       ` Mark Rutland
2017-10-02 17:21         ` Mark Rutland
2017-10-02 17:27         ` Eric Dumazet
2017-10-02 17:27           ` Eric Dumazet
2017-10-02 17:34           ` Mark Rutland
2017-10-02 17:34             ` Mark Rutland

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=20171002150156.GC21696@leverpostej \
    --to=mark.rutland@arm.com \
    --cc=linux-arm-kernel@lists.infradead.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 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.