| Message ID | 20200527155610.156633-1-jlebon@redhat.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v2] selinux: allow reading labels before policy is loaded | expand |
On Wed, May 27, 2020 at 6:02 PM Jonathan Lebon <jlebon@redhat.com> wrote: > This patch does for `getxattr` what 3e3e24b4204 did for `setxattr`; it > allows querying the current SELinux label on disk before the policy is > loaded. > > One of the motivations described in that commit message also drives this > patch: for Fedora CoreOS (and eventually RHEL CoreOS), we want to be > able to move the root filesystem for example, from xfs to ext4 on RAID, > on first boot, at initrd time.[1] > > Because such an operation works at the filesystem level, we need to be > able to read the SELinux labels first from the original root, and apply > them to the files of the new root. Commit 3e3e24b4204 enabled the second > part of this process; this patch enables the first part. > > [1] https://github.com/coreos/fedora-coreos-tracker/issues/94 > > Signed-off-by: Jonathan Lebon <jlebon@redhat.com> > --- > security/selinux/hooks.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c > index 0b4e32161b7..67ee2cfc25b 100644 > --- a/security/selinux/hooks.c > +++ b/security/selinux/hooks.c > @@ -3334,7 +3334,9 @@ static int selinux_inode_getsecurity(struct inode *inode, const char *name, void > char *context = NULL; > struct inode_security_struct *isec; > > - if (strcmp(name, XATTR_SELINUX_SUFFIX)) > + /* If we're not initialized yet, then we can't validate contexts, > + * so just let vfs_getxattr fall back to using the on-disk xattr. */ > + if (strcmp(name, XATTR_SELINUX_SUFFIX) || !selinux_state.initialized) Just two small notes: 1. We now have a helper for accessing selinux_state.initialized - selinux_initialized() - to ensure proper memory access ordering. Please use that instead of accessing it directly. 2. I'd suggest to make the new condition first in the || expression - it is cheaper than strcmp() so it could save some cycles during early boot. Otherwise LGTM. > return -EOPNOTSUPP; > > /* > -- > 2.25.4 > -- Ondrej Mosnacek Software Engineer, Platform Security - SELinux kernel Red Hat, Inc.
On Wed, May 27, 2020 at 3:04 PM Jonathan Lebon <jlebon@redhat.com> wrote: > > This patch does for `getxattr` what 3e3e24b4204 did for `setxattr`; it Both above and down below you need to fix your commit reference to provide the one-line summary ala commit 3e3e24b42043 ("selinux: allow labeling before policy is loaded"); checkpatch.pl would have caught at least the 2nd instance for you. Probably could change the 2nd reference to avoid need to repeat it. > allows querying the current SELinux label on disk before the policy is > loaded. > > One of the motivations described in that commit message also drives this > patch: for Fedora CoreOS (and eventually RHEL CoreOS), we want to be > able to move the root filesystem for example, from xfs to ext4 on RAID, > on first boot, at initrd time.[1] > > Because such an operation works at the filesystem level, we need to be > able to read the SELinux labels first from the original root, and apply > them to the files of the new root. Commit 3e3e24b4204 enabled the second > part of this process; this patch enables the first part. > > [1] https://github.com/coreos/fedora-coreos-tracker/issues/94 > > Signed-off-by: Jonathan Lebon <jlebon@redhat.com> > --- > security/selinux/hooks.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c > index 0b4e32161b7..67ee2cfc25b 100644 > --- a/security/selinux/hooks.c > +++ b/security/selinux/hooks.c > @@ -3334,7 +3334,9 @@ static int selinux_inode_getsecurity(struct inode *inode, const char *name, void > char *context = NULL; > struct inode_security_struct *isec; > > - if (strcmp(name, XATTR_SELINUX_SUFFIX)) > + /* If we're not initialized yet, then we can't validate contexts, > + * so just let vfs_getxattr fall back to using the on-disk xattr. */ > + if (strcmp(name, XATTR_SELINUX_SUFFIX) || !selinux_state.initialized) > return -EOPNOTSUPP; > > /* > -- > 2.25.4 >
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index 0b4e32161b7..67ee2cfc25b 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -3334,7 +3334,9 @@ static int selinux_inode_getsecurity(struct inode *inode, const char *name, void char *context = NULL; struct inode_security_struct *isec; - if (strcmp(name, XATTR_SELINUX_SUFFIX)) + /* If we're not initialized yet, then we can't validate contexts, + * so just let vfs_getxattr fall back to using the on-disk xattr. */ + if (strcmp(name, XATTR_SELINUX_SUFFIX) || !selinux_state.initialized) return -EOPNOTSUPP; /*
This patch does for `getxattr` what 3e3e24b4204 did for `setxattr`; it allows querying the current SELinux label on disk before the policy is loaded. One of the motivations described in that commit message also drives this patch: for Fedora CoreOS (and eventually RHEL CoreOS), we want to be able to move the root filesystem for example, from xfs to ext4 on RAID, on first boot, at initrd time.[1] Because such an operation works at the filesystem level, we need to be able to read the SELinux labels first from the original root, and apply them to the files of the new root. Commit 3e3e24b4204 enabled the second part of this process; this patch enables the first part. [1] https://github.com/coreos/fedora-coreos-tracker/issues/94 Signed-off-by: Jonathan Lebon <jlebon@redhat.com> --- security/selinux/hooks.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)