[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <20240731-gefilde-rehabilitieren-75f77dbdd79f@brauner>
Date: Wed, 31 Jul 2024 16:39:04 +0200
From: Christian Brauner <brauner@...nel.org>
To: Adrian Ratiu <adrian.ratiu@...labora.com>
Cc: Linus Torvalds <torvalds@...ux-foundation.org>,
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>
Subject: Re: [PATCH v4] proc: add config & param to block forcing mem writes
On Wed, Jul 31, 2024 at 02:15:54PM GMT, Adrian Ratiu wrote:
> 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().
I'll just change that directly. No need to resend for that thing.
Powered by blists - more mailing lists