[PATCH net 2/3] rxrpc: Fix DATA decrypt vs splice() by copying data to buffer in recvmsg
David Laight
david.laight.linux at gmail.com
Tue May 12 14:36:09 PDT 2026
On Tue, 12 May 2026 17:52:03 +0100
David Howells <dhowells at redhat.com> wrote:
> David Laight <david.laight.linux at gmail.com> wrote:
...
> > > + size_t size = umin(round_up(sp->len, 32), 2048);
> >
> > Doesn't min() work?
>
> Actually, it should be umax() as I want the largest of the values (as Jeff
> pointed out). I prefer using umin/umax for values that are known to be
> unsigned as you don't get casting errors (see the number of places we end up
> using min/max_t(<unsigned-type>, ...) when we should use umin/umax() instead)
> and the compiler may generate better code as we've told it that it doesn't
> have to worry about negatives.
umin() and umax() are better than min_t() and max_t() (which is why I added
them); but you lose the compile-time check in min() and max() that rejects
comparisons where one side is unsigned and the compiler doesn't know that the
other is always non-negative.
Basically if you compare a signed 32bit value and an unsigned 64bit one
with umin() the 32bit one is zero-extended to 64 bits.
OTOH min_t(u64) will sign-extend the 32bit value and then treat it as unsigned.
In both cases the onus is on the programmer to ensure the 32bit value isn't
negative.
For valid non-negative values the result is the same.
Zero-extending is usually free, sign-extending is particularly horrid on 32bit.
But it is better to use min() or max().
The compile-time tests will reject any cases where the integer promotion
rules could convert a negative value to a large positive one.
Note that the types no longer have to match.
Code like this is (usually) ok:
unsigned int blk_len = ...;
int rval = fun(...);
while (rval > 0) {
u32 len = min(rval, blk_len);
// process len bytes;
rval -= len;
}
even though the types passed to min() differ in signedness the compiler's
value tracking means it knows that rval can never become a large unsigned
value - and min() uses that to allow it all through.
-- David
>
> > That doesn't look right.
> > If sp->len is bigger than 2048 the you keep allocating a new buffer
> > and the call below overruns the allocated buffer.
>
> Yep - see the aforementioned umax comment.
>
> David
>
More information about the linux-afs
mailing list