Commit 057f6c01 authored by James Morris's avatar James Morris Committed by Greg Kroah-Hartman

security: prevent permission checking of file removal via sysfs_remove_group()

Prevent permission checking from being performed when the kernel wants to
unconditionally remove a sysfs group, by introducing an kernel-only variant
of lookup_one_len(), lookup_one_len_kern().

Additionally, as sysfs_remove_group() does not check the return value of
the lookup before using it, a BUG_ON has been added to pinpoint the cause
of any problems potentially caused by this (and as a form of annotation).
Signed-off-by: default avatarJames Morris <jmorris@namei.org>
Cc: Nagendra Singh Tomar <nagendra_tomar@adaptec.com>
Cc: Tejun Heo <htejun@gmail.com>
Cc: Stephen Smalley <sds@tycho.nsa.gov>
Cc: Eric Paris <eparis@redhat.com>
Signed-off-by: default avatarAndrew Morton <akpm@linux-foundation.org>
Signed-off-by: default avatarGreg Kroah-Hartman <gregkh@suse.de>
parent 523ded71
...@@ -1243,22 +1243,13 @@ int __user_path_lookup_open(const char __user *name, unsigned int lookup_flags, ...@@ -1243,22 +1243,13 @@ int __user_path_lookup_open(const char __user *name, unsigned int lookup_flags,
return err; return err;
} }
/* static inline struct dentry *__lookup_hash_kern(struct qstr *name, struct dentry *base, struct nameidata *nd)
* Restricted form of lookup. Doesn't follow links, single-component only,
* needs parent already locked. Doesn't follow mounts.
* SMP-safe.
*/
static struct dentry * __lookup_hash(struct qstr *name, struct dentry * base, struct nameidata *nd)
{ {
struct dentry * dentry; struct dentry *dentry;
struct inode *inode; struct inode *inode;
int err; int err;
inode = base->d_inode; inode = base->d_inode;
err = permission(inode, MAY_EXEC, nd);
dentry = ERR_PTR(err);
if (err)
goto out;
/* /*
* See if the low-level filesystem might want * See if the low-level filesystem might want
...@@ -1287,35 +1278,76 @@ static struct dentry * __lookup_hash(struct qstr *name, struct dentry * base, st ...@@ -1287,35 +1278,76 @@ static struct dentry * __lookup_hash(struct qstr *name, struct dentry * base, st
return dentry; return dentry;
} }
/*
* Restricted form of lookup. Doesn't follow links, single-component only,
* needs parent already locked. Doesn't follow mounts.
* SMP-safe.
*/
static inline struct dentry * __lookup_hash(struct qstr *name, struct dentry *base, struct nameidata *nd)
{
struct dentry *dentry;
struct inode *inode;
int err;
inode = base->d_inode;
err = permission(inode, MAY_EXEC, nd);
dentry = ERR_PTR(err);
if (err)
goto out;
dentry = __lookup_hash_kern(name, base, nd);
out:
return dentry;
}
static struct dentry *lookup_hash(struct nameidata *nd) static struct dentry *lookup_hash(struct nameidata *nd)
{ {
return __lookup_hash(&nd->last, nd->dentry, nd); return __lookup_hash(&nd->last, nd->dentry, nd);
} }
/* SMP-safe */ /* SMP-safe */
struct dentry * lookup_one_len(const char * name, struct dentry * base, int len) static inline int __lookup_one_len(const char *name, struct qstr *this, struct dentry *base, int len)
{ {
unsigned long hash; unsigned long hash;
struct qstr this;
unsigned int c; unsigned int c;
this.name = name; this->name = name;
this.len = len; this->len = len;
if (!len) if (!len)
goto access; return -EACCES;
hash = init_name_hash(); hash = init_name_hash();
while (len--) { while (len--) {
c = *(const unsigned char *)name++; c = *(const unsigned char *)name++;
if (c == '/' || c == '\0') if (c == '/' || c == '\0')
goto access; return -EACCES;
hash = partial_name_hash(c, hash); hash = partial_name_hash(c, hash);
} }
this.hash = end_name_hash(hash); this->hash = end_name_hash(hash);
return 0;
}
struct dentry *lookup_one_len(const char *name, struct dentry *base, int len)
{
int err;
struct qstr this;
err = __lookup_one_len(name, &this, base, len);
if (err)
return ERR_PTR(err);
return __lookup_hash(&this, base, NULL); return __lookup_hash(&this, base, NULL);
access: }
return ERR_PTR(-EACCES);
struct dentry *lookup_one_len_kern(const char *name, struct dentry *base, int len)
{
int err;
struct qstr this;
err = __lookup_one_len(name, &this, base, len);
if (err)
return ERR_PTR(err);
return __lookup_hash_kern(&this, base, NULL);
} }
/* /*
......
...@@ -70,9 +70,11 @@ void sysfs_remove_group(struct kobject * kobj, ...@@ -70,9 +70,11 @@ void sysfs_remove_group(struct kobject * kobj,
{ {
struct dentry * dir; struct dentry * dir;
if (grp->name) if (grp->name) {
dir = lookup_one_len(grp->name, kobj->dentry, dir = lookup_one_len_kern(grp->name, kobj->dentry,
strlen(grp->name)); strlen(grp->name));
BUG_ON(IS_ERR(dir));
}
else else
dir = dget(kobj->dentry); dir = dget(kobj->dentry);
......
...@@ -82,6 +82,7 @@ extern struct file *nameidata_to_filp(struct nameidata *nd, int flags); ...@@ -82,6 +82,7 @@ extern struct file *nameidata_to_filp(struct nameidata *nd, int flags);
extern void release_open_intent(struct nameidata *); extern void release_open_intent(struct nameidata *);
extern struct dentry * lookup_one_len(const char *, struct dentry *, int); extern struct dentry * lookup_one_len(const char *, struct dentry *, int);
extern struct dentry *lookup_one_len_kern(const char *, struct dentry *, int);
extern int follow_down(struct vfsmount **, struct dentry **); extern int follow_down(struct vfsmount **, struct dentry **);
extern int follow_up(struct vfsmount **, struct dentry **); extern int follow_up(struct vfsmount **, struct dentry **);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment