On Mon, Dec 16 2019, quentin.bouget@xxxxxx wrote:
> Hello,
>
> I recently noticed that the syscall open_by_handle_at() automatically
> fails if
> its first argument is a file descriptor opened with O_PATH. I looked at
> the code
> and saw no reason this could not be allowed. Attached to this mail are a
> a reproducer and the patch I came up with.
>
> I am not quite familiar with the kernel's way of processing patches. Any
> pointer
> or advice on this matter is very welcome.
It is probably worth reading through Documentation/process/,
particularly
5.Posting.rst
We generally like the email to be the commit. If you have comments to
make that really don't need to get included in the git commit, they can
go after the "---" after the Signed-off-by.
Also including a reproducer is great, but inline is generally preferred
to attachments as long as it isn't too big.
When making API changes (which this fix does), it is best to Cc
linux-api@xxxxxxxxxxxxxxx.
You might also like to submit a separate patch to linux man-pages
to update the open(2) man page to include open_by_handle_at
in the list of "operations [that] can be performed on the resulting file descriptor"
in the section about O_PATH, but just cc:ing linux-api might be enough,
depending on how busy Michael is.
Thanks,
NeilBrown
>
> Cheers,
> Quentin Bouget
>
> #define _GNU_SOURCE
> #include <errno.h>
> #include <error.h>
> #include <fcntl.h>
> #include <stdlib.h>
> #include <unistd.h>
>
> int
> main()
> {
> struct file_handle *fhandle;
> const char *pathname = "/";
> int mount_fd;
> int mountid;
> int fd;
>
> fhandle = malloc(sizeof(*fhandle) + 128);
> if (fhandle == NULL)
> error(EXIT_FAILURE, errno, "malloc");
> fhandle->handle_bytes = 128;
>
> fd = open(pathname, O_RDONLY | O_PATH | O_NOFOLLOW);
> if (fd < 0)
> error(EXIT_FAILURE, errno, "open");
>
> if (name_to_handle_at(fd, "", fhandle, &mountid, AT_EMPTY_PATH))
> error(EXIT_FAILURE, errno, "name_to_handle_at");
>
> mount_fd = fd;
> fd = open_by_handle_at(mount_fd, fhandle, O_RDONLY | O_PATH | O_NOFOLLOW);
> if (fd < 0)
> error(EXIT_FAILURE, errno, "open_by_handle_at");
>
> if (close(fd))
> error(EXIT_FAILURE, errno, "close");
>
> if (close(mount_fd))
> error(EXIT_FAILURE, errno, "close");
>
> free(fhandle);
>
> return EXIT_SUCCESS;
> }
> From e3717e276444c5711335d398c29beedaf61bac82 Mon Sep 17 00:00:00 2001
> From: Quentin Bouget <quentin.bouget@xxxxxx>
> Date: Thu, 24 Oct 2019 16:54:54 +0200
> Subject: [PATCH] vfs: let open_by_handle_at() use mount_fd opened with O_PATH
>
> The first argument of open_by_handle_at() is `mount_fd':
>
>> a file descriptor for any object (file, directory, etc.) in the
>> mounted filesystem with respect to which `handle' should be
>> interpreted.
>
> This patch allows for this file descriptor to be opened with O_PATH.
>
> Signed-off-by: Quentin Bouget <quentin.bouget@xxxxxx>
> ---
> fs/fhandle.c | 35 +++++++++++++++++++++++------------
> 1 file changed, 23 insertions(+), 12 deletions(-)
>
> diff --git a/fs/fhandle.c b/fs/fhandle.c
> index 01263ffbc..8b67f1b9e 100644
> --- a/fs/fhandle.c
> +++ b/fs/fhandle.c
> @@ -112,22 +112,33 @@ SYSCALL_DEFINE5(name_to_handle_at, int, dfd, const char __user *, name,
> return err;
> }
>
> +static struct vfsmount *get_vfsmount_from_cwd(void)
> +{
> + struct fs_struct *fs = current->fs;
> + struct vfsmount *mnt;
> +
> + spin_lock(&fs->lock);
> + mnt = mntget(fs->pwd.mnt);
> + spin_unlock(&fs->lock);
> +
> + return mnt;
> +}
> +
> static struct vfsmount *get_vfsmount_from_fd(int fd)
> {
> struct vfsmount *mnt;
> + struct path path;
> + int err;
>
> - if (fd == AT_FDCWD) {
> - struct fs_struct *fs = current->fs;
> - spin_lock(&fs->lock);
> - mnt = mntget(fs->pwd.mnt);
> - spin_unlock(&fs->lock);
> - } else {
> - struct fd f = fdget(fd);
> - if (!f.file)
> - return ERR_PTR(-EBADF);
> - mnt = mntget(f.file->f_path.mnt);
> - fdput(f);
> - }
> + if (fd == AT_FDCWD)
> + return get_vfsmount_from_cwd();
> +
> + err = filename_lookup(fd, getname_kernel(""), LOOKUP_EMPTY, &path, NULL);
> + if (err)
> + return ERR_PTR(err);
> +
> + mnt = mntget(path.mnt);
> + path_put(&path);
> return mnt;
> }
>
> --
> 2.18.1
Attachment:
signature.asc
Description: PGP signature
