Re: [PATCH] io_uring/register: guard compat syscall with CONFIG_COMPAT

[Jens Axboe <axboe@kernel.dk>]

On 1/17/24 8:59 AM, Jeff Moyer wrote:
> Hi, Jens,
> 
> Jens Axboe <axboe@kernel.dk> writes:
> 
>> Add compat.h include to avoid a potential build issue:
>>
>> io_uring/register.c:281:6: error: call to undeclared function 'in_compat_syscall'; ISO C99 and later do not support implicit function declarations [-Werror,-Wimplicit-function-declaration]
>>
>> if (in_compat_syscall()) {
>>     ^
>> 1 warning generated.
>> io_uring/register.c:282:9: error: call to undeclared function 'compat_get_bitmap'; ISO C99 and later do not support implicit function declarations [-Werror,-Wimplicit-function-declaration]
>> ret = compat_get_bitmap(cpumask_bits(new_mask),
>>       ^
>>
>> Fixes: c43203154d8a ("io_uring/register: move io_uring_register(2) related code to register.c")
>> Reported-by: Manu Bretelle <chantra@meta.com>
>> Signed-off-by: Jens Axboe <axboe@kernel.dk>
>>
>> ---
>>
>> diff --git a/io_uring/register.c b/io_uring/register.c
>> index 708dd1d89add..5e62c1208996 100644
>> --- a/io_uring/register.c
>> +++ b/io_uring/register.c
>> @@ -14,6 +14,7 @@
>>  #include <linux/slab.h>
>>  #include <linux/uaccess.h>
>>  #include <linux/nospec.h>
>> +#include <linux/compat.h>
>>  #include <linux/io_uring.h>
>>  #include <linux/io_uring_types.h>
> 
> This makes sense to me, but I wasn't able to reproduce that build error
> after disabling CONFIG_COMPAT.

I couldn't either, but apparently it happened internally with our kdump
config variant.

>> @@ -278,13 +279,14 @@ static __cold int io_register_iowq_aff(struct io_ring_ctx *ctx,
>>  	if (len > cpumask_size())
>>  		len = cpumask_size();
>>  
>> -	if (in_compat_syscall()) {
>> +#ifdef CONFIG_COMPAT
>> +	if (in_compat_syscall())
> 
> I don't think this is needed.
> 
> linux/compat.h:
> ...
> #else /* !CONFIG_COMPAT */
> 
> #define is_compat_task() (0)
> /* Ensure no one redefines in_compat_syscall() under !CONFIG_COMPAT */
> #define in_compat_syscall in_compat_syscall
> static inline bool in_compat_syscall(void) { return false; }
> 
> Isn't the code fine as-is?

It probably is, but this makes it consistent with the other spots we do
compat handling. Hence I'd prefer to keep it like that, and then perhaps
we can prune them all at some point.

Thanks for taking a look!

-- 
Jens Axboe