linux-kernel - Re: [PATCH v4] proc: add config & param to block forcing mem writes

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <3ea8c0-66aa3900-3-2bfd8e00@3451942>
Date: Wed, 31 Jul 2024 14:15:54 +0100
From: "Adrian Ratiu" <adrian.ratiu@...labora.com>
To: "Linus Torvalds" <torvalds@...ux-foundation.org>
Cc: "Jeff Xu" <jeffxu@...gle.com>, linux-fsdevel@...r.kernel.org, linux-security-module@...r.kernel.org, linux-kernel@...r.kernel.org, linux-hardening@...r.kernel.org, kernel@...labora.com, gbiv@...gle.com, inglorion@...gle.com, ajordanr@...gle.com, "Doug Anderson" <dianders@...omium.org>, "Jann Horn" <jannh@...gle.com>, "Kees Cook" <kees@...nel.org>, "Ard Biesheuvel" <ardb@...nel.org>, "Christian Brauner" <brauner@...nel.org>
Subject: Re: [PATCH v4] proc: add config & param to 
 block forcing mem writes

On Wednesday, July 31, 2024 02:18 EEST, Linus Torvalds <torvalds@...ux-foundation.org> wrote:

> On Tue, 30 Jul 2024 at 16:09, Jeff Xu <jeffxu@...gle.com> 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().