linux-kernel - Re: [RFC PATCH] sysfs: bin

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:	Wed, 12 May 2010 12:13:36 -0700
From:	Greg KH <greg@...ah.com>
To:	Chris Wright <chrisw@...s-sol.org>
Cc:	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: [RFC PATCH] sysfs: bin_attr permission checking

On Wed, May 12, 2010 at 11:47:13AM -0700, Chris Wright 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.
> 
> When assigning a PCI device directly to a guest with libvirt and KVM, the 
> sysfs config space file is chown'd to the user that the KVM guest will
> run as.  The guest needs to have full access to the device's config
> space since it's responsible for driving the device.  However, despite
> being the owner of the sysfs file, the CAP_SYS_ADMIN check will not
> allow read access beyond the config header.
> 
> This patch adds a new bin_attr->read_file() callback which adds a struct
> file to the normal argument list.  This allows an implementation such as
> PCI config space bin_attr read_file handler to check both inode
> permission as well as privileges to determine whether to allow
> privileged actions through the handler.

Ick, this is all because we like showing different information if the
user is "privileged or not" :(

Turns out, that this probably isn't the best user api to implement,
remind me never to do that again...

> This is just RFC, although I've tested that it does allow the chown +
> read to work as expected.  Any other ideas of how to handle this are
> welcome.

Can we just pass in the 'file' for all users of the bin files instead of
the dentry?  You can always get the dentry from the file (as your patch
showes), and there isn't that many users of this interface.  I'd really
rather not have two different types of callbacks here.

thanks,

greg k-h
--
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/