On 01/28/2019 06:03 PM, Paul Moore wrote:
On Mon, Jan 28, 2019 at 11:57 AM Daniel Jurgens <danielj@xxxxxxxxxxxx> wrote:
On 1/28/2019 10:37 AM, Paul Moore wrote:
On Sun, Jan 27, 2019 at 3:10 AM Leon Romanovsky <leon@xxxxxxxxxx> wrote:
From: Daniel Jurgens <danielj@xxxxxxxxxxxx>
---
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.
Agree that it'd be nice to see it work, but since few (no other?) subsys registered
with selinux/security is doing this, can we 'release' the IB mad agents from this
'imprisonment' and figure out a general solution that resolves this issue, and
then it can be implemented not only for ib mad packet access, but for other
subsys using selinux/security, and wanting the same revocation support.
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.
sooo....
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?
one can have numerous MAD agents. it isn't just one.
Think: you can have any number of apps scanning the fabric for various stats
to draw node/graph pics, gather per-port packet counts, gather bw numbers,
etc. For IB, scanning the fabric involves 'mad pkts'.
In addition to scanning, one could do 'less then nice things' to switches,
not just read stats but set parameters... like routing table entries.
and thus, the security level addition.
One is highly unlikely to grant access to an app to do mad pkt generation/use,
then change your mind (semi-?)randomly to enable/disable it.... generally always enabled,
or always disabled.
Let's take this case as a catalyst to resolve a previously unknown perf issue,
and not hold it up to a higher functional requirement then other apps that want to be secured.
thanks...
-dd
