| Message ID | 20231026021840.GJ800259@ZenIV (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | io_uring: kiocb_done() should *not* trust ->ki_pos if ->{read,write}_iter() failed | expand |
On Thu, Oct 26, 2023 at 03:18:40AM +0100, Al Viro wrote: > [in viro/vfs.git#fixes at the moment] > ->ki_pos value is unreliable in such cases. For an obvious example, > consider O_DSYNC write - we feed the data to page cache and start IO, > then we make sure it's completed. Update of ->ki_pos is dealt with > by the first part; failure in the second ends up with negative value > returned _and_ ->ki_pos left advanced as if sync had been successful. > In the same situation write(2) does not advance the file position > at all. > > Signed-off-by: Al Viro <viro@zeniv.linux.org.uk> > --- Looks good to me, Reviewed-by: Christian Brauner <brauner@kernel.org>
On 10/25/23 8:18 PM, Al Viro wrote: > [in viro/vfs.git#fixes at the moment] > ->ki_pos value is unreliable in such cases. For an obvious example, > consider O_DSYNC write - we feed the data to page cache and start IO, > then we make sure it's completed. Update of ->ki_pos is dealt with > by the first part; failure in the second ends up with negative value > returned _and_ ->ki_pos left advanced as if sync had been successful. > In the same situation write(2) does not advance the file position > at all. Looks good, thanks Al: Reviewed-by: Jens Axboe <axboe@kernel.dk> or let me know if you want me to pick it up.
diff --git a/io_uring/rw.c b/io_uring/rw.c index c8c822fa7980..08d94fb972f0 100644 --- a/io_uring/rw.c +++ b/io_uring/rw.c @@ -339,7 +339,7 @@ static int kiocb_done(struct io_kiocb *req, ssize_t ret, struct io_rw *rw = io_kiocb_to_cmd(req, struct io_rw); unsigned final_ret = io_fixup_rw_res(req, ret); - if (req->flags & REQ_F_CUR_POS) + if (ret >= 0 && req->flags & REQ_F_CUR_POS) req->file->f_pos = rw->kiocb.ki_pos; if (ret >= 0 && (rw->kiocb.ki_complete == io_complete_rw)) { if (!__io_complete_rw_common(req, ret)) {
[in viro/vfs.git#fixes at the moment] ->ki_pos value is unreliable in such cases. For an obvious example, consider O_DSYNC write - we feed the data to page cache and start IO, then we make sure it's completed. Update of ->ki_pos is dealt with by the first part; failure in the second ends up with negative value returned _and_ ->ki_pos left advanced as if sync had been successful. In the same situation write(2) does not advance the file position at all. Signed-off-by: Al Viro <viro@zeniv.linux.org.uk> --- io_uring/rw.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)