* [PATCH] libselinux: fix selinux_restorecon() statfs bug
@ 2020-06-04 20:08 Stephen Smalley
2020-06-04 20:51 ` Jonathan Lebon
2020-06-10 15:56 ` Stephen Smalley
0 siblings, 2 replies; 5+ messages in thread
From: Stephen Smalley @ 2020-06-04 20:08 UTC (permalink / raw)
To: selinux; +Cc: Stephen Smalley
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>
---
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
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] libselinux: fix selinux_restorecon() statfs bug
2020-06-04 20:08 [PATCH] libselinux: fix selinux_restorecon() statfs bug Stephen Smalley
@ 2020-06-04 20:51 ` Jonathan Lebon
2020-06-10 15:56 ` Stephen Smalley
1 sibling, 0 replies; 5+ messages in thread
From: Jonathan Lebon @ 2020-06-04 20:51 UTC (permalink / raw)
To: Stephen Smalley; +Cc: SElinux list
Patch looks good to me, and I can confirm that it fixes the reported
problem upstream.
Tested-by: Jonathan Lebon <jlebon@redhat.com>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] libselinux: fix selinux_restorecon() statfs bug
2020-06-04 20:08 [PATCH] libselinux: fix selinux_restorecon() statfs bug Stephen Smalley
2020-06-04 20:51 ` Jonathan Lebon
@ 2020-06-10 15:56 ` Stephen Smalley
2020-06-10 16:19 ` Petr Lautrbach
1 sibling, 1 reply; 5+ messages in thread
From: Stephen Smalley @ 2020-06-10 15:56 UTC (permalink / raw)
To: SElinux list
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?
> ---
> 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
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] libselinux: fix selinux_restorecon() statfs bug
2020-06-10 15:56 ` Stephen Smalley
@ 2020-06-10 16:19 ` Petr Lautrbach
2020-06-18 19:34 ` Petr Lautrbach
0 siblings, 1 reply; 5+ messages in thread
From: Petr Lautrbach @ 2020-06-10 16:19 UTC (permalink / raw)
To: SElinux list; +Cc: Stephen Smalley
[-- 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 --]
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] libselinux: fix selinux_restorecon() statfs bug
2020-06-10 16:19 ` Petr Lautrbach
@ 2020-06-18 19:34 ` Petr Lautrbach
0 siblings, 0 replies; 5+ messages in thread
From: Petr Lautrbach @ 2020-06-18 19:34 UTC (permalink / raw)
To: SElinux list; +Cc: Stephen Smalley
[-- Attachment #1: Type: text/plain, Size: 3188 bytes --]
On Wed, Jun 10, 2020 at 06:19:22PM +0200, Petr Lautrbach wrote:
> 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>
Applied.
> > > ---
> > > 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 --]
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2020-06-18 19:34 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-06-04 20:08 [PATCH] libselinux: fix selinux_restorecon() statfs bug Stephen Smalley
2020-06-04 20:51 ` Jonathan Lebon
2020-06-10 15:56 ` Stephen Smalley
2020-06-10 16:19 ` Petr Lautrbach
2020-06-18 19:34 ` Petr Lautrbach
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).