linux-kernel - Re: [PATCH 25/27] ipc: Convert mqueue fs to fs

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:   Thu, 15 Jun 2017 11:07:14 +0100
From:   Al Viro <viro@...IV.linux.org.uk>
To:     David Howells <dhowells@...hat.com>
Cc:     mszeredi@...hat.com, linux-nfs@...r.kernel.org, jlayton@...hat.com,
        linux-kernel@...r.kernel.org,
        linux-security-module@...r.kernel.org,
        linux-fsdevel@...r.kernel.org
Subject: Re: [PATCH 25/27] ipc: Convert mqueue fs to fs_context [ver #5]

On Wed, Jun 14, 2017 at 04:19:03PM +0100, David Howells wrote:
> Convert the mqueue filesystem to use the filesystem context stuff.
> 
> Notes:
> 
>  (1) The relevant ipc namespace is selected in when the context is
>      initialised (and it defaults to the current task's ipc namespace).
>      The caller can override this before calling vfs_get_tree().
> 
>  (2) Rather than simply calling kern_mount_data(), mq_init_ns() creates a
>      context, adjusts it and then does the rest of the mount procedure.

> +	fc = vfs_new_fs_context(&mqueue_fs_type, NULL, 0, FS_CONTEXT_FOR_NEW);
> +	if (IS_ERR(fc))
> +		return PTR_ERR(fc);
> +
> +	ctx = container_of(fc, struct mqueue_fs_context, fc);

Ugh...

> +	if (ctx->ipc_ns != ns) {

How could they possibly be equal?  You are setting that ns up here, right?  How
could it be in any process' nsproxy?

> +		put_ipc_ns(ctx->ipc_ns);
> +		ctx->ipc_ns = get_ipc_ns(ns);

> +	mnt = kern_mount_data_fc(fc);
> +	if (IS_ERR(mnt)) {
> +		ret = PTR_ERR(mnt);
> +		goto out_fc;
> +	}
> +
> +	ns->mq_mnt = mnt;
> +	ret = 0;
> +out_fc:
> +	put_fs_context(fc);

Am I right assuming that it's an attempt to cope with "one process starts
setting the things up, passes descriptor to another and buggers off; the
recepient mounts the damn thing and needs something to hold your reference"?

Ugh, again...  Is there any reason for dynamic allocation of that thing in
this particular case?  AFAICS, these contortions are all due to going through
vfs_new_fs_context()/put_fs_context().  And it's not as if they had been
refcounted...