linux-kernel - Re: [patch 00/13] devtmpfs patches

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:	Wed, 13 May 2009 14:20:10 +0100
From:	David Howells <dhowells@...hat.com>
To:	Kay Sievers <kay.sievers@...y.org>
Cc:	dhowells@...hat.com, Stephen Smalley <sds@...ho.nsa.gov>,
	"David P. Quigley" <dpquigl@...ho.nsa.gov>,
	Greg KH <greg@...ah.com>, linux-kernel@...r.kernel.org,
	Greg KH <gregkh@...e.de>, Jan Blunck <jblunck@...e.de>,
	James Morris <jmorris@...ei.org>,
	Eric Paris <eparis@...isplace.org>
Subject: Re: [patch 00/13] devtmpfs patches

Kay Sievers <kay.sievers@...y.org> wrote:

> +static struct cred *kern_cred;

Can I suggest that you call your cred pointer dev_cred rather than kern_cred
so that the naming is consistent with the other globals variables?

> +	kern_cred = prepare_kernel_cred(NULL);

If you have no intention of altering the credentials you create, you might
want to use &init_cred instead of kern_cred.  That said, you might want to
allocate it and let the security module alter it before you use it.

Also, Stephen is right, you should probably wrap all your accesses to the VFS
in your devtmpfs credentials.  For instance, devtmpfs_create_node() calls
vfs_mkdir() with the process's credentials via create_path() and directly with
the kern_cred.

What you probably want is:

	int devtmpfs_create_node(struct device *dev)
	{
		const struct cred *curr_cred;
		const char *tmp = NULL;
		const char *nodename;
		mode_t mode;
		struct nameidata nd;
		struct dentry *dentry;
		int err;

		if (!dev_mnt)
			return 0;

		nodename = device_get_nodename(dev, &tmp);
		if (!nodename)
			return -ENOMEM;

		curr_cred = override_creds(kern_cred);

		if (is_blockdev(dev))
			mode = S_IFBLK|0600;
		else
			mode = S_IFCHR|0600;

		err = vfs_path_lookup(dev_mnt->mnt_root, dev_mnt,
				      nodename, LOOKUP_PARENT, &nd);
		if (err == -ENOENT) {
			/* create missing parent directories */
			create_path(nodename);
			err = vfs_path_lookup(dev_mnt->mnt_root, dev_mnt,
					      nodename, LOOKUP_PARENT, &nd);
			if (err)
				goto out_name;
		}

		dentry = lookup_create(&nd, 0);
		if (!IS_ERR(dentry)) {
			err = vfs_mknod(nd.path.dentry->d_inode,
					dentry, mode, dev->devt);
			/* mark as kernel created inode */
			if (!err)
				dentry->d_inode->i_private = &dev_mnt;
			dput(dentry);
		} else {
			err = PTR_ERR(dentry);
		}
		mutex_unlock(&nd.path.dentry->d_inode->i_mutex);

		path_put(&nd.path);
	out_name:
		revert_creds(curr_cred);
		kfree(tmp);
		return err;
	}

David
--
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/