On 10/3/23 10:30 AM, Jeff Moyer wrote:
> Hi, Jens,
>
> Jens Axboe <axboe@xxxxxxxxx> writes:
>
>> On at least arm32, but presumably any arch with highmem, if the
>> application passes in memory that resides in highmem for the rings,
>> then we should fail that ring creation. We fail it with -EINVAL, which
>> is what kernels that don't support IORING_SETUP_NO_MMAP will do as well.
>>
>> Cc: stable@xxxxxxxxxxxxxxx
>> Fixes: 03d89a2de25b ("io_uring: support for user allocated memory for rings/sqes")
>> Signed-off-by: Jens Axboe <axboe@xxxxxxxxx>
>>
>> ---
>>
>> diff --git a/io_uring/io_uring.c b/io_uring/io_uring.c
>> index 783ed0fff71b..d839a80a6751 100644
>> --- a/io_uring/io_uring.c
>> +++ b/io_uring/io_uring.c
>> @@ -2686,7 +2686,7 @@ static void *__io_uaddr_map(struct page ***pages, unsigned short *npages,
>> {
>> struct page **page_array;
>> unsigned int nr_pages;
>> - int ret;
>> + int ret, i;
>>
>> *npages = 0;
>>
>> @@ -2716,6 +2716,20 @@ static void *__io_uaddr_map(struct page ***pages, unsigned short *npages,
>> */
>> if (page_array[0] != page_array[ret - 1])
>> goto err;
>> +
>> + /*
>> + * Can't support mapping user allocated ring memory on 32-bit archs
>> + * where it could potentially reside in highmem. Just fail those with
>> + * -EINVAL, just like we did on kernels that didn't support this
>> + * feature.
>> + */
>> + for (i = 0; i < nr_pages; i++) {
>> + if (PageHighMem(page_array[i])) {
>> + ret = -EINVAL;
>> + goto err;
>> + }
>> + }
>> +
>
> What do you think about throwing a printk_once in there that explains
> the problem? I'm worried that this will fail somewhat randomly, and it
> may not be apparent to the user why. We should also add documentation,
> of course, and encourage developers to add fallbacks for this case.
For both cases posted, it's rather more advanced use cases. And 32-bit
isn't so prevalent anymore, thankfully. I was going to add to the man
pages explaining this failure case. Not sure it's worth adding a printk
for though.
FWIW, once I got an arm32 vm setup, it fails everytime for me. Not sure
how it'd do on 32-bit x86, similarly or more randomly. But yeah it's
definitely at the mercy of how things are mapped.
--
Jens Axboe
