From: Ari Sundholm > Sent: 07 February 2022 12:07 > > The function generic_copy_file_checks() checks that the ends of the > input and output file ranges do not overflow. Unfortunately, there is > an issue with the check itself. > > Due to the integer promotion rules in C, the expressions > (pos_in + count) and (pos_out + count) have an unsigned type because > the count variable has the type uint64_t. Thus, in many cases where we > should detect signed integer overflow to have occurred (and thus one or > more of the ranges being invalid), the expressions will instead be > interpreted as large unsigned integers. This means the check is broken. > > Fix this by explicitly casting the expressions to loff_t. ... > --- > fs/read_write.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/fs/read_write.c b/fs/read_write.c > index 0074afa7ecb3..64166e74adc5 100644 > --- a/fs/read_write.c > +++ b/fs/read_write.c > @@ -1431,7 +1431,8 @@ static int generic_copy_file_checks(struct file *file_in, loff_t pos_in, > return -ETXTBSY; > > /* Ensure offsets don't wrap. */ > - if (pos_in + count < pos_in || pos_out + count < pos_out) > + if ((loff_t)(pos_in + count) < pos_in || > + (loff_t)(pos_out + count) < pos_out) > return -EOVERFLOW; Hard to convince myself that is right. The old code is the standard check for unsigned addition overflow. The new one is just odd. If pos_in is guaranteed to be +ve in a signed variable you can check: count < (1ull << 63) - pos_in since the RHS is then guaranteed not to wrap. David - Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK Registration No: 1397386 (Wales)
