archive mirror
 help / color / mirror / Atom feed
From: Stephen Smalley <>
To: Will Deacon <>
	"" <>,
	linuxfs <>,
Subject: Re: [RFC PATCH 1/2] selinux: Don't call avc_compute_av() from RCU path walk
Date: Wed, 20 Nov 2019 10:28:31 -0500	[thread overview]
Message-ID: <> (raw)
In-Reply-To: <20191120131229.GA21500@willie-the-truck>

On 11/20/19 8:12 AM, Will Deacon wrote:
> Hi Stephen,
> Thanks for the quick reply.
> On Tue, Nov 19, 2019 at 01:59:40PM -0500, Stephen Smalley wrote:
>> On 11/19/19 1:40 PM, Will Deacon wrote:
>>> 'avc_compute_av()' can block, so we carefully exit the RCU read-side
>>> critical section before calling it in 'avc_has_perm_noaudit()'.
>>> Unfortunately, if we're calling from the VFS layer on the RCU path walk
>>> via 'selinux_inode_permission()' then we're still actually in an RCU
>>> read-side critical section and must not block.
>> avc_compute_av() should never block AFAIK. The blocking concern was with
>> slow_avc_audit(), and even that appears dubious to me. That seems to be more
>> about misuse of d_find_alias in dump_common_audit_data() than anything.
> Apologies, I lost track of GFP_ATOMIC when I reading the code and didn't
> think it was propagated down to all of the potential allocations and
> string functions. Having looked at it again, I can't see where it blocks.
> Might be worth a comment in avc_compute_av(), because the temporary
> dropping of rcu_read_lock() looks really dodgy when we could be running
> on the RCU path walk path anyway.

I don't think that's a problem but I'll defer to the fsdevel and rcu 
folks.  The use of RCU within the SELinux AVC long predates the 
introduction of RCU path walk, and the rcu_read_lock()/unlock() pairs 
inside the AVC are not related in any way to RCU path walk.  Hopefully 
they don't break it.  The SELinux security server (i.e. 
security_compute_av() and the rest of security/selinux/ss/*) internally 
has its own locking for its data structures, primarily the policy 
rwlock.  There was also a patch long ago to convert use of that policy 
rwlock to RCU but it didn't seem justified at the time.  We are 
interested in revisiting that however.  That would introduce its own set 
of rcu_read_lock/unlock pairs inside of security_compute_av() as well.

  reply	other threads:[~2019-11-20 15:29 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-11-19 18:40 [RFC PATCH 0/2] Avoid blocking in selinux inode callbacks on RCU walk Will Deacon
2019-11-19 18:40 ` [RFC PATCH 1/2] selinux: Don't call avc_compute_av() from RCU path walk Will Deacon
2019-11-19 18:59   ` Stephen Smalley
2019-11-20 13:12     ` Will Deacon
2019-11-20 15:28       ` Stephen Smalley [this message]
2019-11-20 19:07         ` Paul E. McKenney
2019-11-20 19:13           ` Will Deacon
2019-11-19 18:40 ` [RFC PATCH 2/2] selinux: Propagate RCU walk status from 'security_inode_follow_link()' Will Deacon
2019-11-19 18:46   ` Stephen Smalley
2019-11-20 13:13     ` Will Deacon
2019-11-20 13:31       ` Stephen Smalley
2019-11-29  7:36   ` [selinux] 5149a783b9: WARNING:at_security/selinux/avc.c:#avc_has_perm_flags kernel test robot

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:

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \ \ \ \ \ \ \ \ \

* 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).