Re: [PATCH 1/2] io_uring: fix invalid flags for io_put_kbuf()

[Jens Axboe <axboe@kernel.dk>]

On 3/25/22 7:00 AM, Pavel Begunkov wrote:
> io_req_complete_failed() doesn't require callers to hold ->uring_lock,
> use IO_URING_F_UNLOCKED version of io_put_kbuf(). The only affected
> place is the fail path of io_apoll_task_func(). Also add a lockdep
> annotation to catch such bugs in the future.
> 
> Fixes: 3b2b78a8eb7cc ("io_uring: extend provided buf return to fails")
> Signed-off-by: Pavel Begunkov <asml.silence@gmail.com>
> ---
>  fs/io_uring.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/io_uring.c b/fs/io_uring.c
> index 862401d23a5a..c83a650ca5fa 100644
> --- a/fs/io_uring.c
> +++ b/fs/io_uring.c
> @@ -1388,6 +1388,8 @@ static inline unsigned int io_put_kbuf(struct io_kiocb *req,
>  		cflags = __io_put_kbuf(req, &ctx->io_buffers_comp);
>  		spin_unlock(&ctx->completion_lock);
>  	} else {
> +		lockdep_assert_held(&req->ctx->uring_lock);
> +
>  		cflags = __io_put_kbuf(req, &req->ctx->io_buffers_cache);
>  	}
>  
> @@ -2182,7 +2184,7 @@ static inline void io_req_complete(struct io_kiocb *req, s32 res)
>  static void io_req_complete_failed(struct io_kiocb *req, s32 res)
>  {
>  	req_set_fail(req);
> -	io_req_complete_post(req, res, io_put_kbuf(req, 0));
> +	io_req_complete_post(req, res, io_put_kbuf(req, IO_URING_F_UNLOCKED));
>  }

That took me a bit to grok, but it's because we don't use the flag here
as "ok we need to grab that same lock, rather we use it for "should we
use something else" which makes it safe. We should probably use a bool
for this case instead rather than abuse issue_flags, for later.

-- 
Jens Axboe