[v3,0/2] iter revert problems

Message

Pavel Begunkov Aug. 23, 2021, 10:18 a.m. UTC

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(-)

Comments

Jens Axboe Aug. 23, 2021, 4:14 p.m. UTC | #1

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 <[e-mail]>

Jens Axboe Sept. 3, 2021, 8:55 p.m. UTC | #2

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.

Al Viro Sept. 3, 2021, 9:55 p.m. UTC | #3

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...

Jens Axboe Sept. 4, 2021, 12:57 a.m. UTC | #4

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.