lkml.org 
[lkml]   [2009]   [Mar]   [29]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH 3/4] fix setuid sometimes wouldn't
    On Sun, Mar 29, 2009 at 11:48:55PM +0200, Oleg Nesterov wrote:
    > On 03/29, Alexey Dobriyan wrote:
    > >
    > > On Sat, Mar 28, 2009 at 11:21:27PM +0000, Hugh Dickins wrote:
    > > >
    > > > -static struct fs_struct *get_fs_struct(struct task_struct *task)
    > > > +static int get_fs_path(struct task_struct *task, struct path *path, bool root)
    > > > {
    > > > struct fs_struct *fs;
    > > > + int result = -ENOENT;
    > > > +
    > > > task_lock(task);
    > > > fs = task->fs;
    > > > - if(fs)
    > > > - atomic_inc(&fs->count);
    > > > + if (fs) {
    > > > + read_lock(&fs->lock);
    > > > + *path = root ? fs->root : fs->pwd;
    > > > + path_get(path);
    > > > + read_unlock(&fs->lock);
    > > > + result = 0;
    > > > + }
    > > > task_unlock(task);
    > > > - return fs;
    > > > + return result;
    > > > }
    > >
    > > I think it's better to open-code. "root" parameter is unnatural.
    >
    > IMHO, open-coding doesn't improve readability. I am not even talking
    > about code size (it doesn't matter of course, the kernel is so tiny ;).
    >
    > Perhaps "enum what" is more natural compared to "bool root", but this
    > helper is simple enough.
    >
    > Personally, I think this patch makes sense even if it didn't fix the
    > bug, it cleanups the code and makes it more readable.

    Aye. Applied, with enum, but an anonymous one; no point breeding
    tags without a reason.

    Another note: chroot_fs_refs() shouldn't bump refcount at all; it should
    do replacements, count them and then do path_release(old_root) that many
    times - after having gone through all tasks.

    I'm taking fs_struct handling into a new file (fs/fs_struct.c, unless
    somebody has a better name), will do that in the same patch.


    \
     
     \ /
      Last update: 2009-03-30 00:41    [W:0.022 / U:1.076 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site