On 28/03/2021 20:02, Linus Torvalds wrote: > On Sat, Mar 27, 2021 at 6:02 PM Jens Axboe <axboe@xxxxxxxxx> 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
