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