linux-kernel - Re: [PATCH 2/2] pci: allow sysfs file owner to read device dependent config space

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]

Date:	Thu, 13 May 2010 08:05:54 -0700
From:	Chris Wright <chrisw@...s-sol.org>
To:	Alan Cox <alan@...rguk.ukuu.org.uk>
Cc:	Chris Wright <chrisw@...s-sol.org>, greg@...ah.com,
	jbarnes@...tuousgeek.org, matthew@....cx,
	linux-pci@...r.kernel.org, linux-kernel@...r.kernel.org,
	kvm@...r.kernel.org, ddutile@...hat.com, alex.williamson@...hat.com
Subject: Re: [PATCH 2/2] pci: allow sysfs file owner to read device dependent
 config space

* Alan Cox (alan@...rguk.ukuu.org.uk) wrote:
> On Wed, 12 May 2010 18:29:57 -0700
> Chris Wright <chrisw@...s-sol.org> wrote:
> 
> > The PCI config space bin_attr read handler has a hardcoded CAP_SYS_ADMIN
> > check to verify privileges before allowing a user to read device
> > dependent config space.  This is meant to protect from an unprivileged
> > user potentially locking up the box.
> 
> Or hacking it.

With read access?  You mean from read side-effect on a device register?

>  You could argue the correct requirement is to change it to
> require CAP_SYS_RAWIO in fact !

It's also pervasive through sysfs.  I found 22 CAP_SYS_ADMIN checks via
sysfs bin_attrs, most are firmware, config space, and similar; most of
which have could be abused to hack box in one way or another.

> > With this patch the sysfs file owner is also considered privileged enough
> > to read all of the config space.
> 
> Which breaks the main reason the check was there in the first place. To
> stop accidents of the form
> 
> 	find / -exec grep {} "wibble"
> 
> blowing up in people's faces.

Right, this won't change w/out sysadmin intervention.

Currently, any random user doing that won't trigger an ill-behaving
device dependent config space read.  It requires an admin to chown the
file to the user, at which point you've given the device to the user.
This is typically only done in the privileged context of libvirt launching
a KVM guest that has a host PCI device directly assigned to it, and the
chown is typically to a non-shell user.

> I agree with the problem - but IMHO the fix is to require opening the file
> checks CAP_SYS_something instead:

We can't require CAP_SYS_something on open as that will break basic tools
like lspci.  We could note the privileges when opened and check them
later.

>  not to hack the read method and make it
> even weirder and more un-Linux than it is now.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/