diff mbox series

9pfs: local: clarify fchmodat_nofollow() implementation

Message ID 150450989314.1143.5582306195355968158.stgit@bahia.lan
State New
Headers show
Series 9pfs: local: clarify fchmodat_nofollow() implementation | expand

Commit Message

Greg Kurz Sept. 4, 2017, 7:24 a.m. UTC
Since fchmodat(2) on Linux doesn't support AT_SYMLINK_NOFOLLOW, we have to
implement it using workarounds. There are two different ways, depending on
whether the system supports O_PATH or not.

In the case O_PATH is supported, we rely on the behavhior of openat(2)
when passing O_NOFOLLOW | O_PATH and the file is a symbolic link. Even
if openat_file() already adds O_NOFOLLOW to the flags, this patch makes
it explicit that we need both creation flags to obtain the expected
behavior.

This is only cleanup, no functional change.

Signed-off-by: Greg Kurz <groug@kaod.org>
---
 hw/9pfs/9p-local.c |   12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

Comments

Philippe Mathieu-Daudé Sept. 4, 2017, 3:43 p.m. UTC | #1
On 09/04/2017 04:24 AM, Greg Kurz wrote:
> Since fchmodat(2) on Linux doesn't support AT_SYMLINK_NOFOLLOW, we have to
> implement it using workarounds. There are two different ways, depending on
> whether the system supports O_PATH or not.
> 
> In the case O_PATH is supported, we rely on the behavhior of openat(2)
> when passing O_NOFOLLOW | O_PATH and the file is a symbolic link. Even
> if openat_file() already adds O_NOFOLLOW to the flags, this patch makes
> it explicit that we need both creation flags to obtain the expected
> behavior.
> 
> This is only cleanup, no functional change.
> 
> Signed-off-by: Greg Kurz <groug@kaod.org>

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

> ---
>   hw/9pfs/9p-local.c |   12 ++++++++----
>   1 file changed, 8 insertions(+), 4 deletions(-)
> 
> diff --git a/hw/9pfs/9p-local.c b/hw/9pfs/9p-local.c
> index efb0b79a74bf..e51af87309c6 100644
> --- a/hw/9pfs/9p-local.c
> +++ b/hw/9pfs/9p-local.c
> @@ -349,11 +349,11 @@ static int fchmodat_nofollow(int dirfd, const char *name, mode_t mode)
>           return -1;
>       }
>   
> -    /* Access modes are ignored when O_PATH is supported. We try O_RDONLY and
> -     * O_WRONLY for old-systems that don't support O_PATH.
> -     */
> -    fd = openat_file(dirfd, name, O_RDONLY | O_PATH_9P_UTIL, 0);
> +    fd = openat_file(dirfd, name, O_RDONLY | O_PATH_9P_UTIL | O_NOFOLLOW, 0);
>   #if O_PATH_9P_UTIL == 0
> +    /* Fallback for systems that don't support O_PATH: we depend on the file
> +     * being readable or writable.
> +     */
>       if (fd == -1) {
>           /* In case the file is writable-only and isn't a directory. */
>           if (errno == EACCES) {
> @@ -368,6 +368,10 @@ static int fchmodat_nofollow(int dirfd, const char *name, mode_t mode)
>       }
>       ret = fchmod(fd, mode);
>   #else
> +    /* Access modes are ignored when O_PATH is supported. If name is a symbolic
> +     * link, O_PATH | O_NOFOLLOW causes openat(2) to return a file descriptor
> +     * referring to the symbolic link.
> +     */
>       if (fd == -1) {
>           return -1;
>       }
> 
>
diff mbox series

Patch

diff --git a/hw/9pfs/9p-local.c b/hw/9pfs/9p-local.c
index efb0b79a74bf..e51af87309c6 100644
--- a/hw/9pfs/9p-local.c
+++ b/hw/9pfs/9p-local.c
@@ -349,11 +349,11 @@  static int fchmodat_nofollow(int dirfd, const char *name, mode_t mode)
         return -1;
     }
 
-    /* Access modes are ignored when O_PATH is supported. We try O_RDONLY and
-     * O_WRONLY for old-systems that don't support O_PATH.
-     */
-    fd = openat_file(dirfd, name, O_RDONLY | O_PATH_9P_UTIL, 0);
+    fd = openat_file(dirfd, name, O_RDONLY | O_PATH_9P_UTIL | O_NOFOLLOW, 0);
 #if O_PATH_9P_UTIL == 0
+    /* Fallback for systems that don't support O_PATH: we depend on the file
+     * being readable or writable.
+     */
     if (fd == -1) {
         /* In case the file is writable-only and isn't a directory. */
         if (errno == EACCES) {
@@ -368,6 +368,10 @@  static int fchmodat_nofollow(int dirfd, const char *name, mode_t mode)
     }
     ret = fchmod(fd, mode);
 #else
+    /* Access modes are ignored when O_PATH is supported. If name is a symbolic
+     * link, O_PATH | O_NOFOLLOW causes openat(2) to return a file descriptor
+     * referring to the symbolic link.
+     */
     if (fd == -1) {
         return -1;
     }