public inbox for [email protected]
 help / color / mirror / Atom feed
* [PATCH v3 0/2] iter revert problems
@ 2021-08-23 10:18 Pavel Begunkov
  2021-08-23 10:18 ` [PATCH v3 1/2] iov_iter: track truncated size Pavel Begunkov
                   ` (3 more replies)
  0 siblings, 4 replies; 7+ messages in thread
From: Pavel Begunkov @ 2021-08-23 10:18 UTC (permalink / raw)
  To: Jens Axboe, io-uring, Alexander Viro, linux-fsdevel
  Cc: Palash Oswal, Sudip Mukherjee, linux-kernel,
	syzbot+9671693590ef5aad8953, asml.silence

iov_iter_revert() doesn't go well with iov_iter_truncate() in all
cases, see 2/2 for the bug description. As mentioned there the current
problems is because of generic_write_checks(), but there was also a
similar case fixed in 5.12, which should have been triggerable by normal
write(2)/read(2) and others.

It may be better to enforce reexpands as a long term solution, but for
now this patchset is quickier and easier to backport.

v2: don't fail if it was justly fully reverted
v3: use truncated size + reexapand based approach

Pavel Begunkov (2):
  iov_iter: track truncated size
  io_uring: reexpand under-reexpanded iters

 fs/io_uring.c       | 2 ++
 include/linux/uio.h | 6 +++++-
 2 files changed, 7 insertions(+), 1 deletion(-)

-- 
2.32.0


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH v3 1/2] iov_iter: track truncated size
  2021-08-23 10:18 [PATCH v3 0/2] iter revert problems Pavel Begunkov
@ 2021-08-23 10:18 ` Pavel Begunkov
  2021-08-23 10:18 ` [PATCH v3 2/2] io_uring: reexpand under-reexpanded iters Pavel Begunkov
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 7+ messages in thread
From: Pavel Begunkov @ 2021-08-23 10:18 UTC (permalink / raw)
  To: Jens Axboe, io-uring, Alexander Viro, linux-fsdevel
  Cc: Palash Oswal, Sudip Mukherjee, linux-kernel,
	syzbot+9671693590ef5aad8953, asml.silence

Remember how many bytes were truncated and reverted back. Because
not reexpanded iterators don't always work well with reverting, we may
need to know that to reexpand ourselves when needed.

Signed-off-by: Pavel Begunkov <[email protected]>
---
 include/linux/uio.h | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/include/linux/uio.h b/include/linux/uio.h
index 82c3c3e819e0..5265024e8b90 100644
--- a/include/linux/uio.h
+++ b/include/linux/uio.h
@@ -47,6 +47,7 @@ struct iov_iter {
 		};
 		loff_t xarray_start;
 	};
+	size_t truncated;
 };
 
 static inline enum iter_type iov_iter_type(const struct iov_iter *i)
@@ -254,8 +255,10 @@ static inline void iov_iter_truncate(struct iov_iter *i, u64 count)
 	 * conversion in assignement is by definition greater than all
 	 * values of size_t, including old i->count.
 	 */
-	if (i->count > count)
+	if (i->count > count) {
+		i->truncated += i->count - count;
 		i->count = count;
+	}
 }
 
 /*
@@ -264,6 +267,7 @@ static inline void iov_iter_truncate(struct iov_iter *i, u64 count)
  */
 static inline void iov_iter_reexpand(struct iov_iter *i, size_t count)
 {
+	i->truncated -= count - i->count;
 	i->count = count;
 }
 
-- 
2.32.0


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH v3 2/2] io_uring: reexpand under-reexpanded iters
  2021-08-23 10:18 [PATCH v3 0/2] iter revert problems Pavel Begunkov
  2021-08-23 10:18 ` [PATCH v3 1/2] iov_iter: track truncated size Pavel Begunkov
@ 2021-08-23 10:18 ` Pavel Begunkov
  2021-08-23 16:14 ` [PATCH v3 0/2] iter revert problems Jens Axboe
  2021-09-03 20:55 ` Jens Axboe
  3 siblings, 0 replies; 7+ messages in thread
From: Pavel Begunkov @ 2021-08-23 10:18 UTC (permalink / raw)
  To: Jens Axboe, io-uring, Alexander Viro, linux-fsdevel
  Cc: Palash Oswal, Sudip Mukherjee, linux-kernel,
	syzbot+9671693590ef5aad8953, asml.silence

[   74.211232] BUG: KASAN: stack-out-of-bounds in iov_iter_revert+0x809/0x900
[   74.212778] Read of size 8 at addr ffff888025dc78b8 by task
syz-executor.0/828
[   74.214756] CPU: 0 PID: 828 Comm: syz-executor.0 Not tainted
5.14.0-rc3-next-20210730 #1
[   74.216525] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996),
BIOS rel-1.14.0-0-g155821a1990b-prebuilt.qemu.org 04/01/2014
[   74.219033] Call Trace:
[   74.219683]  dump_stack_lvl+0x8b/0xb3
[   74.220706]  print_address_description.constprop.0+0x1f/0x140
[   74.224226]  kasan_report.cold+0x7f/0x11b
[   74.226085]  iov_iter_revert+0x809/0x900
[   74.227960]  io_write+0x57d/0xe40
[   74.232647]  io_issue_sqe+0x4da/0x6a80
[   74.242578]  __io_queue_sqe+0x1ac/0xe60
[   74.245358]  io_submit_sqes+0x3f6e/0x76a0
[   74.248207]  __do_sys_io_uring_enter+0x90c/0x1a20
[   74.257167]  do_syscall_64+0x3b/0x90
[   74.257984]  entry_SYSCALL_64_after_hwframe+0x44/0xae

old_size = iov_iter_count();
...
iov_iter_revert(old_size - iov_iter_count());

If iov_iter_revert() is done base on the initial size as above, and the
iter is truncated and not reexpanded in the middle, it miscalculates
borders causing problems. This trace is due to no one reexpanding after
generic_write_checks().

Now iters store how many bytes has been truncated, so reexpand them to
the initial state right before reverting.

Cc: [email protected]
Reported-by: Palash Oswal <[email protected]>
Reported-by: Sudip Mukherjee <[email protected]>
Reported-and-tested-by: [email protected]
Signed-off-by: Pavel Begunkov <[email protected]>
---
 fs/io_uring.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/fs/io_uring.c b/fs/io_uring.c
index a2e20a6fbfed..b225aff6d586 100644
--- a/fs/io_uring.c
+++ b/fs/io_uring.c
@@ -3323,6 +3323,7 @@ static int io_read(struct io_kiocb *req, unsigned int issue_flags)
 		if (req->flags & REQ_F_NOWAIT)
 			goto done;
 		/* some cases will consume bytes even on error returns */
+		iov_iter_reexpand(iter, iter->count + iter->truncated);
 		iov_iter_revert(iter, io_size - iov_iter_count(iter));
 		ret = 0;
 	} else if (ret == -EIOCBQUEUED) {
@@ -3462,6 +3463,7 @@ static int io_write(struct io_kiocb *req, unsigned int issue_flags)
 	} else {
 copy_iov:
 		/* some cases will consume bytes even on error returns */
+		iov_iter_reexpand(iter, iter->count + iter->truncated);
 		iov_iter_revert(iter, io_size - iov_iter_count(iter));
 		ret = io_setup_async_rw(req, iovec, inline_vecs, iter, false);
 		return ret ?: -EAGAIN;
-- 
2.32.0


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH v3 0/2] iter revert problems
  2021-08-23 10:18 [PATCH v3 0/2] iter revert problems Pavel Begunkov
  2021-08-23 10:18 ` [PATCH v3 1/2] iov_iter: track truncated size Pavel Begunkov
  2021-08-23 10:18 ` [PATCH v3 2/2] io_uring: reexpand under-reexpanded iters Pavel Begunkov
@ 2021-08-23 16:14 ` Jens Axboe
  2021-09-03 20:55 ` Jens Axboe
  3 siblings, 0 replies; 7+ messages in thread
From: Jens Axboe @ 2021-08-23 16:14 UTC (permalink / raw)
  To: Pavel Begunkov, io-uring, Alexander Viro, linux-fsdevel
  Cc: Palash Oswal, Sudip Mukherjee, linux-kernel,
	syzbot+9671693590ef5aad8953

On 8/23/21 4:18 AM, Pavel Begunkov wrote:
> iov_iter_revert() doesn't go well with iov_iter_truncate() in all
> cases, see 2/2 for the bug description. As mentioned there the current
> problems is because of generic_write_checks(), but there was also a
> similar case fixed in 5.12, which should have been triggerable by normal
> write(2)/read(2) and others.
> 
> It may be better to enforce reexpands as a long term solution, but for
> now this patchset is quickier and easier to backport.

Al, given the discussion from this weekend, are you fine with the first
patch? If so, would be great with an ack/review. Or, if you want to
funnel this for 5.14, you can add:

Reviewed-by: Jens Axboe <[email protected]>

-- 
Jens Axboe


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v3 0/2] iter revert problems
  2021-08-23 10:18 [PATCH v3 0/2] iter revert problems Pavel Begunkov
                   ` (2 preceding siblings ...)
  2021-08-23 16:14 ` [PATCH v3 0/2] iter revert problems Jens Axboe
@ 2021-09-03 20:55 ` Jens Axboe
  2021-09-03 21:55   ` Al Viro
  3 siblings, 1 reply; 7+ messages in thread
From: Jens Axboe @ 2021-09-03 20:55 UTC (permalink / raw)
  To: Pavel Begunkov, io-uring, Alexander Viro, linux-fsdevel
  Cc: Palash Oswal, Sudip Mukherjee, linux-kernel,
	syzbot+9671693590ef5aad8953

On 8/23/21 4:18 AM, Pavel Begunkov wrote:
> iov_iter_revert() doesn't go well with iov_iter_truncate() in all
> cases, see 2/2 for the bug description. As mentioned there the current
> problems is because of generic_write_checks(), but there was also a
> similar case fixed in 5.12, which should have been triggerable by normal
> write(2)/read(2) and others.
> 
> It may be better to enforce reexpands as a long term solution, but for
> now this patchset is quickier and easier to backport.
> 
> v2: don't fail if it was justly fully reverted
> v3: use truncated size + reexapand based approach

Al, let's get this upstream. How do you want to handle it? I can take
it through the io_uring tree, or it can go through your tree. I really
don't care which route it takes, but we should get this upstream as
it solves a real problem.

-- 
Jens Axboe


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v3 0/2] iter revert problems
  2021-09-03 20:55 ` Jens Axboe
@ 2021-09-03 21:55   ` Al Viro
  2021-09-04  0:57     ` Jens Axboe
  0 siblings, 1 reply; 7+ messages in thread
From: Al Viro @ 2021-09-03 21:55 UTC (permalink / raw)
  To: Jens Axboe
  Cc: Pavel Begunkov, io-uring, linux-fsdevel, Palash Oswal,
	Sudip Mukherjee, linux-kernel, syzbot+9671693590ef5aad8953

On Fri, Sep 03, 2021 at 02:55:26PM -0600, Jens Axboe wrote:
> On 8/23/21 4:18 AM, Pavel Begunkov wrote:
> > iov_iter_revert() doesn't go well with iov_iter_truncate() in all
> > cases, see 2/2 for the bug description. As mentioned there the current
> > problems is because of generic_write_checks(), but there was also a
> > similar case fixed in 5.12, which should have been triggerable by normal
> > write(2)/read(2) and others.
> > 
> > It may be better to enforce reexpands as a long term solution, but for
> > now this patchset is quickier and easier to backport.
> > 
> > v2: don't fail if it was justly fully reverted
> > v3: use truncated size + reexapand based approach
> 
> Al, let's get this upstream. How do you want to handle it? I can take
> it through the io_uring tree, or it can go through your tree. I really
> don't care which route it takes, but we should get this upstream as
> it solves a real problem.

Grabbed, will test and send a pull request...

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v3 0/2] iter revert problems
  2021-09-03 21:55   ` Al Viro
@ 2021-09-04  0:57     ` Jens Axboe
  0 siblings, 0 replies; 7+ messages in thread
From: Jens Axboe @ 2021-09-04  0:57 UTC (permalink / raw)
  To: Al Viro
  Cc: Pavel Begunkov, io-uring, linux-fsdevel, Palash Oswal,
	Sudip Mukherjee, linux-kernel, syzbot+9671693590ef5aad8953

On 9/3/21 3:55 PM, Al Viro wrote:
> On Fri, Sep 03, 2021 at 02:55:26PM -0600, Jens Axboe wrote:
>> On 8/23/21 4:18 AM, Pavel Begunkov wrote:
>>> iov_iter_revert() doesn't go well with iov_iter_truncate() in all
>>> cases, see 2/2 for the bug description. As mentioned there the current
>>> problems is because of generic_write_checks(), but there was also a
>>> similar case fixed in 5.12, which should have been triggerable by normal
>>> write(2)/read(2) and others.
>>>
>>> It may be better to enforce reexpands as a long term solution, but for
>>> now this patchset is quickier and easier to backport.
>>>
>>> v2: don't fail if it was justly fully reverted
>>> v3: use truncated size + reexapand based approach
>>
>> Al, let's get this upstream. How do you want to handle it? I can take
>> it through the io_uring tree, or it can go through your tree. I really
>> don't care which route it takes, but we should get this upstream as
>> it solves a real problem.
> 
> Grabbed, will test and send a pull request...

Thanks Al! We should mark these for stable as well.

-- 
Jens Axboe


^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2021-09-04  0:57 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2021-08-23 10:18 [PATCH v3 0/2] iter revert problems Pavel Begunkov
2021-08-23 10:18 ` [PATCH v3 1/2] iov_iter: track truncated size Pavel Begunkov
2021-08-23 10:18 ` [PATCH v3 2/2] io_uring: reexpand under-reexpanded iters Pavel Begunkov
2021-08-23 16:14 ` [PATCH v3 0/2] iter revert problems Jens Axboe
2021-09-03 20:55 ` Jens Axboe
2021-09-03 21:55   ` Al Viro
2021-09-04  0:57     ` Jens Axboe

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox