Re: [PATCH v3 02/11] lsm: /proc/$PID/attr/label_map file and getprocattr_seq hook

[Lukasz Pawelczyk <l.pawelczyk@samsung.com>]

On pią, 2015-08-21 at 01:14 -0400, Paul Moore wrote:
> On Fri, Jul 24, 2015 at 6:04 AM, Lukasz Pawelczyk
> <l.pawelczyk@samsung.com> wrote:
> > This commit adds a new proc attribute, label_map that is required 
> > by an
> > upcoming Smack namespace. In general it can be used to hold a map 
> > of
> > labels, e.g. to be used in namespaces.
> > 
> > Due to the nature of this file, the standard getprocattr hook might 
> > not
> > be enough to handle it. The map's output can in principle be 
> > greater
> > than page size to which the aforementioned hook is limited.
> > To handle this properly a getprocattr_seq LSM hook has been added 
> > that
> > makes it possible to handle any chosen proc attr by seq operations.
> > 
> > See the documentation in the patch below for the details about how 
> > to
> > use the hook.
> > 
> > Signed-off-by: Lukasz Pawelczyk <l.pawelczyk@samsung.com>
> > ---
> >  fs/proc/base.c            | 81 
> > +++++++++++++++++++++++++++++++++++++++++++----
> >  include/linux/lsm_hooks.h | 15 +++++++++
> >  include/linux/security.h  |  9 ++++++
> >  security/security.c       |  8 +++++
> >  4 files changed, 107 insertions(+), 6 deletions(-)
> > 
> > diff --git a/fs/proc/base.c b/fs/proc/base.c
> > index aa50d1a..e5ac827 100644
> > --- a/fs/proc/base.c
> > +++ b/fs/proc/base.c
> > @@ -2338,20 +2338,77 @@ out:
> >  }
> > 
> >  #ifdef CONFIG_SECURITY
> > +static int proc_pid_attr_open(struct inode *inode, struct file 
> > *file)
> > +{
> > +       const char *name = file->f_path.dentry->d_name.name;
> > +       const struct seq_operations *ops;
> > +       struct task_struct *task;
> > +       struct seq_file *seq;
> > +       int ret;
> > +
> > +       file->private_data = NULL;
> > +
> > +       task = get_proc_task(inode);
> > +       if (!task)
> > +               return -ESRCH;
> > +
> > +       /* don't use seq_ops if they are not provided by LSM */
> > +       ret = security_getprocattr_seq(task, name, &ops);
> > +       if (ret == -EOPNOTSUPP) {
> > +               put_task_struct(task);
> > +               return 0;
> > +       }
> > +       if (ret) {
> > +               put_task_struct(task);
> > +               return ret;
> > +       }
> > +
> > +       ret = seq_open(file, ops);
> > +       if (ret) {
> > +               put_task_struct(task);
> > +               return ret;
> > +       }
> > +
> > +       seq = file->private_data;
> > +       seq->private = task;
> > +
> > +       return 0;
> > +}
> 
> If you end up having to respin this patchset, you might consider
> moving the "put_task_struct(...); return X;" code into a block at the
> end of the function to simplify things a bit, for example:

I will do so, thanks.

> 
> static int proc_pid_attr_open(struct inode *inode, struct file *file)
> {
>        const char *name = file->f_path.dentry->d_name.name;
>        const struct seq_operations *ops;
>        struct task_struct *task;
>        struct seq_file *seq;
>        int ret;
> 
>        file->private_data = NULL;
> 
>        task = get_proc_task(inode);
>        if (!task)
>                return -ESRCH;
> 
>        /* don't use seq_ops if they are not provided by LSM */
>        ret = security_getprocattr_seq(task, name, &ops);
>        if (ret == -EOPNOTSUPP) {
>                ret = 0;
>                goto put_task;
>        }
>        if (ret)
>                goto put_task;
> 
>        ret = seq_open(file, ops);
>        if (ret)
>                goto put_task;
> 
>        seq = file->private_data;
>        seq->private = task;
> 
>        return 0;
> 
> put_task:
>        put_task_struct(task);
>        return ret;
> }
> 
-- 
Lukasz Pawelczyk
Samsung R&D Institute Poland
Samsung Electronics