On 2/21/25 17:24, Amir Goldstein wrote:
> On Fri, Feb 21, 2025 at 4:36 PM Moinak Bhattacharyya
> <moinakb001@xxxxxxxxx> wrote:
>>
>> Sorry about that. Correctly-formatted patch follows. Should I send out a
>> V2 instead?
>>
>> Add support for opening and closing backing files in the fuse_uring_cmd
>> callback. Store backing_map (for open) and backing_id (for close) in the
>> uring_cmd data.
>> ---
>> fs/fuse/dev_uring.c | 50 +++++++++++++++++++++++++++++++++++++++
>> include/uapi/linux/fuse.h | 6 +++++
>> 2 files changed, 56 insertions(+)
>>
>> diff --git a/fs/fuse/dev_uring.c b/fs/fuse/dev_uring.c
>> index ebd2931b4f2a..df73d9d7e686 100644
>> --- a/fs/fuse/dev_uring.c
>> +++ b/fs/fuse/dev_uring.c
>> @@ -1033,6 +1033,40 @@ fuse_uring_create_ring_ent(struct io_uring_cmd *cmd,
>> return ent;
>> }
>>
>> +/*
>> + * Register new backing file for passthrough, getting backing map from
>> URING_CMD data
>> + */
>> +static int fuse_uring_backing_open(struct io_uring_cmd *cmd,
>> + unsigned int issue_flags, struct fuse_conn *fc)
>> +{
>> + const struct fuse_backing_map *map = io_uring_sqe_cmd(cmd->sqe);
>> + int ret = fuse_backing_open(fc, map);
>> +
>
> I am not that familiar with io_uring, so I need to ask -
> fuse_backing_open() does
> fb->cred = prepare_creds();
> to record server credentials
> what are the credentials that will be recorded in the context of this
> io_uring command?
This is run from the io_uring_enter() syscall - it should not make
a difference to an ioctl, AFAIK. Someone from @io-uring please
correct me if I'm wrong.
>
>
>> + if (ret < 0) {
>> + return ret;
>> + }
>> +
>> + io_uring_cmd_done(cmd, ret, 0, issue_flags);
>> + return 0;
>> +}
>> +
>> +/*
>> + * Remove file from passthrough tracking, getting backing_id from
>> URING_CMD data
>> + */
>> +static int fuse_uring_backing_close(struct io_uring_cmd *cmd,
>> + unsigned int issue_flags, struct fuse_conn *fc)
>> +{
>> + const int *backing_id = io_uring_sqe_cmd(cmd->sqe);
>> + int ret = fuse_backing_close(fc, *backing_id);
>> +
>> + if (ret < 0) {
>> + return ret;
>> + }
>> +
>> + io_uring_cmd_done(cmd, ret, 0, issue_flags);
>> + return 0;
>> +}
>> +
>> /*
>> * Register header and payload buffer with the kernel and puts the
>> * entry as "ready to get fuse requests" on the queue
>> @@ -1144,6 +1178,22 @@ int fuse_uring_cmd(struct io_uring_cmd *cmd,
>> unsigned int issue_flags)
>> return err;
>> }
>> break;
>> + case FUSE_IO_URING_CMD_BACKING_OPEN:
>> + err = fuse_uring_backing_open(cmd, issue_flags, fc);
>> + if (err) {
>> + pr_info_once("FUSE_IO_URING_CMD_BACKING_OPEN failed err=%d\n",
>> + err);
>> + return err;
>> + }
>> + break;
>> + case FUSE_IO_URING_CMD_BACKING_CLOSE:
>> + err = fuse_uring_backing_close(cmd, issue_flags, fc);
>> + if (err) {
>> + pr_info_once("FUSE_IO_URING_CMD_BACKING_CLOSE failed err=%d\n",
>> + err);
>> + return err;
>> + }
>> + break;
>> default:
>> return -EINVAL;
>> }
>> diff --git a/include/uapi/linux/fuse.h b/include/uapi/linux/fuse.h
>> index 5e0eb41d967e..634265da1328 100644
>> --- a/include/uapi/linux/fuse.h
>> +++ b/include/uapi/linux/fuse.h
>> @@ -1264,6 +1264,12 @@ enum fuse_uring_cmd {
>>
>> /* commit fuse request result and fetch next request */
>> FUSE_IO_URING_CMD_COMMIT_AND_FETCH = 2,
>> +
>> + /* add new backing file for passthrough */
>> + FUSE_IO_URING_CMD_BACKING_OPEN = 3,
>> +
>> + /* remove passthrough file by backing_id */
>> + FUSE_IO_URING_CMD_BACKING_CLOSE = 4,
>> };
>>
>
> An anecdote:
> Why are we using FUSE_DEV_IOC_BACKING_OPEN
> and not passing the backing fd directly in OPEN response?
>
> The reason for that was security related - there was a concern that
> an adversary would be able to trick some process into writing some fd
> to /dev/fuse, whereas tricking some proces into doing an ioctl is not
> so realistic.
>
> AFAICT this concern does not exist when OPEN response is via
> io_uring(?), so the backing_id indirection is not strictly needed,
> but for the sake of uniformity with standard fuse protocol,
> I guess we should maintain those commands in io_uring as well.
Yeah, the way it is done is not ideal
fi->backing_id = do_passthrough_open(); /* blocking */
fuse_reply_create()
fill_open()
arg->backing_id = f->backing_id; /* f is fi */
I.e. there are still two operations that depend on each other.
Maybe we could find a way to link the SQEs.
Or maybe easier, if the security concern is gone with IO-URING,
just set FOPEN_PASSTHROUGH for requests over io-uring and then
let the client/kernel side do the passthrough open internally?
Thanks,
Bernd
