On Thu, Sep 4, 2025 at 10:51 AM Al Viro viro@zeniv.linux.org.uk wrote:
Looks like the only lockless reader is struct seccomp_filter *f = READ_ONCE(current->seccomp.filter); in seccomp_run_filters(), so unless I've missed something (and "filter" is not a search-friendly name ;-/) we should be fine; that READ_ONCE() is there to handle *other* threads doing stores (with that smp_store_release() in seccomp_sync_threads()). Incidentally, this if (!lock_task_sighand(task, &flags)) return -ESRCH;
f = READ_ONCE(task->seccomp.filter);in proc_pid_seccomp_cache() looks cargo-culted - it's *not* a lockless access, so this READ_ONCE() is confusing.
Kees, is there any reason not to make it a plain int? And what is that READ_ONCE() doing in proc_pid_seccomp_cache(), while we are at it... That's 0d8315dddd28 "seccomp/cache: Report cache data through /proc/pid/seccomp_cache", by YiFei Zhu yifeifz2@illinois.edu, AFAICS. Looks like YiFei Zhu zhuyifei@google.com is the current address [Cc'd]...
I don't remember the context, but looking at the lore [1], AFAICT, it was initially incorrectly lockless, then locking was added; READ_ONCE was a missed leftover.
Can send a patch to remove it.
YiFei Zhu
[1] https://lore.kernel.org/all/CAG48ez0whaSTobwnoJHW+Eyqg5a8H4JCO-KHrgsuNiEg0qb...