Re: [PATCH rdma-next] IB/core: Don't register MAD agents for LSM notifications

[Paul Moore <paul@paul-moore.com>]

On Mon, Jan 28, 2019 at 11:57 AM Daniel Jurgens <danielj@mellanox.com> wrote:
> On 1/28/2019 10:37 AM, Paul Moore wrote:
> > On Sun, Jan 27, 2019 at 3:10 AM Leon Romanovsky <leon@kernel.org> wrote:
> >> From: Daniel Jurgens <danielj@mellanox.com>
> >>
> >> ---
> >>  drivers/infiniband/core/security.c | 34 ++++--------------------------
> >>  include/rdma/ib_mad.h              |  3 ---
> >>  2 files changed, 4 insertions(+), 33 deletions(-)
> > Perhaps predictably, I'm not very excited about this change.  Have you
> > looked closer into the slowdown to see where the cycles are being
> > spent?  I'm wondering if the issue is that a large number of notifiers
> > are being registered with the same priority causing the while loop in
> > notifier_chain_register() to take a significant amount of time.
>
> That's what's happening, each MAD agent is registering it's own notifier. The bug reporter was creating hundreds or thousands of  short lived MAD agents. With IRQs disabled too long it resulted in timeouts.
>
> When I initially added the notifier mechanism I thought it was you that said it wasn't really needed, since access wasn't generally revoked in these types of scenarios. Given that I didn't think this would be especially controversial. It was nice to have, unfortunately it causes problems even for users that don't enable SELinux.

Revoking permission is difficult, and in some cases likely impossible,
but that doesn't mean we shouldn't try to make it possible when we
can.  I'd like to see if we can sort this out before we give up and
rip it out.

We might be able to modify notifier_chain_register() such that if the
notifier being registered has the same priority as the current node it
is inserted before the current node.  It looks like all of the IB
notifiers have the same priority so that should speed up registration
significantly in this case, unfortunately that doesn't help
unregistering.  I think we would need to move to convert
notifier_block to use list_head if we want to handle removal in a
timely manner.

However, there is also a concern that delivering notifications to
hundreds of thousands of registered notifiers is going to be
problematic.  None of the above is going to fix that.

I'm trying to quickly understand the MAD agent lifecycle, and it looks
like you have your own register/unregister routines, with locking, so
is it reasonable to assume that it would be possible to iterate over
the MAD agents in the IB code?  I wonder if it would be possible to
group MAD agents (per-port grouping, does that make sense?) such that
several agents would share a single LSM notifier registration?

-- 
paul moore
www.paul-moore.com