On Wed, Jul 31, 2024 at 02:15:54PM GMT, Adrian Ratiu wrote: > On Wednesday, July 31, 2024 02:18 EEST, Linus Torvalds > <[email protected]> wrote: > > > On Tue, 30 Jul 2024 at 16:09, Jeff Xu <[email protected]> wrote: > > > > > > > + task = get_proc_task(file_inode(file)); > > > > + if (task) { > > > > + ptrace_active = task->ptrace && task->mm == mm > > > > && task->parent == current; > > > > > > Do we need to call "read_lock(&tasklist_lock);" ? > > > see comments in ptrace_check_attach() of kernel/ptrace.c > > > > Well, technically I guess the tasklist_lock should be taken. > > > > Practically speaking, maybe just using READ_ONCE() for these fields > > would really be sufficient. > > > > Yes, it could "race" with the task exiting or just detaching, but the > > logic would basically be "at one point we were tracing it", and since > > this fundamentally a "one-point" situation (with the actual _accesses_ > > happening later anyway), logically that should be sufficient. > > > > I mean - none of this is about "permissions" per se. We actually did > > the proper *permission* check at open() time regardless of all this > > code. This is more of a further tightening of the rules (ie it has > > gone from "are we allowed to ptrace" to "are we actually actively > > ptracing". > > > > I suspect that the main difference between the two situations is > > probably (a) one extra step required and (b) whatever extra system > > call security things people might have which may disable an actual > > ptrace() or whatever.. > > Either approach is fine with me. > > Will leave v4 a few days longer in case others have a stronger > opinion or to gather & address more feedback. > > If no one objects by then, I'll send v5 with READ_ONCE().
I'll just change that directly. No need to resend for that thing.
