Re: [GIT PULL] io_uring fixes for 5.12-rc5

[Pavel Begunkov <asml.silence@gmail.com>]

On 28/03/2021 20:02, Linus Torvalds wrote:
> On Sat, Mar 27, 2021 at 6:02 PM Jens Axboe <axboe@kernel.dk> wrote:
>>
>> - Fix sign extension issue for IORING_OP_PROVIDE_BUFFERS
> 
> I don't think this fixes anything.
> 
> It may change the sign bit, but as far as I can tell, doesn't actually
> fix anything at all. You're multiplying a 16-bit value with a signed
> 32-bit one. The cast to "unsigned long" makes sure it's done as an
> unsigned multiply, but doesn't change anything funcamental.
> 
>  - "p->len" is an explictly signed type (__s32). Don't ask me why.
> 
>  - the size calculation takes that signed value, turns it into an
> "unsigned long" (which sign-extends it), and then does an unsigned
> multiply of that nonsensical value
> 
>  - that can overflow both in 64-bit and 32-bit (since the 32-bit
> signed value has been made an extremely large "unsigned long"
> 
> So there is absolutely nothing "right" about the typing there. Not
> before, and not after. The whole cast is entirely meaningless, and
> doesn't seem to fix anything. It is basically a random change.
> 
> If you want that calculation to make sense, you need to
> 
>  (a) disallow the insane case of signed "len". Most certainly not
> sign-extend it to a large unsigned value.
> 
>  (b) actually make sure there is no overflow
> 
> because adding a random cast does neither of those things.

Agree, thanks for pointing out. Will get it fixed.


-- 
Pavel Begunkov