All of lore.kernel.org
 help / color / mirror / Atom feed
From: Willem de Bruijn <willemdebruijn.kernel@gmail.com>
To: David Miller <davem@davemloft.net>
Cc: Maxime Chevallier <maxime.chevallier@bootlin.com>,
	LKML <linux-kernel@vger.kernel.org>,
	Network Development <netdev@vger.kernel.org>,
	Willem de Bruijn <willemb@google.com>,
	Eric Dumazet <edumazet@google.com>,
	Antoine Tenart <antoine.tenart@bootlin.com>,
	Thomas Petazzoni <thomas.petazzoni@bootlin.com>
Subject: Re: [PATCH net] packets: Always register packet sk in the same order
Date: Sun, 17 Mar 2019 12:42:01 -0400	[thread overview]
Message-ID: <CAF=yD-LAE+mfs0vkSqmmiCxm9RQNkFBCkjDFN7ScRtfw1kGjtg@mail.gmail.com> (raw)
In-Reply-To: <20190316.182119.1641693988840475497.davem@davemloft.net>

On Sat, Mar 16, 2019 at 9:21 PM David Miller <davem@davemloft.net> wrote:
>
> From: Willem de Bruijn <willemdebruijn.kernel@gmail.com>
> Date: Sat, 16 Mar 2019 14:09:33 -0400
>
> > Note that another consequence of this patch is that insertion on
> > packet create is now O(N) with the number of active packet sockets,
> > due to sklist being an hlist.
>
> Exploitable...

With root in userns? The running time is limited by open file rlimit.
This pattern is already used in a sk_add_node_rcu in some way, so
important to be sure.

In practice I see no significant wall clock time difference when
inserting to a fairly standard default limit of 16K. Regardless of
insertion order, running time is dominated by cleanup on process exit
(synchronize_net barriers?). At higher rlimit it does become
problematic.

The packet socket sklist is not easily converted from hlist to a
regular list, due to the use of seq_hlist_next_rcu in packet_seq_ops.
There is no equivalent seq_list_next_rcu. One option might be instead
to leave insertion order as is, but traverse the list in reverse in
packet_notifier on NETDEV_DOWN. That would require an
sk_for_each_reverse_rcu and hlist_for_each_entry_reverse_rcu. These do
not exist, but since an hlist_pprev_rcu does exist, it is probably
feasible. Though not a trivial change.

Another more narrow option may be to work around the ordering in
fanout itself, e.g., record in the socket the initially assigned
location in the fanout array and try to reclaim this spot on
re-insertion.

  reply	other threads:[~2019-03-17 16:42 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-03-16 13:41 [PATCH net] packets: Always register packet sk in the same order Maxime Chevallier
2019-03-16 18:09 ` Willem de Bruijn
2019-03-17  1:21   ` David Miller
2019-03-17 16:42     ` Willem de Bruijn [this message]
2019-03-19  0:59     ` 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='CAF=yD-LAE+mfs0vkSqmmiCxm9RQNkFBCkjDFN7ScRtfw1kGjtg@mail.gmail.com' \
    --to=willemdebruijn.kernel@gmail.com \
    --cc=antoine.tenart@bootlin.com \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=maxime.chevallier@bootlin.com \
    --cc=netdev@vger.kernel.org \
    --cc=thomas.petazzoni@bootlin.com \
    --cc=willemb@google.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.