Re: [PATCH] io_uring/rw: always clear ->bytes_done on io_async_rw setup

[Gabriel Krisman Bertazi <krisman@suse.de>]

Jens Axboe <axboe@kernel.dk> writes:

> A previous commit mistakenly moved the clearing of the in-progress byte
> count into the section that's dependent on having a cached iovec or not,
> but it should be cleared for any IO. If not, then extra bytes may be
> added at IO completion time, causing potentially weird behavior like
> over-reporting the amount of IO done.

Hi Jens,

Sorry for the delay.  I went completely offline during the christmas
week.

Did this solve the sysbot report?  I'm failing to understand how it can
happen.  This could only be hit if the allocation returned a cached
object that doesn't have a free_iov, since any newly kmalloc'ed object
will have this field cleaned inside the io_rw_async_data_init callback.
But I don't understand where we can cache the rw object without having a
valid free_iov - it didn't seem possible to me before or now.

the iov is freed only by io_rw_iovec_free, which is called from

(1) io_rw_recycle, in the case where we don't cache.  we drop also
drop the CLEANUP flag, so we will just call kfree inside io_clean_op later.
(2) io_readv_writev_cleanup: where we also don't cache, since we are inside
    the io_clean_op, we'll just hit the kfree(req->async_data), and
(3) io_rw_cache_free:  where we are emptying the cache to shut down.

> diff --git a/io_uring/rw.c b/io_uring/rw.c
> index 75f70935ccf4..ca1b19d3d142 100644
> --- a/io_uring/rw.c
> +++ b/io_uring/rw.c
> @@ -228,8 +228,8 @@ static int io_rw_alloc_async(struct io_kiocb *req)
>  		kasan_mempool_unpoison_object(rw->free_iovec,
>  					      rw->free_iov_nr * sizeof(struct iovec));
>  		req->flags |= REQ_F_NEED_CLEANUP;
> -		rw->bytes_done = 0;
>  	}
> +	rw->bytes_done = 0;
>  	return 0;
>  }

-- 
Gabriel Krisman Bertazi