Re: [PATCH v5 3/7] futex: Use explicit sizes for compat_exit_robust_list

[Thomas Gleixner <tglx@linutronix.de>]

On Thu, Jun 26 2025 at 14:11, André Almeida wrote:

$subject lacks a () function notation ....

> There are two functions for handling robust lists during the task

during a tasks exit

> exit: exit_robust_list() and compat_exit_robust_list(). The first one
> handles either 64bit or 32bit lists, depending if it's a 64bit or 32bit
> kernel. The compat_exit_robust_list() only exists in 64bit kernels that

s/The//

> supports 32bit syscalls, and handles 32bit lists.

32-bit 64-bit all over the place

> For the new syscall set_robust_list2(), 64bit kernels need to be able to
> handle 32bit lists despite having or not support for 32bit syscalls, so
> make compat_exit_robust_list() exist regardless of compat_ config.

What new syscall and what are the requirements here? You really want to
add some rationale and background here.

> Also, use explicitly sizing, otherwise in a 32bit kernel both
> exit_robust_list() and compat_exit_robust_list() would be the exactly
> same function, with none of them dealing with 64bit robust lists.

Explicit sizing of what? The changelog should give information which
allows me to verify the implementation and not some blurb which makes me
to oracle the meaning of the changelog out of the actual implementation.

What is the actual gist of this patch? The subject says:

     Use explicit sizes for compat_exit_robust_list

Now you say 'Also,' which means aside of the above actual statement to
make compat_exit_robust_list() unconditional this is now a side effect
or what?

The subject line is misleading because the real purpose of this patch is
to make compat_exit_robust_list() unconditionally available independent
of bitness.

Now the obvious question is why this patch isn't split into two pieces:

    1) The patch matching the above subject line and does the
       struct/argument rename

    2) A subsequent patch which makes the function unconditionally
       available

That's not done because obfuscating changes makes everyones life easier,
right?

> +++ b/include/linux/compat.h
> @@ -385,16 +385,6 @@ struct compat_ifconf {
>  	compat_caddr_t  ifcbuf;
>  };
>  
> -struct compat_robust_list {
> -	compat_uptr_t			next;
> -};
> -
> -struct compat_robust_list_head {
> -	struct compat_robust_list	list;
> -	compat_long_t			futex_offset;
> -	compat_uptr_t			list_op_pending;
> -};
> -
>  #ifdef CONFIG_COMPAT_OLD_SIGACTION
>  struct compat_old_sigaction {
>  	compat_uptr_t			sa_handler;
> @@ -672,7 +662,7 @@ asmlinkage long compat_sys_waitid(int, compat_pid_t,
>  		struct compat_siginfo __user *, int,
>  		struct compat_rusage __user *);
>  asmlinkage long
> -compat_sys_set_robust_list(struct compat_robust_list_head __user *head,
> +compat_sys_set_robust_list(struct robust_list_head32 __user *head,
>  			   compat_size_t len);

How does this even survive a full kernel build without a forward
declaration of struct robust_list_head32?

Not everything which includes compat.h includes futex.h first. There is
a reason why the structs were define here. Sure you can move them, but
not without a forward declaration.

Thanks,

        tglx