From: Stephen Smalley <email@example.com> To: Will Deacon <firstname.lastname@example.org> Cc: email@example.com, firstname.lastname@example.org, "email@example.com" <firstname.lastname@example.org>, linuxfs <email@example.com>, Eric Paris <firstname.lastname@example.org> Subject: Re: [RFC PATCH 2/2] selinux: Propagate RCU walk status from 'security_inode_follow_link()' Date: Wed, 20 Nov 2019 08:31:36 -0500 [thread overview] Message-ID: <email@example.com> (raw) In-Reply-To: <20191120131303.GB21500@willie-the-truck> On 11/20/19 8:13 AM, Will Deacon wrote: > Hi Stephen, > > Thanks for the quick review. > > On Tue, Nov 19, 2019 at 01:46:37PM -0500, Stephen Smalley wrote: >> On 11/19/19 1:40 PM, Will Deacon wrote: >>> 'selinux_inode_follow_link()' can be called as part of an RCU path walk, >>> and is passed a 'bool rcu' parameter to indicate whether or not it is >>> being called from within an RCU read-side critical section. >>> >>> Unfortunately, this knowledge is not propagated further and, instead, >>> 'avc_has_perm()' unconditionally passes a flags argument of '0' to both >>> 'avc_has_perm_noaudit()' and 'avc_audit()' which may block. >>> >>> Introduce 'avc_has_perm_flags()' which can be used safely from within an >>> RCU read-side critical section. >> >> Please see e46e01eebbbcf2ff6d28ee7cae9f117e9d1572c8 ("selinux: stop passing >> MAY_NOT_BLOCK to the AVC upon follow_link"). > > Ha, not sure how I missed that -- my patch is almost a direct revert, > including the name 'avs_has_perm_flags()'! My only concern is that the > commit message for e46e01eebbbc asserts that the only use of MAY_NOT_BLOCK > is in slow_avc_audit(), but AVC_NONBLOCKING is used more widely than that. > > For example: > > selinux_inode_follow_link() > -> avc_has_perm() > -> avc_has_perm_noaudit() > -> avc_denied() > -> avc_update_node() > > where we return early if AVC_NONBLOCKING is set, except flags are always > zero on this path. That was introduced by 3a28cff3bd4bf43f02be0c4e7933aebf3dc8197e ("selinux: avoid silent denials in permissive mode under RCU walk") and is only needed if we have to pass MAY_NOT_BLOCK to slow_avc_audit(), which is only presently needed in the selinux_inode_permission() case AFAICT. Both AVC_NONBLOCKING and MAY_NOT_BLOCK are misnomers wrt the AVC since it should never block regardless; the issue IIUC was rather the inability to safely collect the dentry name in an audit message during RCU walk per commit 0dc1ba24f7fff659725eecbba2c9ad679a0954cd (" SELINUX: Make selinux cache VFS RCU walks safe"). I'm not 100% certain about this; it is possible that the test in slow_avc_audit() is wrong and we ought to be doing this for any of LSM_AUDIT_DATA_PATH, _DENTRY, or _INODE (these were split out of LSM_AUDIT_DATA_FS). In that case, we should revert my earlier commit for follow_link and fix the test inside of slow_avc_audit() instead. I cc'd some additional folks who may have insight. Al, tell us if we got it wrong please!
next prev parent reply other threads:[~2019-11-20 13:31 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 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 [this message] 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: 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 \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --subject='Re: [RFC PATCH 2/2] selinux: Propagate RCU walk status from '\''security_inode_follow_link()'\''' \ /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
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).