* [PATCH v2 0/2] Inode security label invalidation @ 2015-10-04 19:19 Andreas Gruenbacher 2015-10-04 19:19 ` [PATCH v2 1/2] security: Add hook to invalidate inode security labels Andreas Gruenbacher 2015-10-04 19:19 ` [PATCH v2 2/2] gfs2: Invalide security labels of inodes that go invalid Andreas Gruenbacher 0 siblings, 2 replies; 8+ messages in thread From: Andreas Gruenbacher @ 2015-10-04 19:19 UTC (permalink / raw) To: linux-kernel, linux-fsdevel, Alexander Viro, Christoph Hellwig Since the first attempt at making selinux work on top of gfs2 and similar file systems [1] was met with resistance by Christoph, here is another attempt that doesn't require any additional inode operations, but also doesn't guarantee full consistency of inode security labels: in case we cannot reload an invalid label immediately, we keep using the old one. The xattr cleanups that were part of [1] have been split off and posted separately [2]; they seem to be okay. Any thoughts? Thanks, Andreas [1] https://lwn.net/Articles/655294/ [2] https://lkml.org/lkml/2015/10/4/137 Andreas Gruenbacher (2): security: Add hook to invalidate inode security labels gfs2: Invalide security labels of inodes that go invalid fs/gfs2/glops.c | 2 ++ include/linux/lsm_hooks.h | 6 ++++++ include/linux/security.h | 5 +++++ security/security.c | 8 ++++++++ security/selinux/hooks.c | 23 +++++++++++++++++++++-- security/selinux/include/objsec.h | 3 ++- 6 files changed, 44 insertions(+), 3 deletions(-) -- 2.5.0 ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v2 1/2] security: Add hook to invalidate inode security labels 2015-10-04 19:19 [PATCH v2 0/2] Inode security label invalidation Andreas Gruenbacher @ 2015-10-04 19:19 ` Andreas Gruenbacher 2015-10-05 15:08 ` Stephen Smalley 2015-10-05 18:24 ` Casey Schaufler 2015-10-04 19:19 ` [PATCH v2 2/2] gfs2: Invalide security labels of inodes that go invalid Andreas Gruenbacher 1 sibling, 2 replies; 8+ messages in thread From: Andreas Gruenbacher @ 2015-10-04 19:19 UTC (permalink / raw) To: linux-kernel, linux-fsdevel, Alexander Viro, Christoph Hellwig Cc: Paul Moore, Stephen Smalley, Eric Paris, selinux Add a hook to invalidate an inode's security label when the cached information becomes invalid. Implement the new hook in selinux: set a flag when a security label becomes invalid. When hitting a security label which has been marked as invalid in inode_has_perm, try reloading the label. If an inode does not have any dentries attached, we cannot reload its security label because we cannot use the getxattr inode operation. In that case, continue using the old, invalid label until a dentry becomes available. Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com> Cc: Paul Moore <paul@paul-moore.com> Cc: Stephen Smalley <sds@tycho.nsa.gov> Cc: Eric Paris <eparis@parisplace.org> Cc: selinux@tycho.nsa.gov --- include/linux/lsm_hooks.h | 6 ++++++ include/linux/security.h | 5 +++++ security/security.c | 8 ++++++++ security/selinux/hooks.c | 23 +++++++++++++++++++++-- security/selinux/include/objsec.h | 3 ++- 5 files changed, 42 insertions(+), 3 deletions(-) diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h index ec3a6ba..945ae1d 100644 --- a/include/linux/lsm_hooks.h +++ b/include/linux/lsm_hooks.h @@ -1261,6 +1261,10 @@ * audit_rule_init. * @rule contains the allocated rule * + * @inode_invalidate_secctx: + * Notify the security module that it must revalidate the security context + * of an inode. + * * @inode_notifysecctx: * Notify the security module of what the security context of an inode * should be. Initializes the incore security context managed by the @@ -1516,6 +1520,7 @@ union security_list_options { int (*secctx_to_secid)(const char *secdata, u32 seclen, u32 *secid); void (*release_secctx)(char *secdata, u32 seclen); + void (*inode_invalidate_secctx)(struct inode *inode); int (*inode_notifysecctx)(struct inode *inode, void *ctx, u32 ctxlen); int (*inode_setsecctx)(struct dentry *dentry, void *ctx, u32 ctxlen); int (*inode_getsecctx)(struct inode *inode, void **ctx, u32 *ctxlen); @@ -1757,6 +1762,7 @@ struct security_hook_heads { struct list_head secid_to_secctx; struct list_head secctx_to_secid; struct list_head release_secctx; + struct list_head inode_invalidate_secctx; struct list_head inode_notifysecctx; struct list_head inode_setsecctx; struct list_head inode_getsecctx; diff --git a/include/linux/security.h b/include/linux/security.h index 2f4c1f7..9692571 100644 --- a/include/linux/security.h +++ b/include/linux/security.h @@ -353,6 +353,7 @@ int security_secid_to_secctx(u32 secid, char **secdata, u32 *seclen); int security_secctx_to_secid(const char *secdata, u32 seclen, u32 *secid); void security_release_secctx(char *secdata, u32 seclen); +void security_inode_invalidate_secctx(struct inode *inode); int security_inode_notifysecctx(struct inode *inode, void *ctx, u32 ctxlen); int security_inode_setsecctx(struct dentry *dentry, void *ctx, u32 ctxlen); int security_inode_getsecctx(struct inode *inode, void **ctx, u32 *ctxlen); @@ -1093,6 +1094,10 @@ static inline void security_release_secctx(char *secdata, u32 seclen) { } +static inline void security_inode_invalidate_secctx(struct inode *inode) +{ +} + static inline int security_inode_notifysecctx(struct inode *inode, void *ctx, u32 ctxlen) { return -EOPNOTSUPP; diff --git a/security/security.c b/security/security.c index 46f405c..e4371cd 100644 --- a/security/security.c +++ b/security/security.c @@ -1161,6 +1161,12 @@ void security_release_secctx(char *secdata, u32 seclen) } EXPORT_SYMBOL(security_release_secctx); +void security_inode_invalidate_secctx(struct inode *inode) +{ + call_void_hook(inode_invalidate_secctx, inode); +} +EXPORT_SYMBOL(security_inode_invalidate_secctx); + int security_inode_notifysecctx(struct inode *inode, void *ctx, u32 ctxlen) { return call_int_hook(inode_notifysecctx, 0, inode, ctx, ctxlen); @@ -1763,6 +1769,8 @@ struct security_hook_heads security_hook_heads = { LIST_HEAD_INIT(security_hook_heads.secctx_to_secid), .release_secctx = LIST_HEAD_INIT(security_hook_heads.release_secctx), + .inode_invalidate_secctx = + LIST_HEAD_INIT(security_hook_heads.inode_invalidate_secctx), .inode_notifysecctx = LIST_HEAD_INIT(security_hook_heads.inode_notifysecctx), .inode_setsecctx = diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index e4369d8..c5e4ca8 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -1293,11 +1293,11 @@ static int inode_doinit_with_dentry(struct inode *inode, struct dentry *opt_dent unsigned len = 0; int rc = 0; - if (isec->initialized) + if (isec->initialized == 1) goto out; mutex_lock(&isec->lock); - if (isec->initialized) + if (isec->initialized == 1) goto out_unlock; sbsec = inode->i_sb->s_security; @@ -1625,6 +1625,14 @@ static int inode_has_perm(const struct cred *cred, sid = cred_sid(cred); isec = inode->i_security; + /* + * Try reloading the inode security label when it has been invalidated. + * This will fail if no dentry for this inode can be found; in that case, + * we will continue using the old label. + */ + if (isec->initialized == 2) + inode_doinit(inode); + return avc_has_perm(sid, isec->sid, isec->sclass, perms, adp); } @@ -3509,6 +3517,7 @@ static int selinux_file_open(struct file *file, const struct cred *cred) fsec = file->f_security; isec = file_inode(file)->i_security; + /* * Save inode label and policy sequence number * at open-time so that selinux_file_permission @@ -5762,6 +5771,15 @@ static void selinux_release_secctx(char *secdata, u32 seclen) kfree(secdata); } +static void selinux_inode_invalidate_secctx(struct inode *inode) +{ + struct inode_security_struct *isec = inode->i_security; + + mutex_lock(&isec->lock); + isec->initialized = 2; + mutex_unlock(&isec->lock); +} + /* * called with inode->i_mutex locked */ @@ -5993,6 +6011,7 @@ static struct security_hook_list selinux_hooks[] = { LSM_HOOK_INIT(secid_to_secctx, selinux_secid_to_secctx), LSM_HOOK_INIT(secctx_to_secid, selinux_secctx_to_secid), LSM_HOOK_INIT(release_secctx, selinux_release_secctx), + LSM_HOOK_INIT(inode_invalidate_secctx, selinux_inode_invalidate_secctx), LSM_HOOK_INIT(inode_notifysecctx, selinux_inode_notifysecctx), LSM_HOOK_INIT(inode_setsecctx, selinux_inode_setsecctx), LSM_HOOK_INIT(inode_getsecctx, selinux_inode_getsecctx), diff --git a/security/selinux/include/objsec.h b/security/selinux/include/objsec.h index 81fa718..6d1fe19 100644 --- a/security/selinux/include/objsec.h +++ b/security/selinux/include/objsec.h @@ -46,7 +46,8 @@ struct inode_security_struct { u32 task_sid; /* SID of creating task */ u32 sid; /* SID of this object */ u16 sclass; /* security class of this object */ - unsigned char initialized; /* initialization flag */ + unsigned char initialized; /* 0: not initialized, 1: initialized, + 2: invalidated */ struct mutex lock; }; -- 2.5.0 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH v2 1/2] security: Add hook to invalidate inode security labels 2015-10-04 19:19 ` [PATCH v2 1/2] security: Add hook to invalidate inode security labels Andreas Gruenbacher @ 2015-10-05 15:08 ` Stephen Smalley 2015-10-05 21:56 ` Andreas Gruenbacher 2015-10-05 18:24 ` Casey Schaufler 1 sibling, 1 reply; 8+ messages in thread From: Stephen Smalley @ 2015-10-05 15:08 UTC (permalink / raw) To: Andreas Gruenbacher, linux-kernel, linux-fsdevel, Alexander Viro, Christoph Hellwig Cc: Paul Moore, Eric Paris, selinux On 10/04/2015 03:19 PM, Andreas Gruenbacher wrote: > Add a hook to invalidate an inode's security label when the cached > information becomes invalid. > > Implement the new hook in selinux: set a flag when a security label becomes > invalid. When hitting a security label which has been marked as invalid in > inode_has_perm, try reloading the label. > > If an inode does not have any dentries attached, we cannot reload its > security label because we cannot use the getxattr inode operation. In that > case, continue using the old, invalid label until a dentry becomes > available. > > Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com> > Cc: Paul Moore <paul@paul-moore.com> > Cc: Stephen Smalley <sds@tycho.nsa.gov> > Cc: Eric Paris <eparis@parisplace.org> > Cc: selinux@tycho.nsa.gov > --- > include/linux/lsm_hooks.h | 6 ++++++ > include/linux/security.h | 5 +++++ > security/security.c | 8 ++++++++ > security/selinux/hooks.c | 23 +++++++++++++++++++++-- > security/selinux/include/objsec.h | 3 ++- > 5 files changed, 42 insertions(+), 3 deletions(-) > > diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h > index ec3a6ba..945ae1d 100644 > --- a/include/linux/lsm_hooks.h > +++ b/include/linux/lsm_hooks.h > @@ -1261,6 +1261,10 @@ > * audit_rule_init. > * @rule contains the allocated rule > * > + * @inode_invalidate_secctx: > + * Notify the security module that it must revalidate the security context > + * of an inode. > + * > * @inode_notifysecctx: > * Notify the security module of what the security context of an inode > * should be. Initializes the incore security context managed by the > @@ -1516,6 +1520,7 @@ union security_list_options { > int (*secctx_to_secid)(const char *secdata, u32 seclen, u32 *secid); > void (*release_secctx)(char *secdata, u32 seclen); > > + void (*inode_invalidate_secctx)(struct inode *inode); > int (*inode_notifysecctx)(struct inode *inode, void *ctx, u32 ctxlen); > int (*inode_setsecctx)(struct dentry *dentry, void *ctx, u32 ctxlen); > int (*inode_getsecctx)(struct inode *inode, void **ctx, u32 *ctxlen); > @@ -1757,6 +1762,7 @@ struct security_hook_heads { > struct list_head secid_to_secctx; > struct list_head secctx_to_secid; > struct list_head release_secctx; > + struct list_head inode_invalidate_secctx; > struct list_head inode_notifysecctx; > struct list_head inode_setsecctx; > struct list_head inode_getsecctx; > diff --git a/include/linux/security.h b/include/linux/security.h > index 2f4c1f7..9692571 100644 > --- a/include/linux/security.h > +++ b/include/linux/security.h > @@ -353,6 +353,7 @@ int security_secid_to_secctx(u32 secid, char **secdata, u32 *seclen); > int security_secctx_to_secid(const char *secdata, u32 seclen, u32 *secid); > void security_release_secctx(char *secdata, u32 seclen); > > +void security_inode_invalidate_secctx(struct inode *inode); > int security_inode_notifysecctx(struct inode *inode, void *ctx, u32 ctxlen); > int security_inode_setsecctx(struct dentry *dentry, void *ctx, u32 ctxlen); > int security_inode_getsecctx(struct inode *inode, void **ctx, u32 *ctxlen); > @@ -1093,6 +1094,10 @@ static inline void security_release_secctx(char *secdata, u32 seclen) > { > } > > +static inline void security_inode_invalidate_secctx(struct inode *inode) > +{ > +} > + > static inline int security_inode_notifysecctx(struct inode *inode, void *ctx, u32 ctxlen) > { > return -EOPNOTSUPP; > diff --git a/security/security.c b/security/security.c > index 46f405c..e4371cd 100644 > --- a/security/security.c > +++ b/security/security.c > @@ -1161,6 +1161,12 @@ void security_release_secctx(char *secdata, u32 seclen) > } > EXPORT_SYMBOL(security_release_secctx); > > +void security_inode_invalidate_secctx(struct inode *inode) > +{ > + call_void_hook(inode_invalidate_secctx, inode); > +} > +EXPORT_SYMBOL(security_inode_invalidate_secctx); > + > int security_inode_notifysecctx(struct inode *inode, void *ctx, u32 ctxlen) > { > return call_int_hook(inode_notifysecctx, 0, inode, ctx, ctxlen); > @@ -1763,6 +1769,8 @@ struct security_hook_heads security_hook_heads = { > LIST_HEAD_INIT(security_hook_heads.secctx_to_secid), > .release_secctx = > LIST_HEAD_INIT(security_hook_heads.release_secctx), > + .inode_invalidate_secctx = > + LIST_HEAD_INIT(security_hook_heads.inode_invalidate_secctx), > .inode_notifysecctx = > LIST_HEAD_INIT(security_hook_heads.inode_notifysecctx), > .inode_setsecctx = > diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c > index e4369d8..c5e4ca8 100644 > --- a/security/selinux/hooks.c > +++ b/security/selinux/hooks.c > @@ -1293,11 +1293,11 @@ static int inode_doinit_with_dentry(struct inode *inode, struct dentry *opt_dent > unsigned len = 0; > int rc = 0; > > - if (isec->initialized) > + if (isec->initialized == 1) > goto out; > > mutex_lock(&isec->lock); > - if (isec->initialized) > + if (isec->initialized == 1) > goto out_unlock; > > sbsec = inode->i_sb->s_security; > @@ -1625,6 +1625,14 @@ static int inode_has_perm(const struct cred *cred, > sid = cred_sid(cred); > isec = inode->i_security; > > + /* > + * Try reloading the inode security label when it has been invalidated. > + * This will fail if no dentry for this inode can be found; in that case, > + * we will continue using the old label. > + */ > + if (isec->initialized == 2) > + inode_doinit(inode); Not fond of these magic initialized values. Is it always safe to call inode_doinit() from all callers of inode_has_perm()? What about the cases where isec->sid is used without going through inode_has_perm()? > + > return avc_has_perm(sid, isec->sid, isec->sclass, perms, adp); > } > > @@ -3509,6 +3517,7 @@ static int selinux_file_open(struct file *file, const struct cred *cred) > > fsec = file->f_security; > isec = file_inode(file)->i_security; > + > /* > * Save inode label and policy sequence number > * at open-time so that selinux_file_permission > @@ -5762,6 +5771,15 @@ static void selinux_release_secctx(char *secdata, u32 seclen) > kfree(secdata); > } > > +static void selinux_inode_invalidate_secctx(struct inode *inode) > +{ > + struct inode_security_struct *isec = inode->i_security; > + > + mutex_lock(&isec->lock); > + isec->initialized = 2; > + mutex_unlock(&isec->lock); > +} > + > /* > * called with inode->i_mutex locked > */ > @@ -5993,6 +6011,7 @@ static struct security_hook_list selinux_hooks[] = { > LSM_HOOK_INIT(secid_to_secctx, selinux_secid_to_secctx), > LSM_HOOK_INIT(secctx_to_secid, selinux_secctx_to_secid), > LSM_HOOK_INIT(release_secctx, selinux_release_secctx), > + LSM_HOOK_INIT(inode_invalidate_secctx, selinux_inode_invalidate_secctx), > LSM_HOOK_INIT(inode_notifysecctx, selinux_inode_notifysecctx), > LSM_HOOK_INIT(inode_setsecctx, selinux_inode_setsecctx), > LSM_HOOK_INIT(inode_getsecctx, selinux_inode_getsecctx), > diff --git a/security/selinux/include/objsec.h b/security/selinux/include/objsec.h > index 81fa718..6d1fe19 100644 > --- a/security/selinux/include/objsec.h > +++ b/security/selinux/include/objsec.h > @@ -46,7 +46,8 @@ struct inode_security_struct { > u32 task_sid; /* SID of creating task */ > u32 sid; /* SID of this object */ > u16 sclass; /* security class of this object */ > - unsigned char initialized; /* initialization flag */ > + unsigned char initialized; /* 0: not initialized, 1: initialized, > + 2: invalidated */ > struct mutex lock; > }; > > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2 1/2] security: Add hook to invalidate inode security labels 2015-10-05 15:08 ` Stephen Smalley @ 2015-10-05 21:56 ` Andreas Gruenbacher 2015-10-06 21:29 ` Stephen Smalley 0 siblings, 1 reply; 8+ messages in thread From: Andreas Gruenbacher @ 2015-10-05 21:56 UTC (permalink / raw) To: Stephen Smalley Cc: LKML, linux-fsdevel, Alexander Viro, Christoph Hellwig, Paul Moore, Eric Paris, selinux On Mon, Oct 5, 2015 at 5:08 PM, Stephen Smalley <sds@tycho.nsa.gov> wrote: > Not fond of these magic initialized values. That should be a solvable problem. > Is it always safe to call inode_doinit() from all callers of > inode_has_perm()? As long as inode_has_perm is only used in contexts in which a file permission check / acl check would be possible, I don't see why not. > What about the cases where isec->sid is used without going through > inode_has_perm()? inode_has_perm seems to be called frequently and invalid labels seem to be reload quickly, so this change may make SELinux work well enough to be useful on top of gfs2 or similar. More checks would of course be better. The ideal case would be to always reload invalid labels, but that currently won't be possible because we don't have dentries everywhere. I can't tell if this is this good enough to provide a useful level of protection. In any case, without a patch like this, on gfs2 and similar file systems, SELinux currently doesn't work at all. How we can make progress with this problem? Thanks, Andreas ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2 1/2] security: Add hook to invalidate inode security labels 2015-10-05 21:56 ` Andreas Gruenbacher @ 2015-10-06 21:29 ` Stephen Smalley 0 siblings, 0 replies; 8+ messages in thread From: Stephen Smalley @ 2015-10-06 21:29 UTC (permalink / raw) To: Andreas Gruenbacher Cc: LKML, linux-fsdevel, Alexander Viro, Christoph Hellwig, Paul Moore, Eric Paris, selinux On 10/05/2015 05:56 PM, Andreas Gruenbacher wrote: > On Mon, Oct 5, 2015 at 5:08 PM, Stephen Smalley <sds@tycho.nsa.gov> wrote: >> Not fond of these magic initialized values. > > That should be a solvable problem. > >> Is it always safe to call inode_doinit() from all callers of >> inode_has_perm()? > > As long as inode_has_perm is only used in contexts in which a file > permission check / acl check would be possible, I don't see why not. > >> What about the cases where isec->sid is used without going through >> inode_has_perm()? > > inode_has_perm seems to be called frequently and invalid labels seem > to be reload quickly, so this change may make SELinux work well enough > to be useful on top of gfs2 or similar. More checks would of course be > better. The ideal case would be to always reload invalid labels, but > that currently won't be possible because we don't have dentries > everywhere. > > I can't tell if this is this good enough to provide a useful level of > protection. In any case, without a patch like this, on gfs2 and > similar file systems, SELinux currently doesn't work at all. > > How we can make progress with this problem? I think we'd need to wrap all uses of inode->i_security with a helper that applies this test. FWIW, many/most of them seem to have a dentry available, including all callers of inode_has_perm itself, so you could just use inode_doinit_with_dentry() for all of those cases. Maybe just inline inode_has_perm() and get rid of it. Need to deal appropriately with situations like selinux_inode_permission with MAY_NOT_BLOCK. ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2 1/2] security: Add hook to invalidate inode security labels 2015-10-04 19:19 ` [PATCH v2 1/2] security: Add hook to invalidate inode security labels Andreas Gruenbacher 2015-10-05 15:08 ` Stephen Smalley @ 2015-10-05 18:24 ` Casey Schaufler 2015-10-05 18:39 ` Andreas Gruenbacher 1 sibling, 1 reply; 8+ messages in thread From: Casey Schaufler @ 2015-10-05 18:24 UTC (permalink / raw) To: Andreas Gruenbacher, linux-kernel, linux-fsdevel, Alexander Viro, Christoph Hellwig Cc: Paul Moore, Stephen Smalley, Eric Paris, selinux, LSM, Casey Schaufler On 10/4/2015 12:19 PM, Andreas Gruenbacher wrote: > Add a hook to invalidate an inode's security label when the cached > information becomes invalid. Where is this used? If I need to do the same for Smack or any other module, how would I know that it works right? > > Implement the new hook in selinux: set a flag when a security label becomes > invalid. When hitting a security label which has been marked as invalid in > inode_has_perm, try reloading the label. > > If an inode does not have any dentries attached, we cannot reload its > security label because we cannot use the getxattr inode operation. In that > case, continue using the old, invalid label until a dentry becomes > available. > > Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com> > Cc: Paul Moore <paul@paul-moore.com> > Cc: Stephen Smalley <sds@tycho.nsa.gov> > Cc: Eric Paris <eparis@parisplace.org> > Cc: selinux@tycho.nsa.gov > --- > include/linux/lsm_hooks.h | 6 ++++++ > include/linux/security.h | 5 +++++ > security/security.c | 8 ++++++++ > security/selinux/hooks.c | 23 +++++++++++++++++++++-- > security/selinux/include/objsec.h | 3 ++- > 5 files changed, 42 insertions(+), 3 deletions(-) > > diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h > index ec3a6ba..945ae1d 100644 > --- a/include/linux/lsm_hooks.h > +++ b/include/linux/lsm_hooks.h > @@ -1261,6 +1261,10 @@ > * audit_rule_init. > * @rule contains the allocated rule > * > + * @inode_invalidate_secctx: > + * Notify the security module that it must revalidate the security context > + * of an inode. > + * > * @inode_notifysecctx: > * Notify the security module of what the security context of an inode > * should be. Initializes the incore security context managed by the > @@ -1516,6 +1520,7 @@ union security_list_options { > int (*secctx_to_secid)(const char *secdata, u32 seclen, u32 *secid); > void (*release_secctx)(char *secdata, u32 seclen); > > + void (*inode_invalidate_secctx)(struct inode *inode); > int (*inode_notifysecctx)(struct inode *inode, void *ctx, u32 ctxlen); > int (*inode_setsecctx)(struct dentry *dentry, void *ctx, u32 ctxlen); > int (*inode_getsecctx)(struct inode *inode, void **ctx, u32 *ctxlen); > @@ -1757,6 +1762,7 @@ struct security_hook_heads { > struct list_head secid_to_secctx; > struct list_head secctx_to_secid; > struct list_head release_secctx; > + struct list_head inode_invalidate_secctx; > struct list_head inode_notifysecctx; > struct list_head inode_setsecctx; > struct list_head inode_getsecctx; > diff --git a/include/linux/security.h b/include/linux/security.h > index 2f4c1f7..9692571 100644 > --- a/include/linux/security.h > +++ b/include/linux/security.h > @@ -353,6 +353,7 @@ int security_secid_to_secctx(u32 secid, char **secdata, u32 *seclen); > int security_secctx_to_secid(const char *secdata, u32 seclen, u32 *secid); > void security_release_secctx(char *secdata, u32 seclen); > > +void security_inode_invalidate_secctx(struct inode *inode); > int security_inode_notifysecctx(struct inode *inode, void *ctx, u32 ctxlen); > int security_inode_setsecctx(struct dentry *dentry, void *ctx, u32 ctxlen); > int security_inode_getsecctx(struct inode *inode, void **ctx, u32 *ctxlen); > @@ -1093,6 +1094,10 @@ static inline void security_release_secctx(char *secdata, u32 seclen) > { > } > > +static inline void security_inode_invalidate_secctx(struct inode *inode) > +{ > +} > + > static inline int security_inode_notifysecctx(struct inode *inode, void *ctx, u32 ctxlen) > { > return -EOPNOTSUPP; > diff --git a/security/security.c b/security/security.c > index 46f405c..e4371cd 100644 > --- a/security/security.c > +++ b/security/security.c > @@ -1161,6 +1161,12 @@ void security_release_secctx(char *secdata, u32 seclen) > } > EXPORT_SYMBOL(security_release_secctx); > > +void security_inode_invalidate_secctx(struct inode *inode) > +{ > + call_void_hook(inode_invalidate_secctx, inode); > +} > +EXPORT_SYMBOL(security_inode_invalidate_secctx); > + > int security_inode_notifysecctx(struct inode *inode, void *ctx, u32 ctxlen) > { > return call_int_hook(inode_notifysecctx, 0, inode, ctx, ctxlen); > @@ -1763,6 +1769,8 @@ struct security_hook_heads security_hook_heads = { > LIST_HEAD_INIT(security_hook_heads.secctx_to_secid), > .release_secctx = > LIST_HEAD_INIT(security_hook_heads.release_secctx), > + .inode_invalidate_secctx = > + LIST_HEAD_INIT(security_hook_heads.inode_invalidate_secctx), > .inode_notifysecctx = > LIST_HEAD_INIT(security_hook_heads.inode_notifysecctx), > .inode_setsecctx = > diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c > index e4369d8..c5e4ca8 100644 > --- a/security/selinux/hooks.c > +++ b/security/selinux/hooks.c > @@ -1293,11 +1293,11 @@ static int inode_doinit_with_dentry(struct inode *inode, struct dentry *opt_dent > unsigned len = 0; > int rc = 0; > > - if (isec->initialized) > + if (isec->initialized == 1) > goto out; > > mutex_lock(&isec->lock); > - if (isec->initialized) > + if (isec->initialized == 1) > goto out_unlock; > > sbsec = inode->i_sb->s_security; > @@ -1625,6 +1625,14 @@ static int inode_has_perm(const struct cred *cred, > sid = cred_sid(cred); > isec = inode->i_security; > > + /* > + * Try reloading the inode security label when it has been invalidated. > + * This will fail if no dentry for this inode can be found; in that case, > + * we will continue using the old label. > + */ > + if (isec->initialized == 2) > + inode_doinit(inode); > + > return avc_has_perm(sid, isec->sid, isec->sclass, perms, adp); > } > > @@ -3509,6 +3517,7 @@ static int selinux_file_open(struct file *file, const struct cred *cred) > > fsec = file->f_security; > isec = file_inode(file)->i_security; > + > /* > * Save inode label and policy sequence number > * at open-time so that selinux_file_permission > @@ -5762,6 +5771,15 @@ static void selinux_release_secctx(char *secdata, u32 seclen) > kfree(secdata); > } > > +static void selinux_inode_invalidate_secctx(struct inode *inode) > +{ > + struct inode_security_struct *isec = inode->i_security; > + > + mutex_lock(&isec->lock); > + isec->initialized = 2; > + mutex_unlock(&isec->lock); > +} > + > /* > * called with inode->i_mutex locked > */ > @@ -5993,6 +6011,7 @@ static struct security_hook_list selinux_hooks[] = { > LSM_HOOK_INIT(secid_to_secctx, selinux_secid_to_secctx), > LSM_HOOK_INIT(secctx_to_secid, selinux_secctx_to_secid), > LSM_HOOK_INIT(release_secctx, selinux_release_secctx), > + LSM_HOOK_INIT(inode_invalidate_secctx, selinux_inode_invalidate_secctx), > LSM_HOOK_INIT(inode_notifysecctx, selinux_inode_notifysecctx), > LSM_HOOK_INIT(inode_setsecctx, selinux_inode_setsecctx), > LSM_HOOK_INIT(inode_getsecctx, selinux_inode_getsecctx), > diff --git a/security/selinux/include/objsec.h b/security/selinux/include/objsec.h > index 81fa718..6d1fe19 100644 > --- a/security/selinux/include/objsec.h > +++ b/security/selinux/include/objsec.h > @@ -46,7 +46,8 @@ struct inode_security_struct { > u32 task_sid; /* SID of creating task */ > u32 sid; /* SID of this object */ > u16 sclass; /* security class of this object */ > - unsigned char initialized; /* initialization flag */ > + unsigned char initialized; /* 0: not initialized, 1: initialized, > + 2: invalidated */ > struct mutex lock; > }; > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2 1/2] security: Add hook to invalidate inode security labels 2015-10-05 18:24 ` Casey Schaufler @ 2015-10-05 18:39 ` Andreas Gruenbacher 0 siblings, 0 replies; 8+ messages in thread From: Andreas Gruenbacher @ 2015-10-05 18:39 UTC (permalink / raw) To: Casey Schaufler Cc: LKML, linux-fsdevel, Alexander Viro, Christoph Hellwig, Paul Moore, Stephen Smalley, Eric Paris, selinux, LSM On Mon, Oct 5, 2015 at 8:24 PM, Casey Schaufler <casey@schaufler-ca.com> wrote: > On 10/4/2015 12:19 PM, Andreas Gruenbacher wrote: >> Add a hook to invalidate an inode's security label when the cached >> information becomes invalid. > > Where is this used? See the next patch in this patch queue, gfs2. > If I need to do the same for Smack or any other module, how would I know that > it works right? I haven't looked at Smack but if it does the same thing as SELinux, then label updates on one gfs2 nod ein a cluster won't become visible on other nodes without this fix, and with this fix they will. Andreas ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v2 2/2] gfs2: Invalide security labels of inodes that go invalid 2015-10-04 19:19 [PATCH v2 0/2] Inode security label invalidation Andreas Gruenbacher 2015-10-04 19:19 ` [PATCH v2 1/2] security: Add hook to invalidate inode security labels Andreas Gruenbacher @ 2015-10-04 19:19 ` Andreas Gruenbacher 1 sibling, 0 replies; 8+ messages in thread From: Andreas Gruenbacher @ 2015-10-04 19:19 UTC (permalink / raw) To: linux-kernel, linux-fsdevel, Alexander Viro, Christoph Hellwig Cc: Steven Whitehouse, Bob Peterson, cluster-devel When gfs2 releases the glock of an inode, it must invalidate all information cached for that inode, including the page cache and acls. Use the new security_inode_invalidate_secctx hook to also invalidate security labels in that case. These items will be reread from disk when needed after reacquiring the glock. Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com> Cc: Steven Whitehouse <swhiteho@redhat.com> Cc: Bob Peterson <rpeterso@redhat.com> Cc: cluster-devel@redhat.com --- fs/gfs2/glops.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/fs/gfs2/glops.c b/fs/gfs2/glops.c index 1f6c9c3..0833076 100644 --- a/fs/gfs2/glops.c +++ b/fs/gfs2/glops.c @@ -13,6 +13,7 @@ #include <linux/gfs2_ondisk.h> #include <linux/bio.h> #include <linux/posix_acl.h> +#include <linux/security.h> #include "gfs2.h" #include "incore.h" @@ -262,6 +263,7 @@ static void inode_go_inval(struct gfs2_glock *gl, int flags) if (ip) { set_bit(GIF_INVALID, &ip->i_flags); forget_all_cached_acls(&ip->i_inode); + security_inode_invalidate_secctx(&ip->i_inode); gfs2_dir_hash_inval(ip); } } -- 2.5.0 ^ permalink raw reply related [flat|nested] 8+ messages in thread
end of thread, other threads:[~2015-10-06 21:31 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2015-10-04 19:19 [PATCH v2 0/2] Inode security label invalidation Andreas Gruenbacher 2015-10-04 19:19 ` [PATCH v2 1/2] security: Add hook to invalidate inode security labels Andreas Gruenbacher 2015-10-05 15:08 ` Stephen Smalley 2015-10-05 21:56 ` Andreas Gruenbacher 2015-10-06 21:29 ` Stephen Smalley 2015-10-05 18:24 ` Casey Schaufler 2015-10-05 18:39 ` Andreas Gruenbacher 2015-10-04 19:19 ` [PATCH v2 2/2] gfs2: Invalide security labels of inodes that go invalid Andreas Gruenbacher
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).