Re: [PATCH] exec: fix the racy usage of fs_struct->in_exec

[Mateusz Guzik]

On Mon, Mar 24, 2025 at 7:28 PM Oleg Nesterov <oleg@xxxxxxxxxx> wrote:
> I won't argue with another solution. But this problem is quite old,
> unless I am totally confused this logic was wrong from the very
> beginning when fs->in_exec was introduced by 498052bba55ec.
>
> So to me it would be better to have the trivial fix for stable,
> exactly because it is trivially backportable. Then cleanup/simplify
> this logic on top of it.
>

So I got myself a crap testcase with a CLONE_FS'ed task which can
execve and sanity-checked that suid is indeed not honored as expected.

The most convenient way out of planting a mutex in there does not work
because of the size -- fs_struct is already at 56 bytes and I'm not
going to push it past 64.

However, looks like "wait on bit" machinery can be employed here,
based on what I had seen with inodes (see __wait_on_freeing_inode) and
that should avoid growing the struct, unless I'm grossly misreading
something.

Anyhow, the plan would be to serialize on the bit, synchronized with
the current spin lock. copy_fs would call a helper to wait for it to
clear, would still bump ->users under the spin lock.

This would decouple the handling from cred_mutex and avoid weirdness
like clearing the ->in_exec flag when we never set it.

Should be easy enough and viable for backports, I'll try to hack it up
tomorrow unless the idea is NAKed. The crapper mentioned above will be
used to validate exec vs clone work as expected.

--
Mateusz Guzik <mjguzik gmail.com>