lkml.org 
[lkml]   [2009]   [Sep]   [17]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [RFC][PATCH][bugfix] more checks for negative f_pos handling v3
    On Thu, 17 Sep 2009 14:21:24 +0800
    Wu Fengguang <fengguang.wu@intel.com> wrote:

    > On Thu, Sep 17, 2009 at 02:07:26PM +0800, KAMEZAWA Hiroyuki wrote:
    > > Very sorry...reflesh miss.
    > > ==
    > > From: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
    > >
    > > Now, rw_verify_area() checsk f_pos is negative or not. And if
    > > negative, returns -EINVAL.
    > >
    > > But, some special files as /dev/(k)mem and /proc/<pid>/mem etc..
    > > has negative offsets. And we can't do any access via read/write
    > > to the file(device).
    > >
    > > This patch introduce a flag S_VERYBIG and allow negative file
    > > offsets.
    > >
    > > Changelog:
    > > - fixed bug in rw_verify_area (it cannot be compiled)
    > >
    > > Signed-off-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
    > > ---
    > > drivers/char/mem.c | 23 +++++++++++++----------
    > > fs/proc/base.c | 2 ++
    > > fs/read_write.c | 20 ++++++++++++++++++--
    > > include/linux/fs.h | 2 ++
    > > 4 files changed, 35 insertions(+), 12 deletions(-)
    > >
    > > Index: mmotm-2.6.31-Sep14/fs/read_write.c
    > > ===================================================================
    > > --- mmotm-2.6.31-Sep14.orig/fs/read_write.c
    > > +++ mmotm-2.6.31-Sep14/fs/read_write.c
    > > @@ -205,6 +205,16 @@ bad:
    > > }
    > > #endif
    > >
    > > +static bool
    > > +__negative_fpos_check(struct inode *inode, loff_t pos, size_t count)
    > > +{
    > > + if (pos + count < pos)
    > > + return -EOVERFLOW;
    > > + if (IS_VERYBIG(inode))
    > > + return 0;
    > > + return -EINVAL;
    >
    > This always return negative values for !IS_VERYBIG inodes,
    > looks weird at least.
    >
    I need to refresh my eyes, too...
    Thanks, will fix.. return value is not bool, but int...




    > > +}
    > > +
    > > /*
    > > * rw_verify_area doesn't like huge counts. We limit
    > > * them to something that fits in "int" so that others
    > > @@ -222,8 +232,11 @@ int rw_verify_area(int read_write, struc
    > > if (unlikely((ssize_t) count < 0))
    > > return retval;
    > > pos = *ppos;
    > > - if (unlikely((pos < 0) || (loff_t) (pos + count) < 0))
    > > - return retval;
    > > + if (unlikely((pos < 0) || (loff_t) (pos + count) < 0)) {
    > > + retval = __negative_fpos_check(inode, pos, count);
    > > + if (retval)
    > > + return retval;
    > > + }
    > >
    > > if (unlikely(inode->i_flock && mandatory_lock(inode))) {
    > > retval = locks_mandatory_area(
    > > Index: mmotm-2.6.31-Sep14/include/linux/fs.h
    > > ===================================================================
    > > --- mmotm-2.6.31-Sep14.orig/include/linux/fs.h
    > > +++ mmotm-2.6.31-Sep14/include/linux/fs.h
    > > @@ -231,6 +231,7 @@ struct inodes_stat_t {
    > > #define S_NOCMTIME 128 /* Do not update file c/mtime */
    > > #define S_SWAPFILE 256 /* Do not truncate: swapon got its bmaps */
    > > #define S_PRIVATE 512 /* Inode is fs-internal */
    > > +#define S_VERYBIG 1024 /* Allow file's loff_t can be negative */
    > >
    > > /*
    > > * Note that nosuid etc flags are inode-specific: setting some file-system
    > > @@ -265,6 +266,7 @@ struct inodes_stat_t {
    > > #define IS_NOCMTIME(inode) ((inode)->i_flags & S_NOCMTIME)
    > > #define IS_SWAPFILE(inode) ((inode)->i_flags & S_SWAPFILE)
    > > #define IS_PRIVATE(inode) ((inode)->i_flags & S_PRIVATE)
    > > +#define IS_VERYBIG(inode) ((inode)->i_flags & S_VERYBIG)
    > >
    > > /* the read-only stuff doesn't really belong here, but any other place is
    > > probably as bad and I don't want to create yet another include file. */
    > > Index: mmotm-2.6.31-Sep14/drivers/char/mem.c
    > > ===================================================================
    > > --- mmotm-2.6.31-Sep14.orig/drivers/char/mem.c
    > > +++ mmotm-2.6.31-Sep14/drivers/char/mem.c
    > > @@ -825,22 +825,23 @@ static const struct memdev {
    > > const char *name;
    > > const struct file_operations *fops;
    > > struct backing_dev_info *dev_info;
    > > + bool verybig;
    > > } devlist[] = {
    > > - [ 1] = { "mem", &mem_fops, &directly_mappable_cdev_bdi },
    > > + [1] = { "mem", &mem_fops, &directly_mappable_cdev_bdi, true },
    > > #ifdef CONFIG_DEVKMEM
    > > - [ 2] = { "kmem", &kmem_fops, &directly_mappable_cdev_bdi },
    > > + [2] = { "kmem", &kmem_fops, &directly_mappable_cdev_bdi, true },
    > > #endif
    > > - [ 3] = {"null", &null_fops, NULL },
    > > + [3] = {"null", &null_fops, NULL, false },
    > > #ifdef CONFIG_DEVPORT
    > > - [ 4] = { "port", &port_fops, NULL },
    > > + [4] = { "port", &port_fops, NULL, false },
    > > #endif
    > > - [ 5] = { "zero", &zero_fops, &zero_bdi },
    > > - [ 6] = { "full", &full_fops, NULL },
    > > - [ 7] = { "random", &random_fops, NULL },
    > > - [ 9] = { "urandom", &urandom_fops, NULL },
    > > - [11] = { "kmsg", &kmsg_fops, NULL },
    > > + [5] = { "zero", &zero_fops, &zero_bdi, false },
    > > + [6] = { "full", &full_fops, NULL, false },
    > > + [7] = { "random", &random_fops, NULL, false },
    > > + [9] = { "urandom", &urandom_fops, NULL, false },
    > > + [11] = { "kmsg", &kmsg_fops, NULL, false },
    > > #ifdef CONFIG_CRASH_DUMP
    > > - [12] = { "oldmem", &oldmem_fops, NULL },
    > > + [12] = { "oldmem", &oldmem_fops, NULL, false },
    > > #endif
    >
    > How about make the fields aligned? That would look much nicer :)
    >
    checkpatch.pl complains agaisnt [ 5]...but ok, ajust "=" placement.

    Thanks,
    -kame


    > Thanks,
    > Fengguang
    >
    > > };
    > >
    > > @@ -868,6 +869,8 @@ static int memory_open(struct inode *ino
    > > ret = dev->fops->open(inode, filp);
    > > else
    > > ret = 0;
    > > + if (dev->verybig)
    > > + inode->i_flags |= S_VERYBIG;
    > > out:
    > > unlock_kernel();
    > > return ret;
    > > Index: mmotm-2.6.31-Sep14/fs/proc/base.c
    > > ===================================================================
    > > --- mmotm-2.6.31-Sep14.orig/fs/proc/base.c
    > > +++ mmotm-2.6.31-Sep14/fs/proc/base.c
    > > @@ -779,6 +779,8 @@ static const struct file_operations proc
    > > static int mem_open(struct inode* inode, struct file* file)
    > > {
    > > file->private_data = (void*)((long)current->self_exec_id);
    > > + /* this file is read only and we can catch out-pf-range */
    > > + inode->i_flags |= S_VERYBIG;
    > > return 0;
    > > }
    > >
    >



    \
     
     \ /
      Last update: 2009-09-17 08:35    [W:5.065 / U:0.784 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site