On Thu Sep 11, 2025 at 6:15 AM MDT, Amir Goldstein wrote:
> On Wed, Sep 10, 2025 at 11:47 PM Thomas Bertschinger
> <tahbertschinger@xxxxxxxxx> wrote:
>>
>> Pull the code for allocating and copying a struct file_handle from
>> userspace into a helper function get_user_handle() just for this.
>>
>> do_handle_open() is updated to call get_user_handle() prior to calling
>> handle_to_path(), and the latter now takes a kernel pointer as a
>> parameter instead of a __user pointer.
>>
>> This new helper, as well as handle_to_path(), are also exposed in
>> fs/internal.h. In a subsequent commit, io_uring will use these helpers
>> to support open_by_handle_at(2) in io_uring.
>>
>> Signed-off-by: Thomas Bertschinger <tahbertschinger@xxxxxxxxx>
>> ---
>> fs/fhandle.c | 64 +++++++++++++++++++++++++++++----------------------
>> fs/internal.h | 3 +++
>> 2 files changed, 40 insertions(+), 27 deletions(-)
>>
>> diff --git a/fs/fhandle.c b/fs/fhandle.c
>> index 605ad8e7d93d..36e194dd4cb6 100644
>> --- a/fs/fhandle.c
>> +++ b/fs/fhandle.c
>> @@ -330,25 +330,45 @@ static inline int may_decode_fh(struct handle_to_path_ctx *ctx,
>> return 0;
>> }
>>
>> -static int handle_to_path(int mountdirfd, struct file_handle __user *ufh,
>> - struct path *path, unsigned int o_flags)
>> +struct file_handle *get_user_handle(struct file_handle __user *ufh)
>> {
>> - int retval = 0;
>> struct file_handle f_handle;
>> - struct file_handle *handle __free(kfree) = NULL;
>> - struct handle_to_path_ctx ctx = {};
>> - const struct export_operations *eops;
>> + struct file_handle *handle;
>>
>> if (copy_from_user(&f_handle, ufh, sizeof(struct file_handle)))
>> - return -EFAULT;
>> + return ERR_PTR(-EFAULT);
>>
>> if ((f_handle.handle_bytes > MAX_HANDLE_SZ) ||
>> (f_handle.handle_bytes == 0))
>> - return -EINVAL;
>> + return ERR_PTR(-EINVAL);
>>
>> if (f_handle.handle_type < 0 ||
>> FILEID_USER_FLAGS(f_handle.handle_type) & ~FILEID_VALID_USER_FLAGS)
>> - return -EINVAL;
>> + return ERR_PTR(-EINVAL);
>> +
>> + handle = kmalloc(struct_size(handle, f_handle, f_handle.handle_bytes),
>> + GFP_KERNEL);
>> + if (!handle) {
>> + return ERR_PTR(-ENOMEM);
>> + }
>> +
>> + /* copy the full handle */
>> + *handle = f_handle;
>> + if (copy_from_user(&handle->f_handle,
>> + &ufh->f_handle,
>> + f_handle.handle_bytes)) {
>> + return ERR_PTR(-EFAULT);
>> + }
>> +
>> + return handle;
>> +}
>> +
>> +int handle_to_path(int mountdirfd, struct file_handle *handle,
>> + struct path *path, unsigned int o_flags)
>> +{
>> + int retval = 0;
>> + struct handle_to_path_ctx ctx = {};
>> + const struct export_operations *eops;
>>
>> retval = get_path_anchor(mountdirfd, &ctx.root);
>> if (retval)
>> @@ -362,31 +382,16 @@ static int handle_to_path(int mountdirfd, struct file_handle __user *ufh,
>> if (retval)
>> goto out_path;
>>
>> - handle = kmalloc(struct_size(handle, f_handle, f_handle.handle_bytes),
>> - GFP_KERNEL);
>> - if (!handle) {
>> - retval = -ENOMEM;
>> - goto out_path;
>> - }
>> - /* copy the full handle */
>> - *handle = f_handle;
>> - if (copy_from_user(&handle->f_handle,
>> - &ufh->f_handle,
>> - f_handle.handle_bytes)) {
>> - retval = -EFAULT;
>> - goto out_path;
>> - }
>> -
>> /*
>> * If handle was encoded with AT_HANDLE_CONNECTABLE, verify that we
>> * are decoding an fd with connected path, which is accessible from
>> * the mount fd path.
>> */
>> - if (f_handle.handle_type & FILEID_IS_CONNECTABLE) {
>> + if (handle->handle_type & FILEID_IS_CONNECTABLE) {
>> ctx.fh_flags |= EXPORT_FH_CONNECTABLE;
>> ctx.flags |= HANDLE_CHECK_SUBTREE;
>> }
>> - if (f_handle.handle_type & FILEID_IS_DIR)
>> + if (handle->handle_type & FILEID_IS_DIR)
>> ctx.fh_flags |= EXPORT_FH_DIR_ONLY;
>> /* Filesystem code should not be exposed to user flags */
>> handle->handle_type &= ~FILEID_USER_FLAGS_MASK;
>> @@ -400,12 +405,17 @@ static int handle_to_path(int mountdirfd, struct file_handle __user *ufh,
>> static long do_handle_open(int mountdirfd, struct file_handle __user *ufh,
>> int open_flag)
>> {
>> + struct file_handle *handle __free(kfree) = NULL;
>> long retval = 0;
>> struct path path __free(path_put) = {};
>> struct file *file;
>> const struct export_operations *eops;
>>
>> - retval = handle_to_path(mountdirfd, ufh, &path, open_flag);
>> + handle = get_user_handle(ufh);
>> + if (IS_ERR(handle))
>> + return PTR_ERR(handle);
>
> I don't think you can use __free(kfree) for something that can be an ERR_PTR.
>
> Thanks,
> Amir.
It looks like the error pointer is correctly handled?
in include/linux/slab.h:
DEFINE_FREE(kfree, void *, if (!IS_ERR_OR_NULL(_T)) kfree(_T))
