Re: [PATCH] libselinux: fix selinux_restorecon() statfs bug

[Petr Lautrbach <plautrba@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 2979 bytes --]

On Wed, Jun 10, 2020 at 11:56:28AM -0400, Stephen Smalley wrote:
> On Thu, Jun 4, 2020 at 4:08 PM Stephen Smalley
> <stephen.smalley.work@gmail.com> wrote:
> >
> > As reported in https://github.com/SELinuxProject/selinux/issues/248,
> > setfiles -r (rootpath) fails when the alternate root contains a symlink
> > that is correct relative to the alternate root but not in the current root.
> > This is a regression introduced by commit e016502c0a26 ("libselinux: Save
> > digest of all partial matches for directory").  Do not call statfs(2) here
> > if acting on a symbolic link.  Unfortunately there is no lstatfs() call.
> > Ensure that we initialize the statfs buffer always.  If the supplied
> > file is a symlink, then we don't need to worry about the later tests of
> > filesystem type because we wouldn't be setting the digest anyway and
> > we are not performing a full sysfs relabel.  While here, fix the earlier
> > test for a directory to use the correct test.
> >
> > Reproducer:
> > $ mkdir /root/my-chroot && echo foo > /root/my-chroot/link-target && ln -s /link-target /root/my-chroot/symlink
> > $ echo "/root/my-chroot/symlink" | setfiles -vFi -r /root/my-chroot -f - /etc/selinux/targeted/contexts/files/file_contexts
> >
> > Before:
> > setfiles: statfs(/root/my-chroot/symlink) failed: No such file or directory
> >
> > After:
> > Relabeled /root/my-chroot/symlink from unconfined_u:object_r:admin_home_t:s0 to system_u:object_r:default_t:s0
> >
> > Fixes: https://github.com/SELinuxProject/selinux/issues/248
> > Fixes: e016502c0a26 ("libselinux: Save digest of all partial matches for directory")
> > Signed-off-by: Stephen Smalley <stephen.smalley.work@gmail.com>
> 
> Anyone want to ack or object to this patch?

Acked-by: Petr Lautrbach <plautrba@redhat.com>

> > ---
> >  libselinux/src/selinux_restorecon.c | 5 +++--
> >  1 file changed, 3 insertions(+), 2 deletions(-)
> >
> > diff --git a/libselinux/src/selinux_restorecon.c b/libselinux/src/selinux_restorecon.c
> > index 91dfeb66..d1ce830c 100644
> > --- a/libselinux/src/selinux_restorecon.c
> > +++ b/libselinux/src/selinux_restorecon.c
> > @@ -938,7 +938,7 @@ int selinux_restorecon(const char *pathname_orig,
> >         }
> >
> >         /* Skip digest if not a directory */
> > -       if ((sb.st_mode & S_IFDIR) != S_IFDIR)
> > +       if (!S_ISDIR(sb.st_mode))
> >                 setrestorecondigest = false;
> >
> >         if (!flags.recurse) {
> > @@ -952,7 +952,8 @@ int selinux_restorecon(const char *pathname_orig,
> >         }
> >
> >         /* Obtain fs type */
> > -       if (statfs(pathname, &sfsb) < 0) {
> > +       memset(&sfsb, 0, sizeof sfsb);
> > +       if (!S_ISLNK(sb.st_mode) && statfs(pathname, &sfsb) < 0) {
> >                 selinux_log(SELINUX_ERROR,
> >                             "statfs(%s) failed: %s\n",
> >                             pathname, strerror(errno));
> > --
> > 2.23.3
> >
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]