On 24/03/2021 14:55, Jens Axboe wrote:
> On 3/24/21 8:40 AM, Pavel Begunkov wrote:
>> We are safe with overflows in io_sqe_buffer_register() because it will
>> only yield allocation failure, but it's nicer to check explicitly.
>
> Right, either that or fault when mapping. So nothing serious here, but
> would be nice to clean up though and just explicitly make it return
> -EOVERFLOW when that is the case.
>
>> @@ -8306,6 +8306,8 @@ static int io_buffers_map_alloc(struct io_ring_ctx *ctx, unsigned int nr_args)
>>
>> static int io_buffer_validate(struct iovec *iov)
>> {
>> + u64 tmp, acct_len = iov->iov_len + (PAGE_SIZE - 1);
>> +
>
> No need for those parens.
Right, but purely formally underflows are UB.
>> /*
>> * Don't impose further limits on the size and buffer
>> * constraints here, we'll -EINVAL later when IO is
>> @@ -8318,6 +8320,9 @@ static int io_buffer_validate(struct iovec *iov)
>> if (iov->iov_len > SZ_1G)
>> return -EFAULT;
>>
>> + if (check_add_overflow((u64)iov->iov_base, acct_len, &tmp))
>> + return -EOVERFLOW;
>> +
>
> Is this right for 32-bit?
Nope, better be unsigned long.
btw, imu and io_import_fixed does it through u64, that's confusing.
--
Pavel Begunkov
