linux-kernel - Re: [PATCH v3 02/11] lsm: /proc/$PID/attr/label_map file and getprocattr

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Fri, 21 Aug 2015 01:14:51 -0400
From:	Paul Moore <paul@...l-moore.com>
To:	Lukasz Pawelczyk <l.pawelczyk@...sung.com>
Cc:	"Eric W. Biederman" <ebiederm@...ssion.com>,
	"Serge E. Hallyn" <serge@...lyn.com>,
	Al Viro <viro@...iv.linux.org.uk>,
	Alexey Dobriyan <adobriyan@...il.com>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Andy Lutomirski <luto@...capital.net>,
	Arnd Bergmann <arnd@...db.de>,
	Casey Schaufler <casey@...aufler-ca.com>,
	David Howells <dhowells@...hat.com>,
	Eric Dumazet <edumazet@...gle.com>,
	Eric Paris <eparis@...isplace.org>,
	Fabian Frederick <fabf@...net.be>,
	Greg KH <gregkh@...uxfoundation.org>,
	James Morris <james.l.morris@...cle.com>,
	Jiri Slaby <jslaby@...e.com>, Joe Perches <joe@...ches.com>,
	John Johansen <john.johansen@...onical.com>,
	Jonathan Corbet <corbet@....net>,
	Kees Cook <keescook@...omium.org>,
	Mauro Carvalho Chehab <mchehab@....samsung.com>,
	NeilBrown <neilb@...e.de>, Oleg Nesterov <oleg@...hat.com>,
	Stephen Smalley <sds@...ho.nsa.gov>,
	Tetsuo Handa <penguin-kernel@...ove.sakura.ne.jp>,
	Zefan Li <lizefan@...wei.com>, linux-doc@...r.kernel.org,
	linux-fsdevel@...r.kernel.org, linux-kernel@...r.kernel.org,
	linux-security-module@...r.kernel.org, selinux@...ho.nsa.gov,
	havner@...il.com
Subject: Re: [PATCH v3 02/11] lsm: /proc/$PID/attr/label_map file and
 getprocattr_seq hook

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

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@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/