On 9/3/25 4:44 PM, Alexei Starovoitov wrote:
On Wed, Sep 3, 2025 at 1:52 PM Tom Hromatka <tom.hromatka@xxxxxxxxxx> wrote:
On 9/3/25 2:45 PM, Alexei Starovoitov wrote:
On Wed, Sep 3, 2025 at 1:38 PM Tom Hromatka <tom.hromatka@xxxxxxxxxx> wrote:
+
+ spin_lock_irq(¤t->sighand->siglock);
+ spin_lock_irq(&task->sighand->siglock);
+
+ if (atomic_read(&task->seccomp.filter_count) == 0) {
+ spin_unlock_irq(&task->sighand->siglock);
+ spin_unlock_irq(¤t->sighand->siglock);
did you copy this pattern from somewhere ?
It's obviously buggy.
I tried to mimic the logic in copy_seccomp() in kernel/fork.c,
but as you point out, I probably messed it up :).
Do you have recommendations for a better design pattern?
Several things look wrong here.
Thanks so much for weighing in.
Double _irq() is one obvious bug.
Makes sense. I'll look through the kernel code to see if I can
find another place where two task structs are being locked at
the same time. I've never had to do that before.
Grabbing spin_lock to do atomic_read() is another oddity.
That would indeed be strange.
The spin_lock is needed to ensure that the source and target's
seccomp filters don't change out from underneath me. Once I
read the target's seccomp filter count, I don't want another
thread to make any changes before I've updated the target's
filters.
Thanks!
Tom
