[PATCH 1/1] libselinux: do not dereference symlink with statfs in selinux_restorecon

[PATCH 1/1] libselinux: do not dereference symlink with statfs in selinux_restorecon

[Nicolas Iooss]

When selinux_restorecon() is used to relabel symlinks, it performs the
following syscalls (as seen by running strace on restorecond):

    lstat("/root/symlink", {st_mode=S_IFLNK|0777, st_size=6, ...}) = 0
    statfs("/root/symlink", 0x7ffd6bb4d090) = -1 ENOENT (No such file or directory)
    lstat("/root/symlink", {st_mode=S_IFLNK|0777, st_size=6, ...}) = 0
    lgetxattr("/root/symlink", "security.selinux", "sysadm_u:object_r:user_home_t", 255) = 30

The second one triggers a SELinux check for lnk_file:read, as statfs()
dereferences symbolic links. This call to statfs() is only used to find
out whether "restoreconlast" xattr can be ignored, which is always the
case for non-directory files (the first syscall, lstat(), is actually
used to perform this check).

Skip the call to statfs() when setrestoreconlast is already false.

This silences an AVC denial that would otherwise be reported to
audit.log (cf. https://github.com/SELinuxProject/refpolicy/pull/22).

Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
---
 libselinux/src/selinux_restorecon.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libselinux/src/selinux_restorecon.c b/libselinux/src/selinux_restorecon.c
index 3df2d382d50b..42a48f5a1b0b 100644
--- a/libselinux/src/selinux_restorecon.c
+++ b/libselinux/src/selinux_restorecon.c
@@ -881,7 +881,7 @@ int selinux_restorecon(const char *pathname_orig,
 		setrestoreconlast = false;
 
 	/* Ignore restoreconlast on in-memory filesystems */
-	if (statfs(pathname, &sfsb) == 0) {
+	if (setrestoreconlast && statfs(pathname, &sfsb) == 0) {
 		if (sfsb.f_type == RAMFS_MAGIC || sfsb.f_type == TMPFS_MAGIC)
 			setrestoreconlast = false;
 	}
-- 
2.20.1

Re: [PATCH 1/1] libselinux: do not dereference symlink with statfs in selinux_restorecon

[Stephen Smalley]

On 1/16/19 3:57 PM, Nicolas Iooss wrote:
> When selinux_restorecon() is used to relabel symlinks, it performs the
> following syscalls (as seen by running strace on restorecond):
> 
>      lstat("/root/symlink", {st_mode=S_IFLNK|0777, st_size=6, ...}) = 0
>      statfs("/root/symlink", 0x7ffd6bb4d090) = -1 ENOENT (No such file or directory)
>      lstat("/root/symlink", {st_mode=S_IFLNK|0777, st_size=6, ...}) = 0
>      lgetxattr("/root/symlink", "security.selinux", "sysadm_u:object_r:user_home_t", 255) = 30
> 
> The second one triggers a SELinux check for lnk_file:read, as statfs()
> dereferences symbolic links. This call to statfs() is only used to find
> out whether "restoreconlast" xattr can be ignored, which is always the
> case for non-directory files (the first syscall, lstat(), is actually
> used to perform this check).
> 
> Skip the call to statfs() when setrestoreconlast is already false.
> 
> This silences an AVC denial that would otherwise be reported to
> audit.log (cf. https://github.com/SELinuxProject/refpolicy/pull/22).
> 
> Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>

Acked-by:  Stephen Smalley <sds@tycho.nsa.gov>

> ---
>   libselinux/src/selinux_restorecon.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/libselinux/src/selinux_restorecon.c b/libselinux/src/selinux_restorecon.c
> index 3df2d382d50b..42a48f5a1b0b 100644
> --- a/libselinux/src/selinux_restorecon.c
> +++ b/libselinux/src/selinux_restorecon.c
> @@ -881,7 +881,7 @@ int selinux_restorecon(const char *pathname_orig,
>   		setrestoreconlast = false;
>   
>   	/* Ignore restoreconlast on in-memory filesystems */
> -	if (statfs(pathname, &sfsb) == 0) {
> +	if (setrestoreconlast && statfs(pathname, &sfsb) == 0) {
>   		if (sfsb.f_type == RAMFS_MAGIC || sfsb.f_type == TMPFS_MAGIC)
>   			setrestoreconlast = false;
>   	}
> 

Re: [PATCH 1/1] libselinux: do not dereference symlink with statfs in selinux_restorecon

[Stephen Smalley]

On 1/16/19 3:57 PM, Nicolas Iooss wrote:
> When selinux_restorecon() is used to relabel symlinks, it performs the
> following syscalls (as seen by running strace on restorecond):
> 
>      lstat("/root/symlink", {st_mode=S_IFLNK|0777, st_size=6, ...}) = 0
>      statfs("/root/symlink", 0x7ffd6bb4d090) = -1 ENOENT (No such file or directory)
>      lstat("/root/symlink", {st_mode=S_IFLNK|0777, st_size=6, ...}) = 0
>      lgetxattr("/root/symlink", "security.selinux", "sysadm_u:object_r:user_home_t", 255) = 30
> 
> The second one triggers a SELinux check for lnk_file:read, as statfs()
> dereferences symbolic links. This call to statfs() is only used to find
> out whether "restoreconlast" xattr can be ignored, which is always the
> case for non-directory files (the first syscall, lstat(), is actually
> used to perform this check).
> 
> Skip the call to statfs() when setrestoreconlast is already false.
> 
> This silences an AVC denial that would otherwise be reported to
> audit.log (cf. https://github.com/SELinuxProject/refpolicy/pull/22).
> 
> Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>

Thanks, merged via
https://github.com/SELinuxProject/selinux/pull/131

> ---
>   libselinux/src/selinux_restorecon.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/libselinux/src/selinux_restorecon.c b/libselinux/src/selinux_restorecon.c
> index 3df2d382d50b..42a48f5a1b0b 100644
> --- a/libselinux/src/selinux_restorecon.c
> +++ b/libselinux/src/selinux_restorecon.c
> @@ -881,7 +881,7 @@ int selinux_restorecon(const char *pathname_orig,
>   		setrestoreconlast = false;
>   
>   	/* Ignore restoreconlast on in-memory filesystems */
> -	if (statfs(pathname, &sfsb) == 0) {
> +	if (setrestoreconlast && statfs(pathname, &sfsb) == 0) {
>   		if (sfsb.f_type == RAMFS_MAGIC || sfsb.f_type == TMPFS_MAGIC)
>   			setrestoreconlast = false;
>   	}
>