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

[Paul Moore]

On Fri, Jul 24, 2015 at 6:04 AM, Lukasz Pawelczyk
<l.pawelczyk@xxxxxxxxxxx> 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@xxxxxxxxxxx>
> ---
>  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:

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;
}

-- 
paul moore
www.paul-moore.com
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/