Commit 8297b790 authored by Linus Torvalds's avatar Linus Torvalds
Browse files

Merge tag 'pull-securityfs' of git://git.kernel.org/pub/scm/linux/kernel/git/viro/vfs 

Pull securityfs updates from Al Viro:
 "Securityfs cleanups and fixes:

   - one extra reference is enough to pin a dentry down; no need for
     two. Switch to regular scheme, similar to shmem, debugfs, etc. This
     fixes a securityfs_recursive_remove() dentry leak, among other
     things.

   - we need to have the filesystem pinned to prevent the contents
     disappearing; what we do not need is pinning it for each file.
     Doing that only for files and directories in the root is enough.

   - the previous two changes allow us to get rid of the racy kludges in
     efi_secret_unlink(), where we can use simple_unlink() instead of
     securityfs_remove(). Which does not require unlocking and relocking
     the parent, with all deadlocks that invites.

   - Make securityfs_remove() take the entire subtree out, turning
     securityfs_recursive_remove() into its alias. Makes a lot more
     sense for callers and fixes a mount leak, while we are at it.

   - Making securityfs_remove() remove the entire subtree allows for
     much simpler life in most of the users - efi_secret, ima_fs, evm,
     ipe, tmp get cleaner. I hadn't touched apparmor use of securityfs,
     but I suspect that it would be useful there as well"

* tag 'pull-securityfs' of git://git.kernel.org/pub/scm/linux/kernel/git/viro/vfs:
  tpm: don't bother with removal of files in directory we'll be removing
  ipe: don't bother with removal of files in directory we'll be removing
  evm_secfs: clear securityfs interactions
  ima_fs: get rid of lookup-by-dentry stuff
  ima_fs: don't bother with removal of files in directory we'll be removing
  efi_secret: clean securityfs use up
  make securityfs_remove() remove the entire subtree
  fix locking in efi_secret_unlink()
  securityfs: pin filesystem only for objects directly in root
  securityfs: don't pin dentries twice, once is enough...
parents ddf52f12 f42b8d78

drivers/char/tpm/eventlog/common.c

+12 −34
Original line number Diff line number Diff line
@@ -32,7 +32,7 @@ static int tpm_bios_measurements_open(struct inode *inode, 
	struct tpm_chip *chip;



	inode_lock(inode);

	if (!inode->i_private) {

	if (!inode->i_nlink) {

		inode_unlock(inode);

		return -ENODEV;

	}
@@ -105,7 +105,7 @@ static int tpm_read_log(struct tpm_chip *chip) 
void tpm_bios_log_setup(struct tpm_chip *chip)

{

	const char *name = dev_name(&chip->dev);

	unsigned int cnt;

	struct dentry *dentry;

	int log_version;

	int rc = 0;
@@ -117,14 +117,12 @@ void tpm_bios_log_setup(struct tpm_chip *chip) 
		return;

	log_version = rc;



	cnt = 0;

	chip->bios_dir[cnt] = securityfs_create_dir(name, NULL);

	chip->bios_dir = securityfs_create_dir(name, NULL);

	/* NOTE: securityfs_create_dir can return ENODEV if securityfs is

	 * compiled out. The caller should ignore the ENODEV return code.

	 */

	if (IS_ERR(chip->bios_dir[cnt]))

		goto err;

	cnt++;

	if (IS_ERR(chip->bios_dir))

		return;



	chip->bin_log_seqops.chip = chip;

	if (log_version == EFI_TCG2_EVENT_LOG_FORMAT_TCG_2)
@@ -135,14 +133,13 @@ void tpm_bios_log_setup(struct tpm_chip *chip) 
			&tpm1_binary_b_measurements_seqops;





	chip->bios_dir[cnt] =

	dentry =

	    securityfs_create_file("binary_bios_measurements",

				   0440, chip->bios_dir[0],

				   0440, chip->bios_dir,

				   (void *)&chip->bin_log_seqops,

				   &tpm_bios_measurements_ops);

	if (IS_ERR(chip->bios_dir[cnt]))

	if (IS_ERR(dentry))

		goto err;

	cnt++;



	if (!(chip->flags & TPM_CHIP_FLAG_TPM2)) {
@@ -150,42 +147,23 @@ void tpm_bios_log_setup(struct tpm_chip *chip) 
		chip->ascii_log_seqops.seqops =

			&tpm1_ascii_b_measurements_seqops;



		chip->bios_dir[cnt] =

		dentry =

			securityfs_create_file("ascii_bios_measurements",

					       0440, chip->bios_dir[0],

					       0440, chip->bios_dir,

					       (void *)&chip->ascii_log_seqops,

					       &tpm_bios_measurements_ops);

		if (IS_ERR(chip->bios_dir[cnt]))

		if (IS_ERR(dentry))

			goto err;

		cnt++;

	}



	return;



err:

	chip->bios_dir[cnt] = NULL;

	tpm_bios_log_teardown(chip);

	return;

}



void tpm_bios_log_teardown(struct tpm_chip *chip)

{

	int i;

	struct inode *inode;



	/* securityfs_remove currently doesn't take care of handling sync

	 * between removal and opening of pseudo files. To handle this, a

	 * workaround is added by making i_private = NULL here during removal

	 * and to check it during open(), both within inode_lock()/unlock().

	 * This design ensures that open() either safely gets kref or fails.

	 */

	for (i = (TPM_NUM_EVENT_LOG_FILES - 1); i >= 0; i--) {

		if (chip->bios_dir[i]) {

			inode = d_inode(chip->bios_dir[i]);

			inode_lock(inode);

			inode->i_private = NULL;

			inode_unlock(inode);

			securityfs_remove(chip->bios_dir[i]);

		}

	}

	securityfs_remove(chip->bios_dir);

}

drivers/virt/coco/efi_secret/efi_secret.c

+9 −38
Original line number Diff line number Diff line
@@ -31,8 +31,6 @@ 


struct efi_secret {

	struct dentry *secrets_dir;

	struct dentry *fs_dir;

	struct dentry *fs_files[EFI_SECRET_NUM_FILES];

	void __iomem *secret_data;

	u64 secret_data_len;

};
@@ -119,10 +117,8 @@ static void wipe_memory(void *addr, size_t size) 


static int efi_secret_unlink(struct inode *dir, struct dentry *dentry)

{

	struct efi_secret *s = efi_secret_get();

	struct inode *inode = d_inode(dentry);

	struct secret_entry *e = (struct secret_entry *)inode->i_private;

	int i;



	if (e) {

		/* Zero out the secret data */
@@ -132,19 +128,7 @@ static int efi_secret_unlink(struct inode *dir, struct dentry *dentry) 


	inode->i_private = NULL;



	for (i = 0; i < EFI_SECRET_NUM_FILES; i++)

		if (s->fs_files[i] == dentry)

			s->fs_files[i] = NULL;



	/*

	 * securityfs_remove tries to lock the directory's inode, but we reach

	 * the unlink callback when it's already locked

	 */

	inode_unlock(dir);

	securityfs_remove(dentry);

	inode_lock(dir);



	return 0;

	return simple_unlink(inode, dentry);

}



static const struct inode_operations efi_secret_dir_inode_operations = {
@@ -194,15 +178,6 @@ static int efi_secret_map_area(struct platform_device *dev) 
static void efi_secret_securityfs_teardown(struct platform_device *dev)

{

	struct efi_secret *s = efi_secret_get();

	int i;



	for (i = (EFI_SECRET_NUM_FILES - 1); i >= 0; i--) {

		securityfs_remove(s->fs_files[i]);

		s->fs_files[i] = NULL;

	}



	securityfs_remove(s->fs_dir);

	s->fs_dir = NULL;



	securityfs_remove(s->secrets_dir);

	s->secrets_dir = NULL;
@@ -217,7 +192,7 @@ static int efi_secret_securityfs_setup(struct platform_device *dev) 
	unsigned char *ptr;

	struct secret_header *h;

	struct secret_entry *e;

	struct dentry *dent;

	struct dentry *dent, *dir;

	char guid_str[EFI_VARIABLE_GUID_LEN + 1];



	ptr = (void __force *)s->secret_data;
@@ -240,8 +215,6 @@ static int efi_secret_securityfs_setup(struct platform_device *dev) 
	}



	s->secrets_dir = NULL;

	s->fs_dir = NULL;

	memset(s->fs_files, 0, sizeof(s->fs_files));



	dent = securityfs_create_dir("secrets", NULL);

	if (IS_ERR(dent)) {
@@ -251,14 +224,13 @@ static int efi_secret_securityfs_setup(struct platform_device *dev) 
	}

	s->secrets_dir = dent;



	dent = securityfs_create_dir("coco", s->secrets_dir);

	if (IS_ERR(dent)) {

	dir = securityfs_create_dir("coco", s->secrets_dir);

	if (IS_ERR(dir)) {

		dev_err(&dev->dev, "Error creating coco securityfs directory entry err=%ld\n",

			PTR_ERR(dent));

		return PTR_ERR(dent);

			PTR_ERR(dir));

		return PTR_ERR(dir);

	}

	d_inode(dent)->i_op = &efi_secret_dir_inode_operations;

	s->fs_dir = dent;

	d_inode(dir)->i_op = &efi_secret_dir_inode_operations;



	bytes_left = h->len - sizeof(*h);

	ptr += sizeof(*h);
@@ -274,15 +246,14 @@ static int efi_secret_securityfs_setup(struct platform_device *dev) 
		if (efi_guidcmp(e->guid, NULL_GUID)) {

			efi_guid_to_str(&e->guid, guid_str);



			dent = securityfs_create_file(guid_str, 0440, s->fs_dir, (void *)e,

			dent = securityfs_create_file(guid_str, 0440, dir, (void *)e,

						      &efi_secret_bin_file_fops);

			if (IS_ERR(dent)) {

				dev_err(&dev->dev, "Error creating efi_secret securityfs entry\n");

				ret = PTR_ERR(dent);

				goto err_cleanup;

			}



			s->fs_files[i++] = dent;

			i++;

		}

		ptr += e->len;

		bytes_left -= e->len;

include/linux/security.h

+2 −1
Original line number Diff line number Diff line
@@ -2211,7 +2211,6 @@ struct dentry *securityfs_create_symlink(const char *name, 
					 const char *target,

					 const struct inode_operations *iops);

extern void securityfs_remove(struct dentry *dentry);

extern void securityfs_recursive_remove(struct dentry *dentry);



#else /* CONFIG_SECURITYFS */
@@ -2243,6 +2242,8 @@ static inline void securityfs_remove(struct dentry *dentry) 


#endif



#define securityfs_recursive_remove securityfs_remove



#ifdef CONFIG_BPF_SYSCALL

union bpf_attr;

struct bpf_map;

include/linux/tpm.h

+1 −1
Original line number Diff line number Diff line
@@ -182,7 +182,7 @@ struct tpm_chip { 
	unsigned long duration[TPM_NUM_DURATIONS]; /* jiffies */

	bool duration_adjusted;



	struct dentry *bios_dir[TPM_NUM_EVENT_LOG_FILES];

	struct dentry *bios_dir;



	const struct attribute_group *groups[3 + TPM_MAX_HASHES];

	unsigned int groups_cnt;

security/inode.c

+19 −43
Original line number Diff line number Diff line
@@ -112,18 +112,20 @@ static struct dentry *securityfs_create_dentry(const char *name, umode_t mode, 
	struct dentry *dentry;

	struct inode *dir, *inode;

	int error;

	bool pinned = false;



	if (!(mode & S_IFMT))

		mode = (mode & S_IALLUGO) | S_IFREG;



	pr_debug("securityfs: creating file '%s'\n",name);



	if (!parent) {

		error = simple_pin_fs(&fs_type, &mount, &mount_count);

		if (error)

			return ERR_PTR(error);



	if (!parent)

		pinned = true;

		parent = mount->mnt_root;

	}



	dir = d_inode(parent);
@@ -159,7 +161,6 @@ static struct dentry *securityfs_create_dentry(const char *name, umode_t mode, 
		inode->i_fop = fops;

	}

	d_instantiate(dentry, inode);

	dget(dentry);

	inode_unlock(dir);

	return dentry;
@@ -168,6 +169,7 @@ static struct dentry *securityfs_create_dentry(const char *name, umode_t mode, 
	dentry = ERR_PTR(error);

out:

	inode_unlock(dir);

	if (pinned)

		simple_release_fs(&mount, &mount_count);

	return dentry;

}
@@ -279,6 +281,12 @@ struct dentry *securityfs_create_symlink(const char *name, 
}

EXPORT_SYMBOL_GPL(securityfs_create_symlink);



static void remove_one(struct dentry *victim)

{

	if (victim->d_parent == victim->d_sb->s_root)

		simple_release_fs(&mount, &mount_count);

}



/**

 * securityfs_remove - removes a file or directory from the securityfs filesystem

 *
@@ -291,43 +299,11 @@ EXPORT_SYMBOL_GPL(securityfs_create_symlink); 
 * This function is required to be called in order for the file to be

 * removed. No automatic cleanup of files will happen when a module is

 * removed; you are responsible here.

 */

void securityfs_remove(struct dentry *dentry)

{

	struct inode *dir;



	if (IS_ERR_OR_NULL(dentry))

		return;



	dir = d_inode(dentry->d_parent);

	inode_lock(dir);

	if (simple_positive(dentry)) {

		if (d_is_dir(dentry))

			simple_rmdir(dir, dentry);

		else

			simple_unlink(dir, dentry);

		dput(dentry);

	}

	inode_unlock(dir);

	simple_release_fs(&mount, &mount_count);

}

EXPORT_SYMBOL_GPL(securityfs_remove);



static void remove_one(struct dentry *victim)

{

	simple_release_fs(&mount, &mount_count);

}



/**

 * securityfs_recursive_remove - recursively removes a file or directory

 *

 * @dentry: a pointer to a the dentry of the file or directory to be removed.

 *

 * This function recursively removes a file or directory in securityfs that was

 * previously created with a call to another securityfs function (like

 * securityfs_create_file() or variants thereof.)

 * AV: when applied to directory it will take all children out; no need to call

 * it for descendents if ancestor is getting killed.

 */

void securityfs_recursive_remove(struct dentry *dentry)

void securityfs_remove(struct dentry *dentry)

{

	if (IS_ERR_OR_NULL(dentry))

		return;
@@ -336,7 +312,7 @@ void securityfs_recursive_remove(struct dentry *dentry) 
	simple_recursive_removal(dentry, remove_one);

	simple_release_fs(&mount, &mount_count);

}

EXPORT_SYMBOL_GPL(securityfs_recursive_remove);

EXPORT_SYMBOL_GPL(securityfs_remove);



#ifdef CONFIG_SECURITY

static struct dentry *lsm_dentry;