Re: [patch 00/13] devtmpfs patches

[David Howells <dhowells@redhat.com>]

Kay Sievers <kay.sievers@vrfy.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