Re: [PATCH 1/2] libselinux: selinux_restorecon: fix error message on intermediate symlinks

On Fri, Aug 14, 2026 at 9:35 AM Stephen Smalley
<[email protected]> wrote:
>
> selinux_restorecon(3) was changed by commit 67bc978bfaf9 ("libselinux:
> restorecon: revisit pinning files to avoid TOCTOU issues") to avoid
> following intermediate symlinks on the caller-supplied pathname unless
> the SELINUX_RESTORECON_REALPATH flag was set in the restorecon_flags
> parameter. However, because it calls openat(2) with both O_NOFOLLOW
> and O_DIRECTORY, the errno was being set to ENOTDIR and a message was
> logged with the pathname being "Not a directory" even if the terminal
> node of the path was in fact a directory. Fix the error handling logic
> and log message to correctly reflect the intermediate symlink-induced
> failure.
>
> Reported-by: Krzesimir Nowak <[email protected]>
> Fixes: 67bc978bfaf9 ("libselinux: restorecon: revisit pinning files to avoid TOCTOU issues")
> Fixes: https://github.com/SELinuxProject/selinux/issues/540
> Signed-off-by: Stephen Smalley <[email protected]>

For these two patches:
Acked-by: James Carter <[email protected]>

> ---
>  libselinux/src/selinux_restorecon.c | 31 ++++++++++++++++++++++++++---
>  1 file changed, 28 insertions(+), 3 deletions(-)
>
> diff --git a/libselinux/src/selinux_restorecon.c b/libselinux/src/selinux_restorecon.c
> index 49b8ff7e..fbadf367 100644
> --- a/libselinux/src/selinux_restorecon.c
> +++ b/libselinux/src/selinux_restorecon.c
> @@ -1167,11 +1167,30 @@ static int safe_open(const char *path, struct stat *sb)
>                                 nfd = openat(dfd, cur,
>                                              O_PATH | O_NOFOLLOW | O_DIRECTORY |
>                                                      O_CLOEXEC);
> -                               close(dfd);
>                                 if (nfd < 0) {
> +                                       int err = errno;
> +                                       struct stat st;
> +
> +                                       /*
> +                                        * O_NOFOLLOW | O_DIRECTORY on a
> +                                        * symlink yields ENOTDIR; report
> +                                        * ELOOP so the caller can tell an
> +                                        * unfollowed intermediate symlink
> +                                        * from a genuine non-directory, and
> +                                        * so the fallback matches openat2's
> +                                        * RESOLVE_NO_SYMLINKS.
> +                                        */
> +                                       if (err == ENOTDIR &&
> +                                           fstatat(dfd, cur, &st,
> +                                                   AT_SYMLINK_NOFOLLOW) == 0 &&
> +                                           S_ISLNK(st.st_mode))
> +                                               err = ELOOP;
> +                                       close(dfd);
>                                         free(copy);
> +                                       errno = err;
>                                         return -1;
>                                 }
> +                               close(dfd);
>                                 dfd = nfd;
>                                 cur = next;
>                                 continue;
> @@ -1680,8 +1699,14 @@ static int selinux_restorecon_common(const char *pathname_orig,
>                         free(pathname);
>                         return 0;
>                 } else {
> -                       selinux_log(SELINUX_ERROR, "open(%s) failed: %m\n",
> -                                   pathname);
> +                       if (errno == ELOOP)
> +                               selinux_log(
> +                                       SELINUX_ERROR,
> +                                       "%s: an intermediate path component is a symbolic link (not followed)\n",
> +                                       pathname);
> +                       else
> +                               selinux_log(SELINUX_ERROR,
> +                                           "open(%s) failed: %m\n", pathname);
>                         error = -1;
>                         goto cleanup;
>                 }
> --
> 2.55.0
>