Commit 0aef2868 authored by Alexander Viro's avatar Alexander Viro Committed by Linus Torvalds

[PATCH] Fix cramfs metadata races

There's a few places that use incorrect exclusion for the cramfs raw
data access buffers.  The proper lock is "read_mutex" (and BKL does
nothing).

 - fix mount-time read and block number initialization without the mutex
   held. 
 - cramfs_readdir() needs to copy the name and inode information into a
   separate buffer since it can't hold the semaphore over the
   (potentially blocking) user mode access
 - cramfs_lookup() needs to hold the access lock over the whole
   function, not just the read itself
 - use generic_file_llseek on directories to get i_sem exclusion on
   readdir/lseek
parent d6594193
...@@ -18,7 +18,6 @@ ...@@ -18,7 +18,6 @@
#include <linux/string.h> #include <linux/string.h>
#include <linux/blkdev.h> #include <linux/blkdev.h>
#include <linux/cramfs_fs.h> #include <linux/cramfs_fs.h>
#include <linux/smp_lock.h>
#include <linux/slab.h> #include <linux/slab.h>
#include <linux/cramfs_fs_sb.h> #include <linux/cramfs_fs_sb.h>
#include <linux/buffer_head.h> #include <linux/buffer_head.h>
...@@ -206,10 +205,10 @@ static int cramfs_fill_super(struct super_block *sb, void *data, int silent) ...@@ -206,10 +205,10 @@ static int cramfs_fill_super(struct super_block *sb, void *data, int silent)
sb_set_blocksize(sb, PAGE_CACHE_SIZE); sb_set_blocksize(sb, PAGE_CACHE_SIZE);
/* Invalidate the read buffers on mount: think disk change.. */ /* Invalidate the read buffers on mount: think disk change.. */
down(&read_mutex);
for (i = 0; i < READ_BUFFERS; i++) for (i = 0; i < READ_BUFFERS; i++)
buffer_blocknr[i] = -1; buffer_blocknr[i] = -1;
down(&read_mutex);
/* Read the first block and get the superblock from it */ /* Read the first block and get the superblock from it */
memcpy(&super, cramfs_read(sb, 0, sizeof(super)), sizeof(super)); memcpy(&super, cramfs_read(sb, 0, sizeof(super)), sizeof(super));
up(&read_mutex); up(&read_mutex);
...@@ -217,7 +216,9 @@ static int cramfs_fill_super(struct super_block *sb, void *data, int silent) ...@@ -217,7 +216,9 @@ static int cramfs_fill_super(struct super_block *sb, void *data, int silent)
/* Do sanity checks on the superblock */ /* Do sanity checks on the superblock */
if (super.magic != CRAMFS_MAGIC) { if (super.magic != CRAMFS_MAGIC) {
/* check at 512 byte offset */ /* check at 512 byte offset */
down(&read_mutex);
memcpy(&super, cramfs_read(sb, 512, sizeof(super)), sizeof(super)); memcpy(&super, cramfs_read(sb, 512, sizeof(super)), sizeof(super));
up(&read_mutex);
if (super.magic != CRAMFS_MAGIC) { if (super.magic != CRAMFS_MAGIC) {
if (!silent) if (!silent)
printk(KERN_ERR "cramfs: wrong magic\n"); printk(KERN_ERR "cramfs: wrong magic\n");
...@@ -288,6 +289,7 @@ static int cramfs_readdir(struct file *filp, void *dirent, filldir_t filldir) ...@@ -288,6 +289,7 @@ static int cramfs_readdir(struct file *filp, void *dirent, filldir_t filldir)
{ {
struct inode *inode = filp->f_dentry->d_inode; struct inode *inode = filp->f_dentry->d_inode;
struct super_block *sb = inode->i_sb; struct super_block *sb = inode->i_sb;
char *buf;
unsigned int offset; unsigned int offset;
int copied; int copied;
...@@ -299,18 +301,21 @@ static int cramfs_readdir(struct file *filp, void *dirent, filldir_t filldir) ...@@ -299,18 +301,21 @@ static int cramfs_readdir(struct file *filp, void *dirent, filldir_t filldir)
if (offset & 3) if (offset & 3)
return -EINVAL; return -EINVAL;
lock_kernel(); buf = kmalloc(256, GFP_KERNEL);
if (!buf)
return -ENOMEM;
copied = 0; copied = 0;
while (offset < inode->i_size) { while (offset < inode->i_size) {
struct cramfs_inode *de; struct cramfs_inode *de;
unsigned long nextoffset; unsigned long nextoffset;
char *name; char *name;
ino_t ino;
mode_t mode;
int namelen, error; int namelen, error;
down(&read_mutex); down(&read_mutex);
de = cramfs_read(sb, OFFSET(inode) + offset, sizeof(*de)+256); de = cramfs_read(sb, OFFSET(inode) + offset, sizeof(*de)+256);
up(&read_mutex);
name = (char *)(de+1); name = (char *)(de+1);
/* /*
...@@ -319,17 +324,21 @@ static int cramfs_readdir(struct file *filp, void *dirent, filldir_t filldir) ...@@ -319,17 +324,21 @@ static int cramfs_readdir(struct file *filp, void *dirent, filldir_t filldir)
* with zeroes. * with zeroes.
*/ */
namelen = de->namelen << 2; namelen = de->namelen << 2;
memcpy(buf, name, namelen);
ino = CRAMINO(de);
mode = de->mode;
up(&read_mutex);
nextoffset = offset + sizeof(*de) + namelen; nextoffset = offset + sizeof(*de) + namelen;
for (;;) { for (;;) {
if (!namelen) { if (!namelen) {
unlock_kernel(); kfree(buf);
return -EIO; return -EIO;
} }
if (name[namelen-1]) if (buf[namelen-1])
break; break;
namelen--; namelen--;
} }
error = filldir(dirent, name, namelen, offset, CRAMINO(de), de->mode >> 12); error = filldir(dirent, buf, namelen, offset, ino, mode >> 12);
if (error) if (error)
break; break;
...@@ -337,7 +346,7 @@ static int cramfs_readdir(struct file *filp, void *dirent, filldir_t filldir) ...@@ -337,7 +346,7 @@ static int cramfs_readdir(struct file *filp, void *dirent, filldir_t filldir)
filp->f_pos = offset; filp->f_pos = offset;
copied++; copied++;
} }
unlock_kernel(); kfree(buf);
return 0; return 0;
} }
...@@ -349,16 +358,14 @@ static struct dentry * cramfs_lookup(struct inode *dir, struct dentry *dentry, s ...@@ -349,16 +358,14 @@ static struct dentry * cramfs_lookup(struct inode *dir, struct dentry *dentry, s
unsigned int offset = 0; unsigned int offset = 0;
int sorted; int sorted;
lock_kernel(); down(&read_mutex);
sorted = CRAMFS_SB(dir->i_sb)->flags & CRAMFS_FLAG_SORTED_DIRS; sorted = CRAMFS_SB(dir->i_sb)->flags & CRAMFS_FLAG_SORTED_DIRS;
while (offset < dir->i_size) { while (offset < dir->i_size) {
struct cramfs_inode *de; struct cramfs_inode *de;
char *name; char *name;
int namelen, retval; int namelen, retval;
down(&read_mutex);
de = cramfs_read(dir->i_sb, OFFSET(dir) + offset, sizeof(*de)+256); de = cramfs_read(dir->i_sb, OFFSET(dir) + offset, sizeof(*de)+256);
up(&read_mutex);
name = (char *)(de+1); name = (char *)(de+1);
/* Try to take advantage of sorted directories */ /* Try to take advantage of sorted directories */
...@@ -374,7 +381,7 @@ static struct dentry * cramfs_lookup(struct inode *dir, struct dentry *dentry, s ...@@ -374,7 +381,7 @@ static struct dentry * cramfs_lookup(struct inode *dir, struct dentry *dentry, s
for (;;) { for (;;) {
if (!namelen) { if (!namelen) {
unlock_kernel(); up(&read_mutex);
return ERR_PTR(-EIO); return ERR_PTR(-EIO);
} }
if (name[namelen-1]) if (name[namelen-1])
...@@ -387,15 +394,16 @@ static struct dentry * cramfs_lookup(struct inode *dir, struct dentry *dentry, s ...@@ -387,15 +394,16 @@ static struct dentry * cramfs_lookup(struct inode *dir, struct dentry *dentry, s
if (retval > 0) if (retval > 0)
continue; continue;
if (!retval) { if (!retval) {
d_add(dentry, get_cramfs_inode(dir->i_sb, de)); struct cramfs_inode entry = *de;
unlock_kernel(); up(&read_mutex);
d_add(dentry, get_cramfs_inode(dir->i_sb, &entry));
return NULL; return NULL;
} }
/* else (retval < 0) */ /* else (retval < 0) */
if (sorted) if (sorted)
break; break;
} }
unlock_kernel(); up(&read_mutex);
d_add(dentry, NULL); d_add(dentry, NULL);
return NULL; return NULL;
} }
...@@ -452,6 +460,7 @@ static struct address_space_operations cramfs_aops = { ...@@ -452,6 +460,7 @@ static struct address_space_operations cramfs_aops = {
* A directory can only readdir * A directory can only readdir
*/ */
static struct file_operations cramfs_directory_operations = { static struct file_operations cramfs_directory_operations = {
.llseek = generic_file_llseek,
.read = generic_read_dir, .read = generic_read_dir,
.readdir = cramfs_readdir, .readdir = cramfs_readdir,
}; };
......
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