lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Mon, 28 Apr 2008 11:37:16 -0400 From: Jeff Dike <jdike@...toit.com> To: WANG Cong <xiyou.wangcong@...il.com> Cc: akpm@...l.org, linux-kernel@...r.kernel.org, user-mode-linux-devel@...ts.sourceforge.net Subject: Re: [PATCH 7/19] UML - Move hppfs_kern.c to hppfs.c On Sat, Apr 26, 2008 at 05:17:55PM +0800, WANG Cong wrote: > > +static struct inode *get_inode(struct super_block *, struct dentry *); > > + > > +struct hppfs_data { > > + struct list_head list; > > + char contents[PAGE_SIZE - sizeof(struct list_head)]; > > +}; > > + > > +struct hppfs_private { > > + struct file *proc_file; > > + int host_fd; > > + loff_t len; > > + struct hppfs_data *contents; > > +}; > > + > > +struct hppfs_inode_info { > > + struct dentry *proc_dentry; > > + struct inode vfs_inode; > > +}; > > + > > +static inline struct hppfs_inode_info *HPPFS_I(struct inode *inode) > > +{ > > + return container_of(inode, struct hppfs_inode_info, vfs_inode); > > +} > > + > > +#define HPPFS_SUPER_MAGIC 0xb00000ee > > > These can be put into a single header, e.g. hppfs.h. Why, when this one C file is the only user? > > +static char *dentry_name(struct dentry *dentry, int extra) > > +{ > > + struct dentry *parent; > > + char *root, *name; > > + const char *seg_name; > > + int len, seg_len; > > + > > + len = 0; > > + parent = dentry; > > These can be put into initialization, I think, that's more readable. Yup. > > + if (fd > 0) { > > + os_close_file(fd); > > + return 1; > > + } > > > How about 'fd < 0' ? OK, that would remove the braces at least... > > + new = kmalloc(sizeof(*data), GFP_KERNEL); > > + if (new == 0) { > > > Using NULL is better, at least sparse complains about this. ;) Good, I never liked using 0 (or !ptr) in place of NULL anyway. > > +static int hppfs_filldir(void *d, const char *name, int size, > > + loff_t offset, u64 inode, unsigned int type) > > +{ > > + struct hppfs_dirent *dirent = d; > > + > > + if (file_removed(dirent->dentry, name)) > > + return 0; > > If returning 0 indicates success, this is wrong, since file_removed() may > return -ENOMEM. > > Or I miss something that is too obvious? ;-) Oops, nice spotting. See what you think about the patch below... Jeff -- Work email - jdike at linux dot intel dot com Index: linux-2.6.22/fs/hppfs/hppfs.c =================================================================== --- linux-2.6.22.orig/fs/hppfs/hppfs.c 2008-04-28 11:21:38.000000000 -0400 +++ linux-2.6.22/fs/hppfs/hppfs.c 2008-04-28 11:34:58.000000000 -0400 @@ -64,13 +64,11 @@ static int is_pid(struct dentry *dentry) static char *dentry_name(struct dentry *dentry, int extra) { - struct dentry *parent; - char *root, *name; + struct dentry *parent = dentry; + char *root = "proc", *name; const char *seg_name; - int len, seg_len; + int len = 0, seg_len; - len = 0; - parent = dentry; while (parent->d_parent != parent) { if (is_pid(parent)) len += strlen("pid") + 1; @@ -78,7 +76,6 @@ static char *dentry_name(struct dentry * parent = parent->d_parent; } - root = "proc"; len += strlen(root); name = kmalloc(len + extra + 1, GFP_KERNEL); if (name == NULL) @@ -128,11 +125,11 @@ static int file_removed(struct dentry *d fd = os_open_file(host_file, of_read(OPENFLAGS()), 0); kfree(host_file); - if (fd > 0) { - os_close_file(fd); - return 1; - } - return 0; + if (fd < 0) + return 0; + + os_close_file(fd); + return 1; } static struct dentry *hppfs_lookup(struct inode *ino, struct dentry *dentry, @@ -210,11 +207,10 @@ static ssize_t read_proc(struct file *fi static ssize_t hppfs_read_file(int fd, char __user *buf, ssize_t count) { - ssize_t n; + ssize_t n = -ENOMEM; int cur, err; char *new_buf; - n = -ENOMEM; new_buf = kmalloc(PAGE_SIZE, GFP_KERNEL); if (new_buf == NULL) { printk(KERN_ERR "hppfs_read_file : kmalloc failed\n"); @@ -271,8 +267,10 @@ static ssize_t hppfs_read(struct file *f count = hppfs->len - off; rem = copy_to_user(buf, &data->contents[off], count); *ppos += count - rem; - if (rem > 0) + if (rem == count) return -EFAULT; + else if (rem > 0) + return count - rem; } else if (hppfs->host_fd != -1) { err = os_seek_file(hppfs->host_fd, *ppos); if (err) { @@ -380,7 +378,7 @@ static struct hppfs_data *hppfs_get_data break; new = kmalloc(sizeof(*data), GFP_KERNEL); - if (new == 0) { + if (new == NULL) { printk(KERN_ERR "hppfs_get_data : data allocation " "failed\n"); err = -ENOMEM; @@ -551,8 +549,11 @@ static int hppfs_filldir(void *d, const loff_t offset, u64 inode, unsigned int type) { struct hppfs_dirent *dirent = d; + int gone = file_removed(dirent->dentry, name); - if (file_removed(dirent->dentry, name)) + if (gone < 0) + return gone; + else if (gone) return 0; return (*dirent->filldir)(dirent->vfs_dirent, name, size, offset, -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@...r.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists