Message-ID: <fofcw327c7cqa6vxlfiat6mrrjx6rgnhe6ylhqbfw4qngamhjg@6wkh6rcosqfp>
Date: Thu, 25 Apr 2024 19:02:12 -0400
From: Kent Overstreet <kent.overstreet@...ux.dev>
To: Kees Cook <keescook@...omium.org>
Cc: Matthew Wilcox <willy@...radead.org>, 
	Suren Baghdasaryan <surenb@...gle.com>, Andrew Morton <akpm@...ux-foundation.org>, linux-mm@...ck.org, 
	linux-kernel@...r.kernel.org, linux-hardening@...r.kernel.org
Subject: Re: [PATCH] alloc_tag: Tighten file permissions on /proc/allocinfo

On Thu, Apr 25, 2024 at 03:42:30PM -0700, Kees Cook wrote:
> On Thu, Apr 25, 2024 at 05:04:47PM -0400, Kent Overstreet wrote:
> > On Thu, Apr 25, 2024 at 09:51:56PM +0100, Matthew Wilcox wrote:
> > > On Thu, Apr 25, 2024 at 04:45:51PM -0400, Kent Overstreet wrote:
> > > > On Thu, Apr 25, 2024 at 01:08:50PM -0700, Kees Cook wrote:
> > > > > The /proc/allocinfo file exposes a tremendous about of information about
> > > > > kernel build details, memory allocations (obviously), and potentially
> > > > > even image layout (due to ordering). As this is intended to be consumed
> > > > > by system owners (like /proc/slabinfo), use the same file permissions as
> > > > > there: 0400.
> > > > 
> > > > Err...
> > > > 
> > > > The side effect of locking down more and more reporting interfaces is
> > > > that programs that consume those interfaces now have to run as root.
> > > 
> > > sudo cat /proc/allocinfo | analyse-that-fie
> > 
> > Even that is still an annoyance, but I'm thinking more about a future
> > daemon to collect this every n seconds - that really shouldn't need to
> > be root.
> 
> Open it once and rewind? But regardless, see the end about changing
> ownership/perms/etc.
> 
> > And the "lock everything down" approach really feels like paranoia gone
> > too far - what's next, /proc/cpuinfo? Do we really want to go the
> > Windows approach of UAC pop ups for everything? I'd rather be going the
> > opposite direction, of making it as easy as possible for users to see
> > what's going on with their machine.
> 
> Not unless there's something in /proc/cpuinfo that can't be extracted
> in other methods. :) Anyway, you're offering a slippery-slope argument,
> I just want to avoid new interfaces from having needlessly permissive
> default access permissions.

No, I'm asking where the line is and how we decide what we want to
restrict. I hope you agree that it shouldn't be everything?

I'm pushing hard for making things easier to debug, and making it easier
for normal users to poke around and see what their system is doing.
Simpler, easier to understand tools, tools that teach users how the
system works. In my book, anything that lowers that barrier to entry is
a good thing.

I've had a lot of positive, useful interactions from this, and by
talking with users about what they see (I'm _always_ on IRC, and my
channel is pretty active). I can't overstate how useful this sort of
thing is, because there's only so many of us developers and users will
notice things that we don't - sometimes quite significant things! and if
we focus on making the system easy to debug, often times they'll do a
lot of that debugging for us.

Feeding user's curiosity pays great dividends in the long run.

And on the usability side, we've done a lot over the years to nudge
people away from running as root; if we set things up so that prefixing
every command with 'sudo' becomes the default, then we really lose out
on much of the benefit of that.

> I expect this to be enabled by default in distros, etc, so I'd like
> to make sure it's not giving attackers more information than they
> had before. Even reporting how much has been allocated at a specific
> location means an attacker ends up with extremely accurate information
> when attempting heap grooming, etc. Even the low granularity of slabinfo
> is sufficient to improve attacks. (Which is why it's 0400 by default too.)

This is pretty esoteric stuff to me; what users should be remotely
concerned about heap grooming attacks on their personal machines?

When we let the memory-unsafety of C (as well as spectre/meltdown)
dominate system design, the result is... not good. Those are things that
need to be fixed at the root (and, thank Rust, _are_), so let's maybe
not go _too_ overboard otherwise it's going to take another couple
decades to unwind all the resulting stockholm syndrome.

> > Instead, why not a sysctl, like we already have for perf?
> 
> perf is a whole other beast, including syscalls, etc.

But semantically it's the exact same thing. With perf we've got a knob
to say "I'm not paranoid on this machine, I don't want users to have to
be able to root to do basic debugging" - why not lean into that?

Have a global knob that's more than just for perf, so single user and
desktop machines can probably leave it loose (or even expose it in the
distro installer) and servers can have it on.

Give users a way to say what they want.

> > The concern about leaking image layout could be addressed by sorting the
> > output before returning to userspace.
> 
> It's trivial to change permissions from the default 0400 at boot time.
> It can even have groups and ownership changed, etc. This is why we have
> per-mount-namespace /proc instances:
> 
> # chgrp sysmonitor /proc/allocinfo
> # chmod 0440 /proc/allocinfo
> 
> Poof, instant role-based access control. :)

The trouble is that the average user isn't going to know what reporting
interfaces it's safe to do that with; we don't want to encourage that
kind of mucking around by the unsuspecting. We really should have a
knob.

Powered by blists - more mailing lists