| Message ID | 20181204204209.21542-1-sds@tycho.nsa.gov (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [RFC] selinux: avoid silent denials in permissive mode under RCU walk | expand |
On 12/4/18 3:42 PM, Stephen Smalley wrote: > commit 0dc1ba24f7fff6 ("SELINUX: Make selinux cache VFS RCU walks safe") > results in no audit messages at all if in permissive mode because the > cache is updated during the rcu walk and thus no denial occurs on > the subsequent ref walk. Fix this by not updating the cache when > performing a non-blocking permission check. This only affects search > and symlink read checks during rcu walk. Probably should drop "under RCU walk" from the subject line since the denials remain silent during the rcu walk itself but are then audited properly upon falling back to the ref walk when a denial occurs. > > Fixes: 0dc1ba24f7fff6 ("SELINUX: Make selinux cache VFS RCU walks safe") > Reported-by: BMK <bmktuwien@gmail.com> > Signed-off-by: Stephen Smalley <sds@tycho.nsa.gov> > --- > security/selinux/avc.c | 9 ++++++--- > security/selinux/hooks.c | 4 +++- > security/selinux/include/avc.h | 1 + > 3 files changed, 10 insertions(+), 4 deletions(-) > > diff --git a/security/selinux/avc.c b/security/selinux/avc.c > index 635e5c1e3e48..f0e7bc0dc442 100644 > --- a/security/selinux/avc.c > +++ b/security/selinux/avc.c > @@ -1021,8 +1021,10 @@ static noinline int avc_denied(struct selinux_state *state, > !(avd->flags & AVD_FLAGS_PERMISSIVE)) > return -EACCES; > > - avc_update_node(state->avc, AVC_CALLBACK_GRANT, requested, driver, > - xperm, ssid, tsid, tclass, avd->seqno, NULL, flags); > + if (!(flags & AVC_NONBLOCKING)) > + avc_update_node(state->avc, AVC_CALLBACK_GRANT, requested, > + driver, xperm, ssid, tsid, tclass, avd->seqno, > + NULL, flags); > return 0; > } > > @@ -1199,7 +1201,8 @@ int avc_has_perm_flags(struct selinux_state *state, > struct av_decision avd; > int rc, rc2; > > - rc = avc_has_perm_noaudit(state, ssid, tsid, tclass, requested, 0, > + rc = avc_has_perm_noaudit(state, ssid, tsid, tclass, requested, > + (flags & MAY_NOT_BLOCK) ? AVC_NONBLOCKING : 0, > &avd); > > rc2 = avc_audit(state, ssid, tsid, tclass, requested, &avd, rc, > diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c > index 7ce012d9ec51..9b05f84808d9 100644 > --- a/security/selinux/hooks.c > +++ b/security/selinux/hooks.c > @@ -3196,7 +3196,9 @@ static int selinux_inode_permission(struct inode *inode, int mask) > return PTR_ERR(isec); > > rc = avc_has_perm_noaudit(&selinux_state, > - sid, isec->sid, isec->sclass, perms, 0, &avd); > + sid, isec->sid, isec->sclass, perms, > + (flags & MAY_NOT_BLOCK) ? AVC_NONBLOCKING : 0, > + &avd); > audited = avc_audit_required(perms, &avd, rc, > from_access ? FILE__AUDIT_ACCESS : 0, > &denied); > diff --git a/security/selinux/include/avc.h b/security/selinux/include/avc.h > index ef899bcfd2cb..74ea50977c20 100644 > --- a/security/selinux/include/avc.h > +++ b/security/selinux/include/avc.h > @@ -142,6 +142,7 @@ static inline int avc_audit(struct selinux_state *state, > > #define AVC_STRICT 1 /* Ignore permissive mode. */ > #define AVC_EXTENDED_PERMS 2 /* update extended permissions */ > +#define AVC_NONBLOCKING 4 /* non blocking */ > int avc_has_perm_noaudit(struct selinux_state *state, > u32 ssid, u32 tsid, > u16 tclass, u32 requested, >
On Tue, Dec 4, 2018 at 3:39 PM Stephen Smalley <sds@tycho.nsa.gov> wrote: > commit 0dc1ba24f7fff6 ("SELINUX: Make selinux cache VFS RCU walks safe") > results in no audit messages at all if in permissive mode because the > cache is updated during the rcu walk and thus no denial occurs on > the subsequent ref walk. Fix this by not updating the cache when > performing a non-blocking permission check. This only affects search > and symlink read checks during rcu walk. > > Fixes: 0dc1ba24f7fff6 ("SELINUX: Make selinux cache VFS RCU walks safe") > Reported-by: BMK <bmktuwien@gmail.com> > Signed-off-by: Stephen Smalley <sds@tycho.nsa.gov> > --- > security/selinux/avc.c | 9 ++++++--- > security/selinux/hooks.c | 4 +++- > security/selinux/include/avc.h | 1 + > 3 files changed, 10 insertions(+), 4 deletions(-) > > diff --git a/security/selinux/avc.c b/security/selinux/avc.c > index 635e5c1e3e48..f0e7bc0dc442 100644 > --- a/security/selinux/avc.c > +++ b/security/selinux/avc.c > @@ -1021,8 +1021,10 @@ static noinline int avc_denied(struct selinux_state *state, > !(avd->flags & AVD_FLAGS_PERMISSIVE)) > return -EACCES; > > - avc_update_node(state->avc, AVC_CALLBACK_GRANT, requested, driver, > - xperm, ssid, tsid, tclass, avd->seqno, NULL, flags); > + if (!(flags & AVC_NONBLOCKING)) > + avc_update_node(state->avc, AVC_CALLBACK_GRANT, requested, > + driver, xperm, ssid, tsid, tclass, avd->seqno, > + NULL, flags); I realize the issue here is specific to the VFS code paths, but if we are going to introduce a general AVC flag to tell the AVC code not to block, it seems like we should honor it everywhere. I would suggest moving the check into avc_update_node() itself (it's static, so hopefully any perf impact will be negligible). > return 0; > } > > @@ -1199,7 +1201,8 @@ int avc_has_perm_flags(struct selinux_state *state, > struct av_decision avd; > int rc, rc2; > > - rc = avc_has_perm_noaudit(state, ssid, tsid, tclass, requested, 0, > + rc = avc_has_perm_noaudit(state, ssid, tsid, tclass, requested, > + (flags & MAY_NOT_BLOCK) ? AVC_NONBLOCKING : 0, > &avd); > > rc2 = avc_audit(state, ssid, tsid, tclass, requested, &avd, rc, > diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c > index 7ce012d9ec51..9b05f84808d9 100644 > --- a/security/selinux/hooks.c > +++ b/security/selinux/hooks.c > @@ -3196,7 +3196,9 @@ static int selinux_inode_permission(struct inode *inode, int mask) > return PTR_ERR(isec); > > rc = avc_has_perm_noaudit(&selinux_state, > - sid, isec->sid, isec->sclass, perms, 0, &avd); > + sid, isec->sid, isec->sclass, perms, > + (flags & MAY_NOT_BLOCK) ? AVC_NONBLOCKING : 0, > + &avd); > audited = avc_audit_required(perms, &avd, rc, > from_access ? FILE__AUDIT_ACCESS : 0, > &denied); > diff --git a/security/selinux/include/avc.h b/security/selinux/include/avc.h > index ef899bcfd2cb..74ea50977c20 100644 > --- a/security/selinux/include/avc.h > +++ b/security/selinux/include/avc.h > @@ -142,6 +142,7 @@ static inline int avc_audit(struct selinux_state *state, > > #define AVC_STRICT 1 /* Ignore permissive mode. */ > #define AVC_EXTENDED_PERMS 2 /* update extended permissions */ > +#define AVC_NONBLOCKING 4 /* non blocking */ > int avc_has_perm_noaudit(struct selinux_state *state, > u32 ssid, u32 tsid, > u16 tclass, u32 requested, > -- > 2.19.2 >
On Fri, Dec 7, 2018 at 12:58 AM Paul Moore <paul@paul-moore.com> wrote: > > On Tue, Dec 4, 2018 at 3:39 PM Stephen Smalley <sds@tycho.nsa.gov> wrote: > > commit 0dc1ba24f7fff6 ("SELINUX: Make selinux cache VFS RCU walks safe") > > results in no audit messages at all if in permissive mode because the > > cache is updated during the rcu walk and thus no denial occurs on > > the subsequent ref walk. Fix this by not updating the cache when > > performing a non-blocking permission check. This only affects search > > and symlink read checks during rcu walk. > > > > Fixes: 0dc1ba24f7fff6 ("SELINUX: Make selinux cache VFS RCU walks safe") > > Reported-by: BMK <bmktuwien@gmail.com> > > Signed-off-by: Stephen Smalley <sds@tycho.nsa.gov> > > --- > > security/selinux/avc.c | 9 ++++++--- > > security/selinux/hooks.c | 4 +++- > > security/selinux/include/avc.h | 1 + > > 3 files changed, 10 insertions(+), 4 deletions(-) > > > > diff --git a/security/selinux/avc.c b/security/selinux/avc.c > > index 635e5c1e3e48..f0e7bc0dc442 100644 > > --- a/security/selinux/avc.c > > +++ b/security/selinux/avc.c > > @@ -1021,8 +1021,10 @@ static noinline int avc_denied(struct selinux_state *state, > > !(avd->flags & AVD_FLAGS_PERMISSIVE)) > > return -EACCES; > > > > - avc_update_node(state->avc, AVC_CALLBACK_GRANT, requested, driver, > > - xperm, ssid, tsid, tclass, avd->seqno, NULL, flags); > > + if (!(flags & AVC_NONBLOCKING)) > > + avc_update_node(state->avc, AVC_CALLBACK_GRANT, requested, > > + driver, xperm, ssid, tsid, tclass, avd->seqno, > > + NULL, flags); > > I realize the issue here is specific to the VFS code paths, but if we > are going to introduce a general AVC flag to tell the AVC code not to > block, it seems like we should honor it everywhere. I would suggest > moving the check into avc_update_node() itself (it's static, so > hopefully any perf impact will be negligible). > > > return 0; > > } > > > > @@ -1199,7 +1201,8 @@ int avc_has_perm_flags(struct selinux_state *state, > > struct av_decision avd; > > int rc, rc2; > > > > - rc = avc_has_perm_noaudit(state, ssid, tsid, tclass, requested, 0, > > + rc = avc_has_perm_noaudit(state, ssid, tsid, tclass, requested, > > + (flags & MAY_NOT_BLOCK) ? AVC_NONBLOCKING : 0, > > &avd); > > > > rc2 = avc_audit(state, ssid, tsid, tclass, requested, &avd, rc, > > diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c > > index 7ce012d9ec51..9b05f84808d9 100644 > > --- a/security/selinux/hooks.c > > +++ b/security/selinux/hooks.c > > @@ -3196,7 +3196,9 @@ static int selinux_inode_permission(struct inode *inode, int mask) > > return PTR_ERR(isec); > > > > rc = avc_has_perm_noaudit(&selinux_state, > > - sid, isec->sid, isec->sclass, perms, 0, &avd); > > + sid, isec->sid, isec->sclass, perms, > > + (flags & MAY_NOT_BLOCK) ? AVC_NONBLOCKING : 0, > > + &avd); > > audited = avc_audit_required(perms, &avd, rc, > > from_access ? FILE__AUDIT_ACCESS : 0, > > &denied); > > diff --git a/security/selinux/include/avc.h b/security/selinux/include/avc.h > > index ef899bcfd2cb..74ea50977c20 100644 > > --- a/security/selinux/include/avc.h > > +++ b/security/selinux/include/avc.h > > @@ -142,6 +142,7 @@ static inline int avc_audit(struct selinux_state *state, > > > > #define AVC_STRICT 1 /* Ignore permissive mode. */ > > #define AVC_EXTENDED_PERMS 2 /* update extended permissions */ > > +#define AVC_NONBLOCKING 4 /* non blocking */ > > int avc_has_perm_noaudit(struct selinux_state *state, > > u32 ssid, u32 tsid, > > u16 tclass, u32 requested, > > -- > > 2.19.2 > > > > -- > paul moore > www.paul-moore.com I just want to confirm, that the patch of Stephen fixed the bug in the kernel 4.9. It seems to work stable now... I'll keep an eye on it and report if any other issues should surface. Again, thank you all so much. Really appreciate it.
On 12/6/18 6:57 PM, Paul Moore wrote: > On Tue, Dec 4, 2018 at 3:39 PM Stephen Smalley <sds@tycho.nsa.gov> wrote: >> commit 0dc1ba24f7fff6 ("SELINUX: Make selinux cache VFS RCU walks safe") >> results in no audit messages at all if in permissive mode because the >> cache is updated during the rcu walk and thus no denial occurs on >> the subsequent ref walk. Fix this by not updating the cache when >> performing a non-blocking permission check. This only affects search >> and symlink read checks during rcu walk. >> >> Fixes: 0dc1ba24f7fff6 ("SELINUX: Make selinux cache VFS RCU walks safe") >> Reported-by: BMK <bmktuwien@gmail.com> >> Signed-off-by: Stephen Smalley <sds@tycho.nsa.gov> >> --- >> security/selinux/avc.c | 9 ++++++--- >> security/selinux/hooks.c | 4 +++- >> security/selinux/include/avc.h | 1 + >> 3 files changed, 10 insertions(+), 4 deletions(-) >> >> diff --git a/security/selinux/avc.c b/security/selinux/avc.c >> index 635e5c1e3e48..f0e7bc0dc442 100644 >> --- a/security/selinux/avc.c >> +++ b/security/selinux/avc.c >> @@ -1021,8 +1021,10 @@ static noinline int avc_denied(struct selinux_state *state, >> !(avd->flags & AVD_FLAGS_PERMISSIVE)) >> return -EACCES; >> >> - avc_update_node(state->avc, AVC_CALLBACK_GRANT, requested, driver, >> - xperm, ssid, tsid, tclass, avd->seqno, NULL, flags); >> + if (!(flags & AVC_NONBLOCKING)) >> + avc_update_node(state->avc, AVC_CALLBACK_GRANT, requested, >> + driver, xperm, ssid, tsid, tclass, avd->seqno, >> + NULL, flags); > > I realize the issue here is specific to the VFS code paths, but if we > are going to introduce a general AVC flag to tell the AVC code not to > block, it seems like we should honor it everywhere. I would suggest > moving the check into avc_update_node() itself (it's static, so > hopefully any perf impact will be negligible). Ok, I'll do that. Just to clarify though, avc_update_node() does not block, nor does any of avc_has_perm_noaudit(). The reason we have to skip avc_update_node() in the non-blocking aka rcu-walk code path is not because it would block but because it would grant the denied permissions in the AVC entry and then avc_audit() would subsequently bail with -ECHILD since it would block on calling the audit system, and when we retry on the ref-walk code path, the cache entry will already contain the permissions and nothing will be denied or audited. So it is a tad confusing. I'll add a comment as well. > >> return 0; >> } >> >> @@ -1199,7 +1201,8 @@ int avc_has_perm_flags(struct selinux_state *state, >> struct av_decision avd; >> int rc, rc2; >> >> - rc = avc_has_perm_noaudit(state, ssid, tsid, tclass, requested, 0, >> + rc = avc_has_perm_noaudit(state, ssid, tsid, tclass, requested, >> + (flags & MAY_NOT_BLOCK) ? AVC_NONBLOCKING : 0, >> &avd); >> >> rc2 = avc_audit(state, ssid, tsid, tclass, requested, &avd, rc, >> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c >> index 7ce012d9ec51..9b05f84808d9 100644 >> --- a/security/selinux/hooks.c >> +++ b/security/selinux/hooks.c >> @@ -3196,7 +3196,9 @@ static int selinux_inode_permission(struct inode *inode, int mask) >> return PTR_ERR(isec); >> >> rc = avc_has_perm_noaudit(&selinux_state, >> - sid, isec->sid, isec->sclass, perms, 0, &avd); >> + sid, isec->sid, isec->sclass, perms, >> + (flags & MAY_NOT_BLOCK) ? AVC_NONBLOCKING : 0, >> + &avd); >> audited = avc_audit_required(perms, &avd, rc, >> from_access ? FILE__AUDIT_ACCESS : 0, >> &denied); >> diff --git a/security/selinux/include/avc.h b/security/selinux/include/avc.h >> index ef899bcfd2cb..74ea50977c20 100644 >> --- a/security/selinux/include/avc.h >> +++ b/security/selinux/include/avc.h >> @@ -142,6 +142,7 @@ static inline int avc_audit(struct selinux_state *state, >> >> #define AVC_STRICT 1 /* Ignore permissive mode. */ >> #define AVC_EXTENDED_PERMS 2 /* update extended permissions */ >> +#define AVC_NONBLOCKING 4 /* non blocking */ >> int avc_has_perm_noaudit(struct selinux_state *state, >> u32 ssid, u32 tsid, >> u16 tclass, u32 requested, >> -- >> 2.19.2 >> >
On Fri, Dec 7, 2018 at 8:04 AM Stephen Smalley <sds@tycho.nsa.gov> wrote: > On 12/6/18 6:57 PM, Paul Moore wrote: > > On Tue, Dec 4, 2018 at 3:39 PM Stephen Smalley <sds@tycho.nsa.gov> wrote: > >> commit 0dc1ba24f7fff6 ("SELINUX: Make selinux cache VFS RCU walks safe") > >> results in no audit messages at all if in permissive mode because the > >> cache is updated during the rcu walk and thus no denial occurs on > >> the subsequent ref walk. Fix this by not updating the cache when > >> performing a non-blocking permission check. This only affects search > >> and symlink read checks during rcu walk. > >> > >> Fixes: 0dc1ba24f7fff6 ("SELINUX: Make selinux cache VFS RCU walks safe") > >> Reported-by: BMK <bmktuwien@gmail.com> > >> Signed-off-by: Stephen Smalley <sds@tycho.nsa.gov> > >> --- > >> security/selinux/avc.c | 9 ++++++--- > >> security/selinux/hooks.c | 4 +++- > >> security/selinux/include/avc.h | 1 + > >> 3 files changed, 10 insertions(+), 4 deletions(-) > >> > >> diff --git a/security/selinux/avc.c b/security/selinux/avc.c > >> index 635e5c1e3e48..f0e7bc0dc442 100644 > >> --- a/security/selinux/avc.c > >> +++ b/security/selinux/avc.c > >> @@ -1021,8 +1021,10 @@ static noinline int avc_denied(struct selinux_state *state, > >> !(avd->flags & AVD_FLAGS_PERMISSIVE)) > >> return -EACCES; > >> > >> - avc_update_node(state->avc, AVC_CALLBACK_GRANT, requested, driver, > >> - xperm, ssid, tsid, tclass, avd->seqno, NULL, flags); > >> + if (!(flags & AVC_NONBLOCKING)) > >> + avc_update_node(state->avc, AVC_CALLBACK_GRANT, requested, > >> + driver, xperm, ssid, tsid, tclass, avd->seqno, > >> + NULL, flags); > > > > I realize the issue here is specific to the VFS code paths, but if we > > are going to introduce a general AVC flag to tell the AVC code not to > > block, it seems like we should honor it everywhere. I would suggest > > moving the check into avc_update_node() itself (it's static, so > > hopefully any perf impact will be negligible). > > Ok, I'll do that. Just to clarify though, avc_update_node() does not > block, nor does any of avc_has_perm_noaudit(). The reason we have to > skip avc_update_node() in the non-blocking aka rcu-walk code path is not > because it would block but because it would grant the denied permissions > in the AVC entry and then avc_audit() would subsequently bail with > -ECHILD since it would block on calling the audit system, and when we > retry on the ref-walk code path, the cache entry will already contain > the permissions and nothing will be denied or audited. So it is a tad > confusing. I'll add a comment as well. Yeah, it took me a little while to realize that, a comment in the code would definitely be welcome. I'm also open to a better spot to put the AVC_NONBLOCKING check; I suggested moving it inside avc_update_node() simply to catch any other (future) callers which may need to use it, but I recognize that isn't a great fit either. As you point out, ultimately the problem is that we don't want to skip the audit record, and since the audit queue might block, we have to play it safe and not update the AVC.
On 12/7/18 8:06 AM, Stephen Smalley wrote: > On 12/6/18 6:57 PM, Paul Moore wrote: >> On Tue, Dec 4, 2018 at 3:39 PM Stephen Smalley <sds@tycho.nsa.gov> wrote: >>> commit 0dc1ba24f7fff6 ("SELINUX: Make selinux cache VFS RCU walks safe") >>> results in no audit messages at all if in permissive mode because the >>> cache is updated during the rcu walk and thus no denial occurs on >>> the subsequent ref walk. Fix this by not updating the cache when >>> performing a non-blocking permission check. This only affects search >>> and symlink read checks during rcu walk. >>> >>> Fixes: 0dc1ba24f7fff6 ("SELINUX: Make selinux cache VFS RCU walks safe") >>> Reported-by: BMK <bmktuwien@gmail.com> >>> Signed-off-by: Stephen Smalley <sds@tycho.nsa.gov> >>> --- >>> security/selinux/avc.c | 9 ++++++--- >>> security/selinux/hooks.c | 4 +++- >>> security/selinux/include/avc.h | 1 + >>> 3 files changed, 10 insertions(+), 4 deletions(-) >>> >>> diff --git a/security/selinux/avc.c b/security/selinux/avc.c >>> index 635e5c1e3e48..f0e7bc0dc442 100644 >>> --- a/security/selinux/avc.c >>> +++ b/security/selinux/avc.c >>> @@ -1021,8 +1021,10 @@ static noinline int avc_denied(struct >>> selinux_state *state, >>> !(avd->flags & AVD_FLAGS_PERMISSIVE)) >>> return -EACCES; >>> >>> - avc_update_node(state->avc, AVC_CALLBACK_GRANT, requested, >>> driver, >>> - xperm, ssid, tsid, tclass, avd->seqno, NULL, >>> flags); >>> + if (!(flags & AVC_NONBLOCKING)) >>> + avc_update_node(state->avc, AVC_CALLBACK_GRANT, >>> requested, >>> + driver, xperm, ssid, tsid, tclass, >>> avd->seqno, >>> + NULL, flags); >> >> I realize the issue here is specific to the VFS code paths, but if we >> are going to introduce a general AVC flag to tell the AVC code not to >> block, it seems like we should honor it everywhere. I would suggest >> moving the check into avc_update_node() itself (it's static, so >> hopefully any perf impact will be negligible). > > Ok, I'll do that. Just to clarify though, avc_update_node() does not > block, nor does any of avc_has_perm_noaudit(). The reason we have to > skip avc_update_node() in the non-blocking aka rcu-walk code path is not > because it would block but because it would grant the denied permissions > in the AVC entry and then avc_audit() would subsequently bail with > -ECHILD since it would block on calling the audit system, and when we > retry on the ref-walk code path, the cache entry will already contain > the permissions and nothing will be denied or audited. So it is a tad > confusing. I'll add a comment as well. Hmm...looking again, I'm confused by the current logic in slow_avc_audit(): /* * When in a RCU walk do the audit on the RCU retry. This is because * the collection of the dname in an inode audit message is not RCU * safe. Note this may drop some audits when the situation changes * during retry. However this is logically just as if the operation * happened a little later. */ if ((a->type == LSM_AUDIT_DATA_INODE) && (flags & MAY_NOT_BLOCK)) return -ECHILD; Since we only do this in the LSM_AUDIT_DATA_INODE case, it isn't clear to me why we are passing down MAY_NOT_BLOCK from selinux_inode_follow_link(), which always sets type to LSM_AUDIT_DATA_DENTRY. If we truly don't need special handling of MAY_NOT_BLOCK within the AVC except for LSM_AUDIT_DATA_INODE, then we don't need to pass it down from selinux_inode_follow_link() and therefore don't need avc_has_perm_flags() at all since that is the only user. Can we dispense with that? Or is the logic in slow_avc_audit() wrong and we ought to be bailing on MAY_NOT_BLOCK for more than just LSM_AUDIT_DATA_INODE? Passing MAY_NOT_BLOCK down to the AVC also seems like a misnomer, since it shouldn't be making blocking calls anyway. > >> >>> return 0; >>> } >>> >>> @@ -1199,7 +1201,8 @@ int avc_has_perm_flags(struct selinux_state >>> *state, >>> struct av_decision avd; >>> int rc, rc2; >>> >>> - rc = avc_has_perm_noaudit(state, ssid, tsid, tclass, >>> requested, 0, >>> + rc = avc_has_perm_noaudit(state, ssid, tsid, tclass, requested, >>> + (flags & MAY_NOT_BLOCK) ? >>> AVC_NONBLOCKING : 0, >>> &avd); >>> >>> rc2 = avc_audit(state, ssid, tsid, tclass, requested, &avd, rc, >>> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c >>> index 7ce012d9ec51..9b05f84808d9 100644 >>> --- a/security/selinux/hooks.c >>> +++ b/security/selinux/hooks.c >>> @@ -3196,7 +3196,9 @@ static int selinux_inode_permission(struct >>> inode *inode, int mask) >>> return PTR_ERR(isec); >>> >>> rc = avc_has_perm_noaudit(&selinux_state, >>> - sid, isec->sid, isec->sclass, >>> perms, 0, &avd); >>> + sid, isec->sid, isec->sclass, perms, >>> + (flags & MAY_NOT_BLOCK) ? >>> AVC_NONBLOCKING : 0, >>> + &avd); >>> audited = avc_audit_required(perms, &avd, rc, >>> from_access ? >>> FILE__AUDIT_ACCESS : 0, >>> &denied); >>> diff --git a/security/selinux/include/avc.h >>> b/security/selinux/include/avc.h >>> index ef899bcfd2cb..74ea50977c20 100644 >>> --- a/security/selinux/include/avc.h >>> +++ b/security/selinux/include/avc.h >>> @@ -142,6 +142,7 @@ static inline int avc_audit(struct selinux_state >>> *state, >>> >>> #define AVC_STRICT 1 /* Ignore permissive mode. */ >>> #define AVC_EXTENDED_PERMS 2 /* update extended permissions */ >>> +#define AVC_NONBLOCKING 4 /* non blocking */ >>> int avc_has_perm_noaudit(struct selinux_state *state, >>> u32 ssid, u32 tsid, >>> u16 tclass, u32 requested, >>> -- >>> 2.19.2 >>> >> >
diff --git a/security/selinux/avc.c b/security/selinux/avc.c index 635e5c1e3e48..f0e7bc0dc442 100644 --- a/security/selinux/avc.c +++ b/security/selinux/avc.c @@ -1021,8 +1021,10 @@ static noinline int avc_denied(struct selinux_state *state, !(avd->flags & AVD_FLAGS_PERMISSIVE)) return -EACCES; - avc_update_node(state->avc, AVC_CALLBACK_GRANT, requested, driver, - xperm, ssid, tsid, tclass, avd->seqno, NULL, flags); + if (!(flags & AVC_NONBLOCKING)) + avc_update_node(state->avc, AVC_CALLBACK_GRANT, requested, + driver, xperm, ssid, tsid, tclass, avd->seqno, + NULL, flags); return 0; } @@ -1199,7 +1201,8 @@ int avc_has_perm_flags(struct selinux_state *state, struct av_decision avd; int rc, rc2; - rc = avc_has_perm_noaudit(state, ssid, tsid, tclass, requested, 0, + rc = avc_has_perm_noaudit(state, ssid, tsid, tclass, requested, + (flags & MAY_NOT_BLOCK) ? AVC_NONBLOCKING : 0, &avd); rc2 = avc_audit(state, ssid, tsid, tclass, requested, &avd, rc, diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index 7ce012d9ec51..9b05f84808d9 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -3196,7 +3196,9 @@ static int selinux_inode_permission(struct inode *inode, int mask) return PTR_ERR(isec); rc = avc_has_perm_noaudit(&selinux_state, - sid, isec->sid, isec->sclass, perms, 0, &avd); + sid, isec->sid, isec->sclass, perms, + (flags & MAY_NOT_BLOCK) ? AVC_NONBLOCKING : 0, + &avd); audited = avc_audit_required(perms, &avd, rc, from_access ? FILE__AUDIT_ACCESS : 0, &denied); diff --git a/security/selinux/include/avc.h b/security/selinux/include/avc.h index ef899bcfd2cb..74ea50977c20 100644 --- a/security/selinux/include/avc.h +++ b/security/selinux/include/avc.h @@ -142,6 +142,7 @@ static inline int avc_audit(struct selinux_state *state, #define AVC_STRICT 1 /* Ignore permissive mode. */ #define AVC_EXTENDED_PERMS 2 /* update extended permissions */ +#define AVC_NONBLOCKING 4 /* non blocking */ int avc_has_perm_noaudit(struct selinux_state *state, u32 ssid, u32 tsid, u16 tclass, u32 requested,
commit 0dc1ba24f7fff6 ("SELINUX: Make selinux cache VFS RCU walks safe") results in no audit messages at all if in permissive mode because the cache is updated during the rcu walk and thus no denial occurs on the subsequent ref walk. Fix this by not updating the cache when performing a non-blocking permission check. This only affects search and symlink read checks during rcu walk. Fixes: 0dc1ba24f7fff6 ("SELINUX: Make selinux cache VFS RCU walks safe") Reported-by: BMK <bmktuwien@gmail.com> Signed-off-by: Stephen Smalley <sds@tycho.nsa.gov> --- security/selinux/avc.c | 9 ++++++--- security/selinux/hooks.c | 4 +++- security/selinux/include/avc.h | 1 + 3 files changed, 10 insertions(+), 4 deletions(-)