From: Phillip Lougher Date: Wed, 11 Jul 2007 18:04:24 +0000 (+0100) Subject: UBUNTU: sysfs_readdir NULL ptr dereference causes kernel oops (CVE-2007-3104) X-Git-Url: http://kernel.ubuntu.com/git?p=ubuntu/ubuntu-edgy.git;a=commitdiff;h=a8c3f241ea411211c4802098f23a8da309e8bbd1 UBUNTU: sysfs_readdir NULL ptr dereference causes kernel oops (CVE-2007-3104) A flaw in the sysfs_readdir function allowed a local user to cause a denial of service by dereferencing a NULL pointer. OriginalAuthor: Eric Sandeen (sandeen@redhat.com) Signed-off-by: Phillip Lougher --- Index: pardus-2.6/fs/sysfs/dir.c =================================================================== --- pardus-2.6.orig/fs/sysfs/dir.c +++ pardus-2.6/fs/sysfs/dir.c @@ -12,14 +12,26 @@ #include "sysfs.h" DECLARE_RWSEM(sysfs_rename_sem); +spinlock_t sysfs_lock = SPIN_LOCK_UNLOCKED; static void sysfs_d_iput(struct dentry * dentry, struct inode * inode) { struct sysfs_dirent * sd = dentry->d_fsdata; if (sd) { - BUG_ON(sd->s_dentry != dentry); - sd->s_dentry = NULL; + /* sd->s_dentry is protected with sysfs_lock. This + * allows sysfs_drop_dentry() to dereference it. + */ + spin_lock(&sysfs_lock); + + /* The dentry might have been deleted or another + * lookup could have happened updating sd->s_dentry to + * point the new dentry. Ignore if it isn't pointing + * to this dentry. + */ + if (sd->s_dentry == dentry) + sd->s_dentry = NULL; + spin_unlock(&sysfs_lock); sysfs_put(sd); } iput(inode); @@ -29,6 +41,14 @@ static struct dentry_operations sysfs_de .d_iput = sysfs_d_iput, }; +static unsigned int sysfs_inode_counter; +ino_t sysfs_get_inum(void) +{ + if (unlikely(sysfs_inode_counter < 3)) + sysfs_inode_counter = 3; + return sysfs_inode_counter++; +} + /* * Allocates a new sysfs_dirent and links it to the parent sysfs_dirent */ @@ -42,6 +62,7 @@ static struct sysfs_dirent * sysfs_new_d return NULL; memset(sd, 0, sizeof(*sd)); + sd->s_ino = sysfs_get_inum(); atomic_set(&sd->s_count, 1); atomic_set(&sd->s_event, 0); INIT_LIST_HEAD(&sd->s_children); @@ -209,7 +230,10 @@ static int sysfs_attach_attr(struct sysf } dentry->d_fsdata = sysfs_get(sd); + /* protect sd->s_dentry against sysfs_d_iput */ + spin_lock(&sysfs_lock); sd->s_dentry = dentry; + spin_unlock(&sysfs_lock); error = sysfs_create(dentry, (attr->mode & S_IALLUGO) | S_IFREG, init); if (error) { sysfs_put(sd); @@ -231,7 +255,10 @@ static int sysfs_attach_link(struct sysf int err = 0; dentry->d_fsdata = sysfs_get(sd); + /* protect sd->s_dentry against sysfs_d_iput */ + spin_lock(&sysfs_lock); sd->s_dentry = dentry; + spin_unlock(&sysfs_lock); err = sysfs_create(dentry, S_IFLNK|S_IRWXUGO, init_symlink); if (!err) { dentry->d_op = &sysfs_dentry_ops; @@ -416,7 +443,7 @@ static int sysfs_readdir(struct file * f switch (i) { case 0: - ino = dentry->d_inode->i_ino; + ino = parent_sd->s_ino; if (filldir(dirent, ".", 1, i, ino, DT_DIR) < 0) break; filp->f_pos++; @@ -445,10 +472,7 @@ static int sysfs_readdir(struct file * f name = sysfs_get_name(next); len = strlen(name); - if (next->s_dentry) - ino = next->s_dentry->d_inode->i_ino; - else - ino = iunique(sysfs_sb, 2); + ino = next->s_ino; if (filldir(dirent, name, len, filp->f_pos, ino, dt_type(next)) < 0) Index: pardus-2.6/fs/sysfs/inode.c =================================================================== --- pardus-2.6.orig/fs/sysfs/inode.c +++ pardus-2.6/fs/sysfs/inode.c @@ -128,6 +128,7 @@ struct inode * sysfs_new_inode(mode_t mo inode->i_blocks = 0; inode->i_mapping->a_ops = &sysfs_aops; inode->i_mapping->backing_dev_info = &sysfs_backing_dev_info; + inode->i_ino = sd->s_ino; inode->i_op = &sysfs_inode_operations; lockdep_set_class(&inode->i_mutex, &sysfs_inode_imutex_key); @@ -216,12 +217,25 @@ const unsigned char * sysfs_get_name(str */ void sysfs_drop_dentry(struct sysfs_dirent * sd, struct dentry * parent) { - struct dentry * dentry = sd->s_dentry; + struct dentry *dentry = NULL; + + /* We're not holding a reference to ->s_dentry dentry but the + * field will stay valid as long as sysfs_lock is held. + */ + spin_lock(&sysfs_lock); + spin_lock(&dcache_lock); + + /* dget dentry if it's still alive */ + if (sd->s_dentry && sd->s_dentry->d_inode) + dentry = dget_locked(sd->s_dentry); + + spin_unlock(&dcache_lock); + spin_unlock(&sysfs_lock); if (dentry) { spin_lock(&dcache_lock); spin_lock(&dentry->d_lock); - if (!(d_unhashed(dentry) && dentry->d_inode)) { + if (!d_unhashed(dentry) && dentry->d_inode) { dget_locked(dentry); __d_drop(dentry); spin_unlock(&dentry->d_lock); @@ -231,6 +245,8 @@ void sysfs_drop_dentry(struct sysfs_dire spin_unlock(&dentry->d_lock); spin_unlock(&dcache_lock); } + + dput(dentry); } } Index: pardus-2.6/fs/sysfs/mount.c =================================================================== --- pardus-2.6.orig/fs/sysfs/mount.c +++ pardus-2.6/fs/sysfs/mount.c @@ -29,6 +29,7 @@ static struct sysfs_dirent sysfs_root = .s_element = NULL, .s_type = SYSFS_ROOT, .s_iattr = NULL, + .s_ino = 1, }; static int sysfs_fill_super(struct super_block *sb, void *data, int silent) Index: pardus-2.6/fs/sysfs/sysfs.h =================================================================== --- pardus-2.6.orig/fs/sysfs/sysfs.h +++ pardus-2.6/fs/sysfs/sysfs.h @@ -20,6 +20,7 @@ extern const unsigned char * sysfs_get_n extern void sysfs_drop_dentry(struct sysfs_dirent *sd, struct dentry *parent); extern int sysfs_setattr(struct dentry *dentry, struct iattr *iattr); +extern spinlock_t sysfs_lock; extern struct rw_semaphore sysfs_rename_sem; extern struct super_block * sysfs_sb; extern const struct file_operations sysfs_dir_operations; Index: pardus-2.6/include/linux/sysfs.h =================================================================== --- pardus-2.6.orig/include/linux/sysfs.h +++ pardus-2.6/include/linux/sysfs.h @@ -72,6 +72,7 @@ struct sysfs_dirent { void * s_element; int s_type; umode_t s_mode; + ino_t s_ino; struct dentry * s_dentry; struct iattr * s_iattr; atomic_t s_event;