Re: [RFC PATCH] sysfs: bin_attr permission checking

[Greg KH]

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@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/