Re: [RFC][PATCH] seccomp: Fail immediately if any thread is performing an exec

[Linus Torvalds]

On Mon, Aug 17, 2020 at 12:11 PM Eric W. Biederman
<ebiederm@xxxxxxxxxxxx> wrote:
>
> +static void set_in_execve(bool in_execve)
> +{
> +       struct task_struct *me = current;
> +       spinlock_t *lock = &me->sighand->siglock;
> +
> +       spin_lock_irq(lock);
> +       me->in_execve = in_execve;
> +       spin_unlock_irq(lock);
> +}

No. This is complete voodoo programming. Code like this makes
absolutely zero sense.

Using a lock to serialize a single write is completely bogus. Yes, it
can be required if the field is a bitfield and you want to protect the
other bits in the word, but then you shouldn't be using a bitfield.

It adds zero serialization that a WRITE_ONCE/READ_ONCE pair doesn't
add. The other side will get either the old or the new value
regardless of the locking, so the locking is completely worthless
garbage.

At best it's just wasted CPU time. At worst, it confuses people about
what the locking means and results in bugs down the line. Don't do
things like this.

Locking for reading or writing a single value makes no sense. Locking
is only useful when there is a coherency issue and you have a
*sequence* of writes.

If you have a single value that acts as a flag, use
READ_ONCE/WRITE_ONCE to show that there's no relevant locking. In
fact, better yet, use "smp_store_release()" to set the flag and
"smp_load_acquire()" to read it, and then you get the read/write once
semantics _and_ an ordering between the "I have started doing X,
everything I've done up until this point is now guaranteed to be
visible to whoever reads this value".

           Linus

PS. Yes, there are situations where you want to lock around a single
write because of the serialization it guarantees for things around the
locked code. So you can have valid "lock-write-unlock" sequences. But
then you'd better have a BIG HONKING COMMENT about what the hell
you're really serializing, because it's not the single write, it's
something bigger.