Hi Pavel,
>>>>> @@ -3977,7 +3977,8 @@ static int io_req_defer_prep(struct io_kiocb *req,
>>>>> mmgrab(current->mm);
>>>>> req->work.mm = current->mm;
>>>>> }
>>>>> - req->work.creds = get_current_cred();
>>>>> + if (!req->work.creds)
>>>>> + req->work.creds = get_current_cred();
>>>>>
>>>>> switch (req->opcode) {
>>>>> case IORING_OP_NOP:
>>>>
>>>> The override_creds(personality_creds) has changed current->cred
>>>> and get_current_cred() will just pick it up as in the default case.
>>>>
>>>> This would make the patch much simpler and allows put_cred() to be
>>>> in io_put_work() instead of __io_req_aux_free() as explained above.
>>>>
>>>
>>> It's one extra get_current_cred(). I'd prefer to find another way to
>>> clean this up.
>>
>> As far as I can see it avoids a get_cred() in the IOSQE_PERSONALITY case
>> and the if (!req->work.creds) for both cases.
>
> Great, that you turned attention to that! override_creds() is already
> grabbing a ref, so it shouldn't call get_cred() there.
> So, that's a bug.
>
> It could be I'm wrong with the statement above, need to recheck all this
> code to be sure.
>
> BTW, io_req_defer_prep() may be called twice for a req, so you will
> reassign it without putting a ref. It's safer to leave NULL checks. At
> least, until I've done reworking and fixing preparation paths.
Ok, but that would be already a bug in
"io_uring/io-wq: don't use static creds/mm assignments"
instead of logically being part of
"io_uring: support using a registered personality for commands"
metze
Attachment:
signature.asc
Description: OpenPGP digital signature
