Re: [PATCH 3/6] io_uring: add support for IORING_OP_OPENAT

Stefan Metzmacher
2020-01-08T17:04:11Z

[-- Attachment #1.1: Type: text/plain, Size: 3225 bytes --]

Am 08.01.20 um 17:40 schrieb Jens Axboe:
> On 1/8/20 9:32 AM, Stefan Metzmacher wrote:
>> Am 08.01.20 um 17:20 schrieb Jens Axboe:
>>> On 1/8/20 6:05 AM, Stefan Metzmacher wrote:
>>>> Hi Jens,
>>>>
>>>>> This works just like openat(2), except it can be performed async. For
>>>>> the normal case of a non-blocking path lookup this will complete
>>>>> inline. If we have to do IO to perform the open, it'll be done from
>>>>> async context.
>>>>
>>>> Did you already thought about the credentials being used for the async
>>>> open? The application could call setuid() and similar calls to change
>>>> the credentials of the userspace process/threads. In order for
>>>> applications like samba to use this async openat, it would be required
>>>> to specify the credentials for each open, as we have to multiplex
>>>> requests from multiple user sessions in one process.
>>>>
>>>> This applies to non-fd based syscall. Also for an async connect
>>>> to a unix domain socket.
>>>>
>>>> Do you have comments on this?
>>>
>>> The open works like any of the other commands, it inherits the
>>> credentials that the ring was setup with. Same with the memory context,
>>> file table, etc. There's currently no way to have multiple personalities
>>> within a single ring.
>>
>> Ah, it's user = get_uid(current_user()); and ctx->user = user in
>> io_uring_create(), right?
>
> That's just for the accounting, it's the:
>
> ctx->creds = get_current_cred();

Ok, I just looked at an old checkout.

In kernel-dk-block/for-5.6/io_uring-vfs I see this only used in
the async processing. Does a non-blocking openat also use ctx->creds?

>>> Sounds like you'd like an option for having multiple personalities
>>> within a single ring?
>>
>> I'm not sure anymore, I wasn't aware of the above.
>>
>>> I think it would be better to have a ring per personality instead.
>>
>> We could do that. I guess we could use per user rings for path based
>> operations and a single ring for fd based operations.
>>
>>> One thing we could do to make this more lightweight
>>> is to have rings that are associated, so that we can share a lot of the
>>> backend processing between them.
>>
>> My current idea is to use the ring fd and pass it to our main epoll loop.
>>
>> Can you be more specific about how an api for associated rings could
>> look like?
>
> The API would be the exact same, there would just be some way to
> associate rings when you create them. Probably a new field in struct
> io_uring_params (and an associated flag), which would tell io_uring that
> two separate rings are really the same "user". This would allow io_uring
> to use the same io-wq workqueues, for example, etc.

Ok, this would be just for better performance / better usage of
resources, right?

> This depends on the fact that you can setup the rings with the right
> personalities, that they would be known upfront. From your description,
> I'm not so sure that's the case? If not, then we would indeed need
> something that can pass in the credentials on a per-command basis. Not
> sure what that would look like.

We know the credentials and using a ring per user should be ok.

metze

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]