From: Kay Sievers <[email protected]>
Some drivers want to tell userspace what uid and gid should be used for
their device nodes, so allow that information to percolate through the
driver core to userspace in order to make this happen. This means that
some systems (i.e. Android and friends) will not need to even run a
udev-like daemon for their device node manager and can just rely in
devtmpfs fully, reducing their footprint even more.
Signed-off-by: Kay Sievers <[email protected]>
Signed-off-by: Greg Kroah-Hartman <[email protected]>
---
block/genhd.c | 3 ++-
drivers/base/core.c | 17 +++++++++++++----
drivers/base/devtmpfs.c | 27 +++++++++++++++++----------
drivers/usb/core/usb.c | 3 ++-
include/linux/device.h | 7 +++++--
5 files changed, 39 insertions(+), 18 deletions(-)
--- a/block/genhd.c
+++ b/block/genhd.c
@@ -1111,7 +1111,8 @@ struct class block_class = {
.name = "block",
};
-static char *block_devnode(struct device *dev, umode_t *mode)
+static char *block_devnode(struct device *dev, umode_t *mode,
+ uid_t *uid, gid_t *gid)
{
struct gendisk *disk = dev_to_disk(dev);
--- a/drivers/base/core.c
+++ b/drivers/base/core.c
@@ -283,15 +283,21 @@ static int dev_uevent(struct kset *kset,
const char *tmp;
const char *name;
umode_t mode = 0;
+ uid_t uid = 0;
+ gid_t gid = 0;
add_uevent_var(env, "MAJOR=%u", MAJOR(dev->devt));
add_uevent_var(env, "MINOR=%u", MINOR(dev->devt));
- name = device_get_devnode(dev, &mode, &tmp);
+ name = device_get_devnode(dev, &mode, &uid, &gid, &tmp);
if (name) {
add_uevent_var(env, "DEVNAME=%s", name);
- kfree(tmp);
if (mode)
add_uevent_var(env, "DEVMODE=%#o", mode & 0777);
+ if (uid)
+ add_uevent_var(env, "DEVUID=%u", uid);
+ if (gid)
+ add_uevent_var(env, "DEVGID=%u", gid);
+ kfree(tmp);
}
}
@@ -1274,6 +1280,8 @@ static struct device *next_device(struct
* device_get_devnode - path of device node file
* @dev: device
* @mode: returned file access mode
+ * @uid: returned file owner
+ * @gid: returned file group
* @tmp: possibly allocated string
*
* Return the relative path of a possible device node.
@@ -1282,7 +1290,8 @@ static struct device *next_device(struct
* freed by the caller.
*/
const char *device_get_devnode(struct device *dev,
- umode_t *mode, const char **tmp)
+ umode_t *mode, uid_t *uid, gid_t *gid,
+ const char **tmp)
{
char *s;
@@ -1290,7 +1299,7 @@ const char *device_get_devnode(struct de
/* the device type may provide a specific name */
if (dev->type && dev->type->devnode)
- *tmp = dev->type->devnode(dev, mode);
+ *tmp = dev->type->devnode(dev, mode, uid, gid);
if (*tmp)
return *tmp;
--- a/drivers/base/devtmpfs.c
+++ b/drivers/base/devtmpfs.c
@@ -41,6 +41,8 @@ static struct req {
int err;
const char *name;
umode_t mode; /* 0 => delete */
+ uid_t uid;
+ gid_t gid;
struct device *dev;
} *requests;
@@ -85,7 +87,9 @@ int devtmpfs_create_node(struct device *
return 0;
req.mode = 0;
- req.name = device_get_devnode(dev, &req.mode, &tmp);
+ req.uid = 0;
+ req.gid = 0;
+ req.name = device_get_devnode(dev, &req.mode, &req.uid, &req.gid, &tmp);
if (!req.name)
return -ENOMEM;
@@ -121,7 +125,7 @@ int devtmpfs_delete_node(struct device *
if (!thread)
return 0;
- req.name = device_get_devnode(dev, NULL, &tmp);
+ req.name = device_get_devnode(dev, NULL, NULL, NULL, &tmp);
if (!req.name)
return -ENOMEM;
@@ -187,7 +191,8 @@ static int create_path(const char *nodep
return err;
}
-static int handle_create(const char *nodename, umode_t mode, struct device *dev)
+static int handle_create(const char *nodename, umode_t mode, uid_t uid,
+ gid_t gid, struct device *dev)
{
struct dentry *dentry;
struct path path;
@@ -201,14 +206,14 @@ static int handle_create(const char *nod
if (IS_ERR(dentry))
return PTR_ERR(dentry);
- err = vfs_mknod(path.dentry->d_inode,
- dentry, mode, dev->devt);
+ err = vfs_mknod(path.dentry->d_inode, dentry, mode, dev->devt);
if (!err) {
struct iattr newattrs;
- /* fixup possibly umasked mode */
newattrs.ia_mode = mode;
- newattrs.ia_valid = ATTR_MODE;
+ newattrs.ia_uid = uid;
+ newattrs.ia_gid = gid;
+ newattrs.ia_valid = ATTR_MODE|ATTR_UID|ATTR_GID;
mutex_lock(&dentry->d_inode->i_mutex);
notify_change(dentry, &newattrs);
mutex_unlock(&dentry->d_inode->i_mutex);
@@ -358,10 +363,11 @@ int devtmpfs_mount(const char *mntdir)
static DECLARE_COMPLETION(setup_done);
-static int handle(const char *name, umode_t mode, struct device *dev)
+static int handle(const char *name, umode_t mode, uid_t uid, gid_t gid,
+ struct device *dev)
{
if (mode)
- return handle_create(name, mode, dev);
+ return handle_create(name, mode, uid, gid, dev);
else
return handle_remove(name, dev);
}
@@ -387,7 +393,8 @@ static int devtmpfsd(void *p)
spin_unlock(&req_lock);
while (req) {
struct req *next = req->next;
- req->err = handle(req->name, req->mode, req->dev);
+ req->err = handle(req->name, req->mode,
+ req->uid, req->gid, req->dev);
complete(&req->done);
req = next;
}
--- a/drivers/usb/core/usb.c
+++ b/drivers/usb/core/usb.c
@@ -317,7 +317,8 @@ static const struct dev_pm_ops usb_devic
#endif /* CONFIG_PM */
-static char *usb_devnode(struct device *dev, umode_t *mode)
+static char *usb_devnode(struct device *dev,
+ umode_t *mode, uid_t *uid, gid_t *gid)
{
struct usb_device *usb_dev;
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -25,6 +25,7 @@
#include <linux/pm.h>
#include <linux/atomic.h>
#include <linux/ratelimit.h>
+#include <linux/uidgid.h>
#include <asm/device.h>
struct device;
@@ -471,7 +472,8 @@ struct device_type {
const char *name;
const struct attribute_group **groups;
int (*uevent)(struct device *dev, struct kobj_uevent_env *env);
- char *(*devnode)(struct device *dev, umode_t *mode);
+ char *(*devnode)(struct device *dev, umode_t *mode,
+ uid_t *uid, gid_t *gid);
void (*release)(struct device *dev);
const struct dev_pm_ops *pm;
@@ -849,7 +851,8 @@ extern int device_rename(struct device *
extern int device_move(struct device *dev, struct device *new_parent,
enum dpm_order dpm_order);
extern const char *device_get_devnode(struct device *dev,
- umode_t *mode, const char **tmp);
+ umode_t *mode, uid_t *uid, gid_t *gid,
+ const char **tmp);
extern void *dev_get_drvdata(const struct device *dev);
extern int dev_set_drvdata(struct device *dev, void *data);
On Sat, Apr 06, 2013 at 09:56:00AM -0700, Greg KH wrote:
> From: Kay Sievers <[email protected]>
>
> Some drivers want to tell userspace what uid and gid should be used for
> their device nodes, so allow that information to percolate through the
> driver core to userspace in order to make this happen. This means that
> some systems (i.e. Android and friends) will not need to even run a
> udev-like daemon for their device node manager and can just rely in
> devtmpfs fully, reducing their footprint even more.
Excuse me, but "some drivers" have no fucking business to mess with
uid and gid assignments in the first place...
On Sat, Apr 06, 2013 at 06:09:52PM +0100, Al Viro wrote:
> On Sat, Apr 06, 2013 at 09:56:00AM -0700, Greg KH wrote:
> > From: Kay Sievers <[email protected]>
> >
> > Some drivers want to tell userspace what uid and gid should be used for
> > their device nodes, so allow that information to percolate through the
> > driver core to userspace in order to make this happen. This means that
> > some systems (i.e. Android and friends) will not need to even run a
> > udev-like daemon for their device node manager and can just rely in
> > devtmpfs fully, reducing their footprint even more.
>
> Excuse me, but "some drivers" have no fucking business to mess with
> uid and gid assignments in the first place...
Why not? "closed" systems, like Android and other embedded systems,
have "assigned" uid and gid values that never change. Right now they
have to have a horrible shell-script to set these values in devtmpfs
when the device shows up to the needed numbers. This tiny patch gets
rid of that shell script entirely, allowing them to specify it from the
kernel as needed.
thanks,
greg k-h
On Sat, Apr 06, 2013 at 10:26:12AM -0700, Greg KH wrote:
> Why not? "closed" systems, like Android and other embedded systems,
> have "assigned" uid and gid values that never change. Right now they
> have to have a horrible shell-script to set these values in devtmpfs
> when the device shows up to the needed numbers. This tiny patch gets
> rid of that shell script entirely, allowing them to specify it from the
> kernel as needed.
What's to stop them from using static /dev? Has an extra benefit of
getting rid of devtmpfs shite completely...
On Sat, Apr 06, 2013 at 06:45:12PM +0100, Al Viro wrote:
> On Sat, Apr 06, 2013 at 10:26:12AM -0700, Greg KH wrote:
>
> > Why not? "closed" systems, like Android and other embedded systems,
> > have "assigned" uid and gid values that never change. Right now they
> > have to have a horrible shell-script to set these values in devtmpfs
> > when the device shows up to the needed numbers. This tiny patch gets
> > rid of that shell script entirely, allowing them to specify it from the
> > kernel as needed.
>
> What's to stop them from using static /dev? Has an extra benefit of
> getting rid of devtmpfs shite completely...
True, it would, but they like using devtmpfs :)
This change also allows systems that have "control" devices to properly
be able to pass in the uid for the device they are creating, like rawctl
(which I know is "obsolete"), and probably dm and lvm. I thought loop
devices might also want this, as they can now be created by normal
users, but I don't think that's needed for them.
thanks,
greg k-h
On Sat, Apr 6, 2013 at 7:58 PM, Greg KH <[email protected]> wrote:
> On Sat, Apr 06, 2013 at 06:45:12PM +0100, Al Viro wrote:
>> On Sat, Apr 06, 2013 at 10:26:12AM -0700, Greg KH wrote:
>>
>> > Why not? "closed" systems, like Android and other embedded systems,
>> > have "assigned" uid and gid values that never change. Right now they
>> > have to have a horrible shell-script to set these values in devtmpfs
>> > when the device shows up to the needed numbers. This tiny patch gets
>> > rid of that shell script entirely, allowing them to specify it from the
>> > kernel as needed.
>>
>> What's to stop them from using static /dev? Has an extra benefit of
>> getting rid of devtmpfs shite completely...
>
> True, it would, but they like using devtmpfs :)
>
> This change also allows systems that have "control" devices to properly
> be able to pass in the uid for the device they are creating, like rawctl
> (which I know is "obsolete"), and probably dm and lvm. I thought loop
> devices might also want this, as they can now be created by normal
> users, but I don't think that's needed for them.
It is generally useful to be able to control the uid/gid of
userspace-initiated device nodes, instead of racy post-adjusting this
setting from udev rules with an unpredictable long window between the
request and the adjustment.
The created device node can inherit the ownership of the calling
process, in a similar way like we do for devpts.
We have the same for the mode already, and want to be able to do the
same for the other permissions properties.
Thanks,
Kay
On 04/06/2013 11:56:00 AM, Greg KH wrote:
> From: Kay Sievers <[email protected]>
>
> Some drivers want to tell userspace what uid and gid should be used
> for
> their device nodes, so allow that information to percolate through the
> driver core to userspace in order to make this happen. This means
> that
> some systems (i.e. Android and friends) will not need to even run a
> udev-like daemon for their device node manager and can just rely in
> devtmpfs fully, reducing their footprint even more.
Wasn't the entire "devfsd" saga because this was policy and didn't
belong in kernel space? I guess it's not policy if Android wants it?
It's just The One True Way?
Or is this because containers allow UID/GID to be redefined, and thus
imposing magic values on userspace can now be mapped away or something?
(I studied this fairly closely before writing busybox mdev way back,
and I'm really not following the change in rationale here.)
Rob-
On Mon, Apr 08, 2013 at 01:14:13PM -0500, Rob Landley wrote:
> On 04/06/2013 11:56:00 AM, Greg KH wrote:
> >From: Kay Sievers <[email protected]>
> >
> >Some drivers want to tell userspace what uid and gid should be
> >used for
> >their device nodes, so allow that information to percolate through the
> >driver core to userspace in order to make this happen. This means
> >that
> >some systems (i.e. Android and friends) will not need to even run a
> >udev-like daemon for their device node manager and can just rely in
> >devtmpfs fully, reducing their footprint even more.
>
> Wasn't the entire "devfsd" saga because this was policy and didn't
> belong in kernel space?
devfs did a number of things "wrong", not the least being it set a
naming policy that was non-standard, and it had unfixable race conditons
in it.
> I guess it's not policy if Android wants it?
> It's just The One True Way?
Don't be facetious please.
> Or is this because containers allow UID/GID to be redefined, and
> thus imposing magic values on userspace can now be mapped away or
> something?
I don't understand, what do you mean by this?
greg k-h
On 04/08/2013 01:25:27 PM, Greg KH wrote:
> On Mon, Apr 08, 2013 at 01:14:13PM -0500, Rob Landley wrote:
> > On 04/06/2013 11:56:00 AM, Greg KH wrote:
> > >From: Kay Sievers <[email protected]>
> > >
> > >Some drivers want to tell userspace what uid and gid should be
> > >used for
> > >their device nodes, so allow that information to percolate through
> the
> > >driver core to userspace in order to make this happen. This means
> > >that
> > >some systems (i.e. Android and friends) will not need to even run
> a
> > >udev-like daemon for their device node manager and can just rely in
> > >devtmpfs fully, reducing their footprint even more.
> >
> > Wasn't the entire "devfsd" saga because this was policy and didn't
> > belong in kernel space?
>
> devfs did a number of things "wrong",
"He killed people and had bad breath. My breath is minty fresh."
Ok...
> not the least being it set a
> naming policy that was non-standard, and it had unfixable race
> conditons
> in it.
Yes, it had multiple types of policy in the kernel that people objected
to, which is why it went away replaced by userspace notifiers that let
stuff like udev and mdev do it right. But the motivation for going down
that path in the first place was to keep knowledge of things like uids
out of the kernel.
This used to be verbotten as policy in kernel space. Now it's not. I'm
wondering if there's a rationale for the change other than "android
wants what we used to go to great lengths to forbid".
(Half the reason for capability bits was so that even UID 0 wouldn't be
special. But this is offhandedly hardwiring in unlimited magic UIDs and
GIDs into drivers, with nobody even bothering to track them that I've
noticed...)
> > I guess it's not policy if Android wants it?
> > It's just The One True Way?
>
> Don't be facetious please.
So ubuntu will never want to use a driver that started life under
bionic, because the hardware is never going to overlap or because
they'll rewrite them? Bionic will coordinate with gentoo or LSB or
something to make sure that the UIDs it's claiming are out of a
reserved driver-only space?
Have you informed lanana of the need to start a UID/GID tracking list,
and gotten buy-in from the android developers (and the other distros)
to coordinate with them when adding new UIDs to drivers?
Or do you think it will simply never cause a problem, so there's no
need to worry?
> > Or is this because containers allow UID/GID to be redefined, and
> > thus imposing magic values on userspace can now be mapped away or
> > something?
>
> I don't understand, what do you mean by this?
I mean that each container can have its own UID/GID namespace now, I
was wondering if a driver claims a UID that an existing root filsystem
is already using for something else, can a container remap it away so
it doesn't conflict? Or will it still need manual udev rules to adjust
at hotplug time?
If a container _does_ remap its UID range, will its devtmpfs instance
populate with the host UID/GID or the remapped UID/GID? (If remapped,
will it gracefully handle the mapped UID/GID range not being big
enough?)
> greg k-h
Rob-
On Sun, Apr 7, 2013 at 12:56 AM, Greg KH <[email protected]> wrote:
> @@ -201,14 +206,14 @@ static int handle_create(const char *nod
> if (IS_ERR(dentry))
> return PTR_ERR(dentry);
>
> - err = vfs_mknod(path.dentry->d_inode,
> - dentry, mode, dev->devt);
> + err = vfs_mknod(path.dentry->d_inode, dentry, mode, dev->devt);
> if (!err) {
> struct iattr newattrs;
>
> - /* fixup possibly umasked mode */
> newattrs.ia_mode = mode;
> - newattrs.ia_valid = ATTR_MODE;
> + newattrs.ia_uid = uid;
> + newattrs.ia_gid = gid;
drivers/base/devtmpfs.c: In function 'handle_create':
drivers/base/devtmpfs.c:214:19: error: incompatible types when
assigning to type 'kuid_t' from type 'uid_t'
drivers/base/devtmpfs.c:215:19: error: incompatible types when
assigning to type 'kgid_t' from type 'gid_t'
make[2]: *** [drivers/base/devtmpfs.o] Error 1
Looks the below patch may fix it.
--
diff --git a/drivers/base/devtmpfs.c b/drivers/base/devtmpfs.c
index fda5256..e268fc5 100644
--- a/drivers/base/devtmpfs.c
+++ b/drivers/base/devtmpfs.c
@@ -211,8 +211,8 @@ static int handle_create(const char *nodename,
umode_t mode, uid_t uid,
struct iattr newattrs;
newattrs.ia_mode = mode;
- newattrs.ia_uid = uid;
- newattrs.ia_gid = gid;
+ newattrs.ia_uid = KUIDT_INIT(uid);
+ newattrs.ia_gid = KGIDT_INIT(gid);
newattrs.ia_valid = ATTR_MODE|ATTR_UID|ATTR_GID;
mutex_lock(&dentry->d_inode->i_mutex);
notify_change(dentry, &newattrs);
Thanks,
--
Ming Lei
On Wed, Apr 10, 2013 at 05:12:17PM +0800, Ming Lei wrote:
> On Sun, Apr 7, 2013 at 12:56 AM, Greg KH <[email protected]> wrote:
> > @@ -201,14 +206,14 @@ static int handle_create(const char *nod
> > if (IS_ERR(dentry))
> > return PTR_ERR(dentry);
> >
> > - err = vfs_mknod(path.dentry->d_inode,
> > - dentry, mode, dev->devt);
> > + err = vfs_mknod(path.dentry->d_inode, dentry, mode, dev->devt);
> > if (!err) {
> > struct iattr newattrs;
> >
> > - /* fixup possibly umasked mode */
> > newattrs.ia_mode = mode;
> > - newattrs.ia_valid = ATTR_MODE;
> > + newattrs.ia_uid = uid;
> > + newattrs.ia_gid = gid;
>
> drivers/base/devtmpfs.c: In function 'handle_create':
> drivers/base/devtmpfs.c:214:19: error: incompatible types when
> assigning to type 'kuid_t' from type 'uid_t'
> drivers/base/devtmpfs.c:215:19: error: incompatible types when
> assigning to type 'kgid_t' from type 'gid_t'
> make[2]: *** [drivers/base/devtmpfs.o] Error 1
I can't duplicate this here, what options am I missing?
> Looks the below patch may fix it.
> --
> diff --git a/drivers/base/devtmpfs.c b/drivers/base/devtmpfs.c
> index fda5256..e268fc5 100644
> --- a/drivers/base/devtmpfs.c
> +++ b/drivers/base/devtmpfs.c
> @@ -211,8 +211,8 @@ static int handle_create(const char *nodename,
> umode_t mode, uid_t uid,
> struct iattr newattrs;
>
> newattrs.ia_mode = mode;
> - newattrs.ia_uid = uid;
> - newattrs.ia_gid = gid;
> + newattrs.ia_uid = KUIDT_INIT(uid);
> + newattrs.ia_gid = KGIDT_INIT(gid);
> newattrs.ia_valid = ATTR_MODE|ATTR_UID|ATTR_GID;
> mutex_lock(&dentry->d_inode->i_mutex);
> notify_change(dentry, &newattrs);
>
Care to resend this in a format I can apply it in? :)
thanks,
greg k-h
On Wed, Apr 10, 2013 at 11:56 PM, Greg KH <[email protected]> wrote:
> On Wed, Apr 10, 2013 at 05:12:17PM +0800, Ming Lei wrote:
>> On Sun, Apr 7, 2013 at 12:56 AM, Greg KH <[email protected]> wrote:
>> > @@ -201,14 +206,14 @@ static int handle_create(const char *nod
>> > if (IS_ERR(dentry))
>> > return PTR_ERR(dentry);
>> >
>> > - err = vfs_mknod(path.dentry->d_inode,
>> > - dentry, mode, dev->devt);
>> > + err = vfs_mknod(path.dentry->d_inode, dentry, mode, dev->devt);
>> > if (!err) {
>> > struct iattr newattrs;
>> >
>> > - /* fixup possibly umasked mode */
>> > newattrs.ia_mode = mode;
>> > - newattrs.ia_valid = ATTR_MODE;
>> > + newattrs.ia_uid = uid;
>> > + newattrs.ia_gid = gid;
>>
>> drivers/base/devtmpfs.c: In function 'handle_create':
>> drivers/base/devtmpfs.c:214:19: error: incompatible types when
>> assigning to type 'kuid_t' from type 'uid_t'
>> drivers/base/devtmpfs.c:215:19: error: incompatible types when
>> assigning to type 'kgid_t' from type 'gid_t'
>> make[2]: *** [drivers/base/devtmpfs.o] Error 1
>
> I can't duplicate this here, what options am I missing?
With CONFIG_UIDGID_STRICT_TYPE_CHECKS
>
>> Looks the below patch may fix it.
>> --
>> diff --git a/drivers/base/devtmpfs.c b/drivers/base/devtmpfs.c
>> index fda5256..e268fc5 100644
>> --- a/drivers/base/devtmpfs.c
>> +++ b/drivers/base/devtmpfs.c
>> @@ -211,8 +211,8 @@ static int handle_create(const char *nodename,
>> umode_t mode, uid_t uid,
>> struct iattr newattrs;
>>
>> newattrs.ia_mode = mode;
>> - newattrs.ia_uid = uid;
>> - newattrs.ia_gid = gid;
>> + newattrs.ia_uid = KUIDT_INIT(uid);
>> + newattrs.ia_gid = KGIDT_INIT(gid);
>> newattrs.ia_valid = ATTR_MODE|ATTR_UID|ATTR_GID;
>> mutex_lock(&dentry->d_inode->i_mutex);
>> notify_change(dentry, &newattrs);
>>
>
> Care to resend this in a format I can apply it in? :)
OK.
thanks,
--
Ming Lei
Greg KH <[email protected]> writes:
> From: Kay Sievers <[email protected]>
>
> Some drivers want to tell userspace what uid and gid should be used for
> their device nodes, so allow that information to percolate through the
> driver core to userspace in order to make this happen. This means that
> some systems (i.e. Android and friends) will not need to even run a
> udev-like daemon for their device node manager and can just rely in
> devtmpfs fully, reducing their footprint even more.
This patch is really badly broken in it's uid and gid handling.
Nearly every line of this patch is wrong.
uids and gids in the kernel need to be stored in kuid_t's and kgid_t's.
> Signed-off-by: Kay Sievers <[email protected]>
> Signed-off-by: Greg Kroah-Hartman <[email protected]>
> ---
> block/genhd.c | 3 ++-
> drivers/base/core.c | 17 +++++++++++++----
> drivers/base/devtmpfs.c | 27 +++++++++++++++++----------
> drivers/usb/core/usb.c | 3 ++-
> include/linux/device.h | 7 +++++--
> 5 files changed, 39 insertions(+), 18 deletions(-)
>
> --- a/block/genhd.c
> +++ b/block/genhd.c
> @@ -1111,7 +1111,8 @@ struct class block_class = {
> .name = "block",
> };
>
> -static char *block_devnode(struct device *dev, umode_t *mode)
> +static char *block_devnode(struct device *dev, umode_t *mode,
> + uid_t *uid, gid_t *gid)
This needs be kuid_t and kgid_t.
> {
> struct gendisk *disk = dev_to_disk(dev);
>
> --- a/drivers/base/core.c
> +++ b/drivers/base/core.c
> @@ -283,15 +283,21 @@ static int dev_uevent(struct kset *kset,
> const char *tmp;
> const char *name;
> umode_t mode = 0;
> + uid_t uid = 0;
> + gid_t gid = 0;
This needs to be:
kuid_t uid = GLOBAL_ROOT_UID;
kgid_t gid = GLOBAL_ROOT_GID;
> add_uevent_var(env, "MAJOR=%u", MAJOR(dev->devt));
> add_uevent_var(env, "MINOR=%u", MINOR(dev->devt));
> - name = device_get_devnode(dev, &mode, &tmp);
> + name = device_get_devnode(dev, &mode, &uid, &gid, &tmp);
> if (name) {
> add_uevent_var(env, "DEVNAME=%s", name);
> - kfree(tmp);
> if (mode)
> add_uevent_var(env, "DEVMODE=%#o", mode & 0777);
> + if (uid)
> + add_uevent_var(env, "DEVUID=%u", uid);
> + if (gid)
> + add_uevent_var(env, "DEVGID=%u", gid);
Which user namespace are your users in?
At the very least this should be:
+ if (!uid_eq(uid, GLOBAL_ROOT_UID)
+ add_uevent_var(env, "DEVUID=%u", from_kuid(&init_user_ns, uid));
+ if (!gid_eq(gid, GLOBAL_ROOT_GID))
+ add_uevent_var(env, "DEVGID=%u", from_kgid(&init_user_ns, gid));
I suppose you can assume your users are in the initial user namespace,
as mknod won't work in any other user namespace.
Still it approaches being twisted to have files like
/sys/class/net/eth0/uevent that anyone can read that will only return
values in the initial user namespace.
> + kfree(tmp);
> }
> }
>
> @@ -1274,6 +1280,8 @@ static struct device *next_device(struct
> * device_get_devnode - path of device node file
> * @dev: device
> * @mode: returned file access mode
> + * @uid: returned file owner
> + * @gid: returned file group
> * @tmp: possibly allocated string
> *
> * Return the relative path of a possible device node.
> @@ -1282,7 +1290,8 @@ static struct device *next_device(struct
> * freed by the caller.
> */
> const char *device_get_devnode(struct device *dev,
> - umode_t *mode, const char **tmp)
> + umode_t *mode, uid_t *uid, gid_t *gid,
> + const char **tmp)
This needs to be kuid_t and kgid_t
> {
> char *s;
>
> @@ -1290,7 +1299,7 @@ const char *device_get_devnode(struct de
>
> /* the device type may provide a specific name */
> if (dev->type && dev->type->devnode)
> - *tmp = dev->type->devnode(dev, mode);
> + *tmp = dev->type->devnode(dev, mode, uid, gid);
> if (*tmp)
> return *tmp;
>
> --- a/drivers/base/devtmpfs.c
> +++ b/drivers/base/devtmpfs.c
> @@ -41,6 +41,8 @@ static struct req {
> int err;
> const char *name;
> umode_t mode; /* 0 => delete */
> + uid_t uid;
> + gid_t gid;
This needs to be kuid_t and kgid_t.
> struct device *dev;
> } *requests;
>
> @@ -85,7 +87,9 @@ int devtmpfs_create_node(struct device *
> return 0;
>
> req.mode = 0;
> - req.name = device_get_devnode(dev, &req.mode, &tmp);
> + req.uid = 0;
> + req.gid = 0;
This needs to be.
req.uid = GLOBAL_ROOT_UID;
req.gid = GLOBAL_ROOT_GID;
> + req.name = device_get_devnode(dev, &req.mode, &req.uid, &req.gid, &tmp);
> if (!req.name)
> return -ENOMEM;
>
> @@ -121,7 +125,7 @@ int devtmpfs_delete_node(struct device *
> if (!thread)
> return 0;
>
> - req.name = device_get_devnode(dev, NULL, &tmp);
> + req.name = device_get_devnode(dev, NULL, NULL, NULL, &tmp);
> if (!req.name)
> return -ENOMEM;
>
> @@ -187,7 +191,8 @@ static int create_path(const char *nodep
> return err;
> }
>
> -static int handle_create(const char *nodename, umode_t mode, struct device *dev)
> +static int handle_create(const char *nodename, umode_t mode, uid_t uid,
> + gid_t gid, struct device *dev)
This needs tobe kuid_t and kgid_t.
> {
> struct dentry *dentry;
> struct path path;
> @@ -201,14 +206,14 @@ static int handle_create(const char *nod
> if (IS_ERR(dentry))
> return PTR_ERR(dentry);
>
> - err = vfs_mknod(path.dentry->d_inode,
> - dentry, mode, dev->devt);
> + err = vfs_mknod(path.dentry->d_inode, dentry, mode, dev->devt);
Why this gratuitious change?
> if (!err) {
> struct iattr newattrs;
>
> - /* fixup possibly umasked mode */
> newattrs.ia_mode = mode;
> - newattrs.ia_valid = ATTR_MODE;
> + newattrs.ia_uid = uid;
> + newattrs.ia_gid = gid;
This doesn't even compile because the types are wrong.
> + newattrs.ia_valid = ATTR_MODE|ATTR_UID|ATTR_GID;
> mutex_lock(&dentry->d_inode->i_mutex);
> notify_change(dentry, &newattrs);
> mutex_unlock(&dentry->d_inode->i_mutex);
> @@ -358,10 +363,11 @@ int devtmpfs_mount(const char *mntdir)
>
> static DECLARE_COMPLETION(setup_done);
>
> -static int handle(const char *name, umode_t mode, struct device *dev)
> +static int handle(const char *name, umode_t mode, uid_t uid, gid_t gid,
The types are wrong here.
> + struct device *dev)
> {
> if (mode)
> - return handle_create(name, mode, dev);
> + return handle_create(name, mode, uid, gid, dev);
> else
> return handle_remove(name, dev);
> }
> @@ -387,7 +393,8 @@ static int devtmpfsd(void *p)
> spin_unlock(&req_lock);
> while (req) {
> struct req *next = req->next;
> - req->err = handle(req->name, req->mode, req->dev);
> + req->err = handle(req->name, req->mode,
> + req->uid, req->gid, req->dev);
> complete(&req->done);
> req = next;
> }
> --- a/drivers/usb/core/usb.c
> +++ b/drivers/usb/core/usb.c
> @@ -317,7 +317,8 @@ static const struct dev_pm_ops usb_devic
> #endif /* CONFIG_PM */
>
>
> -static char *usb_devnode(struct device *dev, umode_t *mode)
> +static char *usb_devnode(struct device *dev,
> + umode_t *mode, uid_t *uid, gid_t *gid)
The types are again wrong here.
> {
> struct usb_device *usb_dev;
>
> --- a/include/linux/device.h
> +++ b/include/linux/device.h
> @@ -25,6 +25,7 @@
> #include <linux/pm.h>
> #include <linux/atomic.h>
> #include <linux/ratelimit.h>
> +#include <linux/uidgid.h>
> #include <asm/device.h>
>
> struct device;
> @@ -471,7 +472,8 @@ struct device_type {
> const char *name;
> const struct attribute_group **groups;
> int (*uevent)(struct device *dev, struct kobj_uevent_env *env);
> - char *(*devnode)(struct device *dev, umode_t *mode);
> + char *(*devnode)(struct device *dev, umode_t *mode,
> + uid_t *uid, gid_t *gid);
The types are wrong here.
> void (*release)(struct device *dev);
>
> const struct dev_pm_ops *pm;
> @@ -849,7 +851,8 @@ extern int device_rename(struct device *
> extern int device_move(struct device *dev, struct device *new_parent,
> enum dpm_order dpm_order);
> extern const char *device_get_devnode(struct device *dev,
> - umode_t *mode, const char **tmp);
> + umode_t *mode, uid_t *uid, gid_t *gid,
And the types are wrong here.
> + const char **tmp);
> extern void *dev_get_drvdata(const struct device *dev);
> extern int dev_set_drvdata(struct device *dev, void *data);
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
On Wed, Apr 10, 2013 at 09:10:12PM -0700, Eric W. Biederman wrote:
> Greg KH <[email protected]> writes:
>
> > From: Kay Sievers <[email protected]>
> >
> > Some drivers want to tell userspace what uid and gid should be used for
> > their device nodes, so allow that information to percolate through the
> > driver core to userspace in order to make this happen. This means that
> > some systems (i.e. Android and friends) will not need to even run a
> > udev-like daemon for their device node manager and can just rely in
> > devtmpfs fully, reducing their footprint even more.
>
> This patch is really badly broken in it's uid and gid handling.
> Nearly every line of this patch is wrong.
>
> uids and gids in the kernel need to be stored in kuid_t's and kgid_t's.
Stored, yes, but defined that way as well?
> > -static char *block_devnode(struct device *dev, umode_t *mode)
> > +static char *block_devnode(struct device *dev, umode_t *mode,
> > + uid_t *uid, gid_t *gid)
> This needs be kuid_t and kgid_t.
>
> > {
> > struct gendisk *disk = dev_to_disk(dev);
> >
> > --- a/drivers/base/core.c
> > +++ b/drivers/base/core.c
> > @@ -283,15 +283,21 @@ static int dev_uevent(struct kset *kset,
> > const char *tmp;
> > const char *name;
> > umode_t mode = 0;
> > + uid_t uid = 0;
> > + gid_t gid = 0;
> This needs to be:
> kuid_t uid = GLOBAL_ROOT_UID;
> kgid_t gid = GLOBAL_ROOT_GID;
Ok, I'll fix this up and send a follow-on patch, thanks for the review.
>
> > add_uevent_var(env, "MAJOR=%u", MAJOR(dev->devt));
> > add_uevent_var(env, "MINOR=%u", MINOR(dev->devt));
> > - name = device_get_devnode(dev, &mode, &tmp);
> > + name = device_get_devnode(dev, &mode, &uid, &gid, &tmp);
> > if (name) {
> > add_uevent_var(env, "DEVNAME=%s", name);
> > - kfree(tmp);
> > if (mode)
> > add_uevent_var(env, "DEVMODE=%#o", mode & 0777);
> > + if (uid)
> > + add_uevent_var(env, "DEVUID=%u", uid);
> > + if (gid)
> > + add_uevent_var(env, "DEVGID=%u", gid);
>
> Which user namespace are your users in?
> At the very least this should be:
> + if (!uid_eq(uid, GLOBAL_ROOT_UID)
> + add_uevent_var(env, "DEVUID=%u", from_kuid(&init_user_ns, uid));
> + if (!gid_eq(gid, GLOBAL_ROOT_GID))
> + add_uevent_var(env, "DEVGID=%u", from_kgid(&init_user_ns, gid));
>
> I suppose you can assume your users are in the initial user namespace,
> as mknod won't work in any other user namespace.
Yes.
> Still it approaches being twisted to have files like
> /sys/class/net/eth0/uevent that anyone can read that will only return
> values in the initial user namespace.
As the nodes are only valid in the initial namespace, why would this
matter?
> > + newattrs.ia_uid = uid;
> > + newattrs.ia_gid = gid;
> This doesn't even compile because the types are wrong.
Yes, this has been fixed. But note, it only fails the build if
namespaces are enabled, and given that no one seems to have reported
this build error in either the 0-day tester, or linux-next, or the
distro developers that tested this patch on their systems, it seems that
no one enables namespaces on their systems :(
Again, thanks for the review, I'll go enable namespaces in my test
kernel and fix up the fallout.
greg k-h
On Thu, Apr 11, 2013 at 08:56:06AM -0700, Greg KH wrote:
> > > + newattrs.ia_uid = uid;
> > > + newattrs.ia_gid = gid;
> > This doesn't even compile because the types are wrong.
>
> Yes, this has been fixed. But note, it only fails the build if
> namespaces are enabled, and given that no one seems to have reported
> this build error in either the 0-day tester, or linux-next, or the
> distro developers that tested this patch on their systems, it seems that
> no one enables namespaces on their systems :(
USER_NS that is, not namespaces in general, sorry about that.
greg k-h
On Tue, Apr 09, 2013 at 10:11:55AM -0500, Rob Landley wrote:
>
> Or do you think it will simply never cause a problem, so there's no
> need to worry?
Based on the limited number of drivers that will be using this
interface, I don't think there's any need to worry. But I will be glad
to revisit this in the future if needed.
> >> Or is this because containers allow UID/GID to be redefined, and
> >> thus imposing magic values on userspace can now be mapped away or
> >> something?
> >
> >I don't understand, what do you mean by this?
>
> I mean that each container can have its own UID/GID namespace now, I
> was wondering if a driver claims a UID that an existing root
> filsystem is already using for something else, can a container remap
> it away so it doesn't conflict? Or will it still need manual udev
> rules to adjust at hotplug time?
I don't know how containers deal with the userspace major/minor device
nodes today at all, so I can't answer that. But see the review comments
from Eric that he sent, I'll make the needed changes based on that so
all should be ok after that.
thanks,
greg k-h
On Wed, Apr 10, 2013 at 09:10:12PM -0700, Eric W. Biederman wrote:
> Still it approaches being twisted to have files like
> /sys/class/net/eth0/uevent that anyone can read that will only return
> values in the initial user namespace.
Side note, I don't think that ethernet network devices have uids :)
Greg KH <[email protected]> writes:
> On Wed, Apr 10, 2013 at 09:10:12PM -0700, Eric W. Biederman wrote:
>> Greg KH <[email protected]> writes:
>>
>> > From: Kay Sievers <[email protected]>
>> >
>> > Some drivers want to tell userspace what uid and gid should be used for
>> > their device nodes, so allow that information to percolate through the
>> > driver core to userspace in order to make this happen. This means that
>> > some systems (i.e. Android and friends) will not need to even run a
>> > udev-like daemon for their device node manager and can just rely in
>> > devtmpfs fully, reducing their footprint even more.
>>
>> This patch is really badly broken in it's uid and gid handling.
>> Nearly every line of this patch is wrong.
>>
>> uids and gids in the kernel need to be stored in kuid_t's and kgid_t's.
>
> Stored, yes, but defined that way as well?
At this point uid_t and gid_t are like __user pointers. Something you
really only want to have in code that is directly interacting with
userspace.
Since there are nice typing properties and no runtime downsides we want
to use kuid_t and kgid_t as much as possible.
What I find is that if I push kuid_t and kgid_t everywhere I find kernel
interfaces that I would never has suspected of using uids and gids.
>> > -static char *block_devnode(struct device *dev, umode_t *mode)
>> > +static char *block_devnode(struct device *dev, umode_t *mode,
>> > + uid_t *uid, gid_t *gid)
>> This needs be kuid_t and kgid_t.
>>
>> > {
>> > struct gendisk *disk = dev_to_disk(dev);
>> >
>> > --- a/drivers/base/core.c
>> > +++ b/drivers/base/core.c
>> > @@ -283,15 +283,21 @@ static int dev_uevent(struct kset *kset,
>> > const char *tmp;
>> > const char *name;
>> > umode_t mode = 0;
>> > + uid_t uid = 0;
>> > + gid_t gid = 0;
>> This needs to be:
>> kuid_t uid = GLOBAL_ROOT_UID;
>> kgid_t gid = GLOBAL_ROOT_GID;
>
> Ok, I'll fix this up and send a follow-on patch, thanks for the review.
>
>>
>> > add_uevent_var(env, "MAJOR=%u", MAJOR(dev->devt));
>> > add_uevent_var(env, "MINOR=%u", MINOR(dev->devt));
>> > - name = device_get_devnode(dev, &mode, &tmp);
>> > + name = device_get_devnode(dev, &mode, &uid, &gid, &tmp);
>> > if (name) {
>> > add_uevent_var(env, "DEVNAME=%s", name);
>> > - kfree(tmp);
>> > if (mode)
>> > add_uevent_var(env, "DEVMODE=%#o", mode & 0777);
>> > + if (uid)
>> > + add_uevent_var(env, "DEVUID=%u", uid);
>> > + if (gid)
>> > + add_uevent_var(env, "DEVGID=%u", gid);
>>
>> Which user namespace are your users in?
>> At the very least this should be:
>> + if (!uid_eq(uid, GLOBAL_ROOT_UID)
>> + add_uevent_var(env, "DEVUID=%u", from_kuid(&init_user_ns, uid));
>> + if (!gid_eq(gid, GLOBAL_ROOT_GID))
>> + add_uevent_var(env, "DEVGID=%u", from_kgid(&init_user_ns, gid));
>>
>> I suppose you can assume your users are in the initial user namespace,
>> as mknod won't work in any other user namespace.
>
> Yes.
>
>> Still it approaches being twisted to have files like
>> /sys/class/net/eth0/uevent that anyone can read that will only return
>> values in the initial user namespace.
>
> As the nodes are only valid in the initial namespace, why would this
> matter?
>
>> > + newattrs.ia_uid = uid;
>> > + newattrs.ia_gid = gid;
>> This doesn't even compile because the types are wrong.
>
> Yes, this has been fixed. But note, it only fails the build if
> namespaces are enabled, and given that no one seems to have reported
> this build error in either the 0-day tester, or linux-next, or the
> distro developers that tested this patch on their systems, it seems that
> no one enables namespaces on their systems :(
Yeah. Not my favorite aspect of this. Although you could call it the
version of CONFIG_EXPERIMENTAL that distros respect.
Right now I have all of the kernel converted to use kuid_t and kgid_t
except for XFS. So there is a config guard that prevents XFS and user
namespaces from being enabled at the same time. Right now allyesconfig
and allmodconfig enable XFS and thus turn off user namespaces.
Hopefully I will get XFS sorted out before too much longer.
Until that happens I have been doing periodic sweeps during the merge
window and after -rc1 to keep things from bit-rotting.
> Again, thanks for the review, I'll go enable namespaces in my test
> kernel and fix up the fallout.
Thanks.
Eric
Greg KH <[email protected]> writes:
> On Wed, Apr 10, 2013 at 09:10:12PM -0700, Eric W. Biederman wrote:
>> Still it approaches being twisted to have files like
>> /sys/class/net/eth0/uevent that anyone can read that will only return
>> values in the initial user namespace.
>
> Side note, I don't think that ethernet network devices have uids :)
I didn't think any devices had uids... :)
The generic uevent file was the first place I could think of where we
output this information to userspace. And I don't think that uevent file
is specific network devices.
There isn't anything that limits our netlink messages to clients in the
initial user namespace either.
Nothing huge, but there are some goofy kernel/user interaction cases
that show up when you add this functionality.
Eric
On Thu, Apr 11, 2013 at 08:56:06AM -0700, Greg KH wrote:
> Again, thanks for the review, I'll go enable namespaces in my test
> kernel and fix up the fallout.
Here's the fixup patch, Eric, does it look correct to you?
thanks,
greg k-h
diff --git a/block/genhd.c b/block/genhd.c
index dfcec43..20625ee 100644
--- a/block/genhd.c
+++ b/block/genhd.c
@@ -1112,7 +1112,7 @@ struct class block_class = {
};
static char *block_devnode(struct device *dev, umode_t *mode,
- uid_t *uid, gid_t *gid)
+ kuid_t *uid, kgid_t *gid)
{
struct gendisk *disk = dev_to_disk(dev);
diff --git a/drivers/base/core.c b/drivers/base/core.c
index 8a428b5..f88d9e2 100644
--- a/drivers/base/core.c
+++ b/drivers/base/core.c
@@ -283,8 +283,8 @@ static int dev_uevent(struct kset *kset, struct kobject *kobj,
const char *tmp;
const char *name;
umode_t mode = 0;
- uid_t uid = 0;
- gid_t gid = 0;
+ kuid_t uid = GLOBAL_ROOT_UID;
+ kgid_t gid = GLOBAL_ROOT_GID;
add_uevent_var(env, "MAJOR=%u", MAJOR(dev->devt));
add_uevent_var(env, "MINOR=%u", MINOR(dev->devt));
@@ -293,10 +293,10 @@ static int dev_uevent(struct kset *kset, struct kobject *kobj,
add_uevent_var(env, "DEVNAME=%s", name);
if (mode)
add_uevent_var(env, "DEVMODE=%#o", mode & 0777);
- if (uid)
- add_uevent_var(env, "DEVUID=%u", uid);
- if (gid)
- add_uevent_var(env, "DEVGID=%u", gid);
+ if (!uid_eq(uid, GLOBAL_ROOT_UID))
+ add_uevent_var(env, "DEVUID=%u", from_kuid(&init_user_ns, uid));
+ if (!gid_eq(gid, GLOBAL_ROOT_GID))
+ add_uevent_var(env, "DEVGID=%u", from_kgid(&init_user_ns, gid));
kfree(tmp);
}
}
@@ -1297,7 +1297,7 @@ static struct device *next_device(struct klist_iter *i)
* freed by the caller.
*/
const char *device_get_devnode(struct device *dev,
- umode_t *mode, uid_t *uid, gid_t *gid,
+ umode_t *mode, kuid_t *uid, kgid_t *gid,
const char **tmp)
{
char *s;
diff --git a/drivers/base/devtmpfs.c b/drivers/base/devtmpfs.c
index abd4eee..7413d06 100644
--- a/drivers/base/devtmpfs.c
+++ b/drivers/base/devtmpfs.c
@@ -42,8 +42,8 @@ static struct req {
int err;
const char *name;
umode_t mode; /* 0 => delete */
- uid_t uid;
- gid_t gid;
+ kuid_t uid;
+ kgid_t gid;
struct device *dev;
} *requests;
@@ -88,8 +88,8 @@ int devtmpfs_create_node(struct device *dev)
return 0;
req.mode = 0;
- req.uid = 0;
- req.gid = 0;
+ req.uid = GLOBAL_ROOT_UID;
+ req.gid = GLOBAL_ROOT_GID;
req.name = device_get_devnode(dev, &req.mode, &req.uid, &req.gid, &tmp);
if (!req.name)
return -ENOMEM;
@@ -192,8 +192,8 @@ static int create_path(const char *nodepath)
return err;
}
-static int handle_create(const char *nodename, umode_t mode, uid_t uid,
- gid_t gid, struct device *dev)
+static int handle_create(const char *nodename, umode_t mode, kuid_t uid,
+ kgid_t gid, struct device *dev)
{
struct dentry *dentry;
struct path path;
@@ -212,8 +212,8 @@ static int handle_create(const char *nodename, umode_t mode, uid_t uid,
struct iattr newattrs;
newattrs.ia_mode = mode;
- newattrs.ia_uid = KUIDT_INIT(uid);
- newattrs.ia_gid = KGIDT_INIT(gid);
+ newattrs.ia_uid = uid;
+ newattrs.ia_gid = gid;
newattrs.ia_valid = ATTR_MODE|ATTR_UID|ATTR_GID;
mutex_lock(&dentry->d_inode->i_mutex);
notify_change(dentry, &newattrs);
@@ -364,7 +364,7 @@ int devtmpfs_mount(const char *mntdir)
static DECLARE_COMPLETION(setup_done);
-static int handle(const char *name, umode_t mode, uid_t uid, gid_t gid,
+static int handle(const char *name, umode_t mode, kuid_t uid, kgid_t gid,
struct device *dev)
{
if (mode)
diff --git a/drivers/usb/core/usb.c b/drivers/usb/core/usb.c
index 1700283..e092b41 100644
--- a/drivers/usb/core/usb.c
+++ b/drivers/usb/core/usb.c
@@ -318,7 +318,7 @@ static const struct dev_pm_ops usb_device_pm_ops = {
static char *usb_devnode(struct device *dev,
- umode_t *mode, uid_t *uid, gid_t *gid)
+ umode_t *mode, kuid_t *uid, kgid_t *gid)
{
struct usb_device *usb_dev;
diff --git a/include/linux/device.h b/include/linux/device.h
index 851b85c..88615cc 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -467,7 +467,7 @@ struct device_type {
const struct attribute_group **groups;
int (*uevent)(struct device *dev, struct kobj_uevent_env *env);
char *(*devnode)(struct device *dev, umode_t *mode,
- uid_t *uid, gid_t *gid);
+ kuid_t *uid, kgid_t *gid);
void (*release)(struct device *dev);
const struct dev_pm_ops *pm;
@@ -845,7 +845,7 @@ extern int device_rename(struct device *dev, const char *new_name);
extern int device_move(struct device *dev, struct device *new_parent,
enum dpm_order dpm_order);
extern const char *device_get_devnode(struct device *dev,
- umode_t *mode, uid_t *uid, gid_t *gid,
+ umode_t *mode, kuid_t *uid, kgid_t *gid,
const char **tmp);
extern void *dev_get_drvdata(const struct device *dev);
extern int dev_set_drvdata(struct device *dev, void *data);
Greg KH <[email protected]> writes:
> On Thu, Apr 11, 2013 at 08:56:06AM -0700, Greg KH wrote:
>> Again, thanks for the review, I'll go enable namespaces in my test
>> kernel and fix up the fallout.
>
> Here's the fixup patch, Eric, does it look correct to you?
That looks reasonable to me.
Eric