[RFC,PATCH] efivarfs: Don't delete efivar_entry structures on unlink

[RFC,PATCH] efivarfs: Don't delete efivar_entry structures on unlink

[Jeremy Kerr]

We can cause an oops by unlinking an open efivarfs file, then reading
(or writing) from the deleted file. The unlink operation causes the
efivarfs_entry struct to be freed, which is referenced again in the
read.

This change allow the efivar_entry structures to remain present after
the variable has been removed, but flagged as not present in firmware by
way of having an empty ->list member. We keep a refcount to determine
when the entry can actually be freed.

Signed-off-by: Jeremy Kerr <jk-mnsaURCQ41sdnm+yROfE0A@public.gmane.org>

---
 drivers/firmware/efivars.c |  105 +++++++++++++++++++++++++++++++------
 1 file changed, 89 insertions(+), 16 deletions(-)

diff --git a/drivers/firmware/efivars.c b/drivers/firmware/efivars.c
index 053f28b..764f548 100644
--- a/drivers/firmware/efivars.c
+++ b/drivers/firmware/efivars.c
@@ -121,6 +121,15 @@ struct efi_variable {
 struct efivar_entry {
 	struct efivars *efivars;
 	struct efi_variable var;
+
+	/* references to the kref are held by:
+	 *  - open efivarfs files
+	 *  - efivarfs inodes
+	 *  - the kobj
+	 *  - presence in the global efivar list
+	 */
+	struct kref kref;
+
 	struct list_head list;
 	struct kobject kobj;
 };
@@ -147,7 +156,8 @@ struct efivar_attribute efivar_attr_##_name = { \
 };
 
 #define to_efivar_attr(_attr) container_of(_attr, struct efivar_attribute, attr)
-#define to_efivar_entry(obj)  container_of(obj, struct efivar_entry, kobj)
+#define kobj_to_efivar_entry(obj)  container_of(obj, struct efivar_entry, kobj)
+#define kref_to_efivar_entry(obj)  container_of(obj, struct efivar_entry, kref)
 
 /*
  * Prototype for sysfs creation function
@@ -580,7 +590,7 @@ efivar_show_raw(struct efivar_entry *entry, char *buf)
 static ssize_t efivar_attr_show(struct kobject *kobj, struct attribute *attr,
 				char *buf)
 {
-	struct efivar_entry *var = to_efivar_entry(kobj);
+	struct efivar_entry *var = kobj_to_efivar_entry(kobj);
 	struct efivar_attribute *efivar_attr = to_efivar_attr(attr);
 	ssize_t ret = -EIO;
 
@@ -596,7 +606,7 @@ static ssize_t efivar_attr_show(struct kobject *kobj, struct attribute *attr,
 static ssize_t efivar_attr_store(struct kobject *kobj, struct attribute *attr,
 				const char *buf, size_t count)
 {
-	struct efivar_entry *var = to_efivar_entry(kobj);
+	struct efivar_entry *var = kobj_to_efivar_entry(kobj);
 	struct efivar_attribute *efivar_attr = to_efivar_attr(attr);
 	ssize_t ret = -EIO;
 
@@ -614,12 +624,28 @@ static const struct sysfs_ops efivar_attr_ops = {
 	.store = efivar_attr_store,
 };
 
-static void efivar_release(struct kobject *kobj)
+static void efivar_entry_release(struct kref *kref)
 {
-	struct efivar_entry *var = container_of(kobj, struct efivar_entry, kobj);
+	struct efivar_entry *var = kref_to_efivar_entry(kref);
 	kfree(var);
 }
 
+static void efivar_entry_put(struct efivar_entry *entry)
+{
+	kref_put(&entry->kref, efivar_entry_release);
+}
+
+static void efivar_entry_get(struct efivar_entry *entry)
+{
+	kref_get(&entry->kref);
+}
+
+static void efivar_release(struct kobject *kobj)
+{
+	struct efivar_entry *var = kobj_to_efivar_entry(kobj);
+	efivar_entry_put(var);
+}
+
 static EFIVAR_ATTR(guid, 0400, efivar_guid_read, NULL);
 static EFIVAR_ATTR(attributes, 0400, efivar_attr_read, NULL);
 static EFIVAR_ATTR(size, 0400, efivar_size_read, NULL);
@@ -647,9 +673,22 @@ efivar_unregister(struct efivar_entry *var)
 	kobject_put(&var->kobj);
 }
 
+static bool efivar_is_present(struct efivar_entry *entry)
+{
+	bool present;
+
+	spin_lock(&entry->efivars->lock);
+	present = !list_empty(&entry->list);
+	spin_unlock(&entry->efivars->lock);
+
+	return present;
+}
+
 static int efivarfs_file_open(struct inode *inode, struct file *file)
 {
-	file->private_data = inode->i_private;
+	struct efivar_entry *entry = inode->i_private;
+	file->private_data = entry;
+	efivar_entry_get(entry);
 	return 0;
 }
 
@@ -706,6 +745,15 @@ static ssize_t efivarfs_file_write(struct file *file,
 	if (attributes & ~(EFI_VARIABLE_MASK))
 		return -EINVAL;
 
+	/*
+	 * It's possible that the variable has been removed from firmware
+	 * since the open(), in which case we need to flag an error
+	 * TODO: allow writes that would reinstate the variable, but check
+	 * against duplicate variables...
+	 */
+	if (!efivar_is_present(var))
+		return -EIO;
+
 	efivars = var->efivars;
 
 	/*
@@ -789,9 +837,10 @@ static ssize_t efivarfs_file_write(struct file *file,
 		mutex_unlock(&inode->i_mutex);
 
 	} else if (status == EFI_NOT_FOUND) {
-		list_del(&var->list);
+		list_del_init(&var->list);
 		spin_unlock(&efivars->lock);
 		efivar_unregister(var);
+		efivar_entry_put(var);
 		drop_nlink(inode);
 		d_delete(file->f_dentry);
 		dput(file->f_dentry);
@@ -819,6 +868,13 @@ static ssize_t efivarfs_file_read(struct file *file, char __user *userbuf,
 	void *data;
 	ssize_t size = 0;
 
+	/*
+	 * It's possible that the variable has been removed from firmware
+	 * since the open(), in which case we need to flag an error.
+	 */
+	if (!efivar_is_present(var))
+		return -EIO;
+
 	spin_lock(&efivars->lock);
 	status = efivars->ops->get_variable(var->var.VariableName,
 					    &var->var.VendorGuid,
@@ -854,8 +910,15 @@ out_free:
 	return size;
 }
 
+static int efivarfs_file_release(struct inode *inode, struct file *file)
+{
+	efivar_entry_put(file->private_data);
+	return 0;
+}
+
 static void efivarfs_evict_inode(struct inode *inode)
 {
+	efivar_entry_put(inode->i_private);
 	clear_inode(inode);
 }
 
@@ -871,10 +934,11 @@ static struct super_block *efivarfs_sb;
 static const struct inode_operations efivarfs_dir_inode_operations;
 
 static const struct file_operations efivarfs_file_operations = {
-	.open	= efivarfs_file_open,
-	.read	= efivarfs_file_read,
-	.write	= efivarfs_file_write,
-	.llseek	= no_llseek,
+	.open    = efivarfs_file_open,
+	.read    = efivarfs_file_read,
+	.write   = efivarfs_file_write,
+	.release = efivarfs_file_release,
+	.llseek  = no_llseek,
 };
 
 static struct inode *efivarfs_get_inode(struct super_block *sb,
@@ -946,6 +1010,8 @@ static int efivarfs_create(struct inode *dir, struct dentry *dentry,
 		goto out;
 	}
 
+	kref_init(&var->kref);
+
 	/* length of the variable name itself: remove GUID and separator */
 	namelen = dentry->d_name.len - GUID_LEN - 1;
 
@@ -969,6 +1035,7 @@ static int efivarfs_create(struct inode *dir, struct dentry *dentry,
 	kobject_uevent(&var->kobj, KOBJ_ADD);
 	spin_lock(&efivars->lock);
 	list_add(&var->list, &efivars->list);
+	efivar_entry_get(var);
 	spin_unlock(&efivars->lock);
 	d_instantiate(dentry, inode);
 	dget(dentry);
@@ -993,9 +1060,10 @@ static int efivarfs_unlink(struct inode *dir, struct dentry *dentry)
 					    0, 0, NULL);
 
 	if (status == EFI_SUCCESS || status == EFI_NOT_FOUND) {
-		list_del(&var->list);
+		list_del_init(&var->list);
 		spin_unlock(&efivars->lock);
 		efivar_unregister(var);
+		efivar_entry_put(var);
 		drop_nlink(dentry->d_inode);
 		dput(dentry);
 		return 0;
@@ -1077,6 +1145,7 @@ int efivarfs_fill_super(struct super_block *sb, void *data, int silent)
 
 		mutex_lock(&inode->i_mutex);
 		inode->i_private = entry;
+		efivar_entry_get(entry);
 		i_size_write(inode, size+4);
 		mutex_unlock(&inode->i_mutex);
 		d_add(dentry, inode);
@@ -1221,8 +1290,10 @@ static int efi_pstore_write(enum pstore_type_id type,
 					   0, NULL);
 	}
 
-	if (found)
-		list_del(&found->list);
+	if (found) {
+		list_del_init(&found->list);
+		efivar_entry_put(found);
+	}
 
 	for (i = 0; i < DUMP_NAME_LEN; i++)
 		efi_name[i] = name[i];
@@ -1414,7 +1485,7 @@ static ssize_t efivar_delete(struct file *filp, struct kobject *kobj,
 		spin_unlock(&efivars->lock);
 		return -EIO;
 	}
-	list_del(&search_efivar->list);
+	list_del_init(&search_efivar->list);
 	/* We need to release this lock before unregistering. */
 	spin_unlock(&efivars->lock);
 	efivar_unregister(search_efivar);
@@ -1533,6 +1604,7 @@ efivar_create_sysfs_entry(struct efivars *efivars,
 
 	spin_lock(&efivars->lock);
 	list_add(&new_efivar->list, &efivars->list);
+	efivar_entry_get(new_efivar);
 	spin_unlock(&efivars->lock);
 
 	return 0;
@@ -1603,8 +1675,9 @@ void unregister_efivars(struct efivars *efivars)
 
 	list_for_each_entry_safe(entry, n, &efivars->list, list) {
 		spin_lock(&efivars->lock);
-		list_del(&entry->list);
+		list_del_init(&entry->list);
 		spin_unlock(&efivars->lock);
+		efivar_entry_put(entry);
 		efivar_unregister(entry);
 	}
 	if (efivars->new_var)

Re: [RFC,PATCH] efivarfs: Don't delete efivar_entry structures on unlink

[Matt Fleming]

(Including the pstore folks and quoting in full)

On Sun, 2013-01-27 at 06:52 +0800, Jeremy Kerr wrote:
> We can cause an oops by unlinking an open efivarfs file, then reading
> (or writing) from the deleted file. The unlink operation causes the
> efivarfs_entry struct to be freed, which is referenced again in the
> read.
> 
> This change allow the efivar_entry structures to remain present after
> the variable has been removed, but flagged as not present in firmware by
> way of having an empty ->list member. We keep a refcount to determine
> when the entry can actually be freed.
> 
> Signed-off-by: Jeremy Kerr <jk-mnsaURCQ41sdnm+yROfE0A@public.gmane.org>
> 
> ---
>  drivers/firmware/efivars.c |  105 +++++++++++++++++++++++++++++++------
>  1 file changed, 89 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/firmware/efivars.c b/drivers/firmware/efivars.c
> index 053f28b..764f548 100644
> --- a/drivers/firmware/efivars.c
> +++ b/drivers/firmware/efivars.c
> @@ -121,6 +121,15 @@ struct efi_variable {
>  struct efivar_entry {
>  	struct efivars *efivars;
>  	struct efi_variable var;
> +
> +	/* references to the kref are held by:
> +	 *  - open efivarfs files
> +	 *  - efivarfs inodes
> +	 *  - the kobj
> +	 *  - presence in the global efivar list
> +	 */
> +	struct kref kref;
> +
>  	struct list_head list;
>  	struct kobject kobj;
>  };
> @@ -147,7 +156,8 @@ struct efivar_attribute efivar_attr_##_name = { \
>  };
>  
>  #define to_efivar_attr(_attr) container_of(_attr, struct efivar_attribute, attr)
> -#define to_efivar_entry(obj)  container_of(obj, struct efivar_entry, kobj)
> +#define kobj_to_efivar_entry(obj)  container_of(obj, struct efivar_entry, kobj)
> +#define kref_to_efivar_entry(obj)  container_of(obj, struct efivar_entry, kref)
>  
>  /*
>   * Prototype for sysfs creation function
> @@ -580,7 +590,7 @@ efivar_show_raw(struct efivar_entry *entry, char *buf)
>  static ssize_t efivar_attr_show(struct kobject *kobj, struct attribute *attr,
>  				char *buf)
>  {
> -	struct efivar_entry *var = to_efivar_entry(kobj);
> +	struct efivar_entry *var = kobj_to_efivar_entry(kobj);
>  	struct efivar_attribute *efivar_attr = to_efivar_attr(attr);
>  	ssize_t ret = -EIO;
>  
> @@ -596,7 +606,7 @@ static ssize_t efivar_attr_show(struct kobject *kobj, struct attribute *attr,
>  static ssize_t efivar_attr_store(struct kobject *kobj, struct attribute *attr,
>  				const char *buf, size_t count)
>  {
> -	struct efivar_entry *var = to_efivar_entry(kobj);
> +	struct efivar_entry *var = kobj_to_efivar_entry(kobj);
>  	struct efivar_attribute *efivar_attr = to_efivar_attr(attr);
>  	ssize_t ret = -EIO;
>  
> @@ -614,12 +624,28 @@ static const struct sysfs_ops efivar_attr_ops = {
>  	.store = efivar_attr_store,
>  };
>  
> -static void efivar_release(struct kobject *kobj)
> +static void efivar_entry_release(struct kref *kref)
>  {
> -	struct efivar_entry *var = container_of(kobj, struct efivar_entry, kobj);
> +	struct efivar_entry *var = kref_to_efivar_entry(kref);
>  	kfree(var);
>  }
>  
> +static void efivar_entry_put(struct efivar_entry *entry)
> +{
> +	kref_put(&entry->kref, efivar_entry_release);
> +}
> +
> +static void efivar_entry_get(struct efivar_entry *entry)
> +{
> +	kref_get(&entry->kref);
> +}
> +
> +static void efivar_release(struct kobject *kobj)
> +{
> +	struct efivar_entry *var = kobj_to_efivar_entry(kobj);
> +	efivar_entry_put(var);
> +}
> +
>  static EFIVAR_ATTR(guid, 0400, efivar_guid_read, NULL);
>  static EFIVAR_ATTR(attributes, 0400, efivar_attr_read, NULL);
>  static EFIVAR_ATTR(size, 0400, efivar_size_read, NULL);
> @@ -647,9 +673,22 @@ efivar_unregister(struct efivar_entry *var)
>  	kobject_put(&var->kobj);
>  }
>  
> +static bool efivar_is_present(struct efivar_entry *entry)
> +{
> +	bool present;
> +
> +	spin_lock(&entry->efivars->lock);
> +	present = !list_empty(&entry->list);
> +	spin_unlock(&entry->efivars->lock);
> +
> +	return present;
> +}
> +
>  static int efivarfs_file_open(struct inode *inode, struct file *file)
>  {
> -	file->private_data = inode->i_private;
> +	struct efivar_entry *entry = inode->i_private;
> +	file->private_data = entry;
> +	efivar_entry_get(entry);
>  	return 0;
>  }

Isn't there a race here? Can't the efivar_entry be deleted and freed
before we call efivar_get_entry()?

The problem is you need to be able to ensure the validity of
inode->i_private, but you can't in the open() function if you haven't
already taken a reference to the variable. A ref count of some
description needs to be incremented in efivarfs_fill_super() before the
efivar_entry pointer is stored in the inode, and while we're holding the
lock.

> @@ -706,6 +745,15 @@ static ssize_t efivarfs_file_write(struct file *file,
>  	if (attributes & ~(EFI_VARIABLE_MASK))
>  		return -EINVAL;
>  
> +	/*
> +	 * It's possible that the variable has been removed from firmware
> +	 * since the open(), in which case we need to flag an error
> +	 * TODO: allow writes that would reinstate the variable, but check
> +	 * against duplicate variables...
> +	 */
> +	if (!efivar_is_present(var))
> +		return -EIO;
> +
>  	efivars = var->efivars;
>  
>  	/*
> @@ -789,9 +837,10 @@ static ssize_t efivarfs_file_write(struct file *file,
>  		mutex_unlock(&inode->i_mutex);
>  
>  	} else if (status == EFI_NOT_FOUND) {
> -		list_del(&var->list);
> +		list_del_init(&var->list);
>  		spin_unlock(&efivars->lock);
>  		efivar_unregister(var);
> +		efivar_entry_put(var);
>  		drop_nlink(inode);
>  		d_delete(file->f_dentry);
>  		dput(file->f_dentry);
> @@ -819,6 +868,13 @@ static ssize_t efivarfs_file_read(struct file *file, char __user *userbuf,
>  	void *data;
>  	ssize_t size = 0;
>  
> +	/*
> +	 * It's possible that the variable has been removed from firmware
> +	 * since the open(), in which case we need to flag an error.
> +	 */
> +	if (!efivar_is_present(var))
> +		return -EIO;
> +
>  	spin_lock(&efivars->lock);
>  	status = efivars->ops->get_variable(var->var.VariableName,
>  					    &var->var.VendorGuid,
> @@ -854,8 +910,15 @@ out_free:
>  	return size;
>  }
>  
> +static int efivarfs_file_release(struct inode *inode, struct file *file)
> +{
> +	efivar_entry_put(file->private_data);
> +	return 0;
> +}
> +
>  static void efivarfs_evict_inode(struct inode *inode)
>  {
> +	efivar_entry_put(inode->i_private);
>  	clear_inode(inode);
>  }
>  
> @@ -871,10 +934,11 @@ static struct super_block *efivarfs_sb;
>  static const struct inode_operations efivarfs_dir_inode_operations;
>  
>  static const struct file_operations efivarfs_file_operations = {
> -	.open	= efivarfs_file_open,
> -	.read	= efivarfs_file_read,
> -	.write	= efivarfs_file_write,
> -	.llseek	= no_llseek,
> +	.open    = efivarfs_file_open,
> +	.read    = efivarfs_file_read,
> +	.write   = efivarfs_file_write,
> +	.release = efivarfs_file_release,
> +	.llseek  = no_llseek,
>  };
>  
>  static struct inode *efivarfs_get_inode(struct super_block *sb,
> @@ -946,6 +1010,8 @@ static int efivarfs_create(struct inode *dir, struct dentry *dentry,
>  		goto out;
>  	}
>  
> +	kref_init(&var->kref);
> +
>  	/* length of the variable name itself: remove GUID and separator */
>  	namelen = dentry->d_name.len - GUID_LEN - 1;
>  
> @@ -969,6 +1035,7 @@ static int efivarfs_create(struct inode *dir, struct dentry *dentry,
>  	kobject_uevent(&var->kobj, KOBJ_ADD);
>  	spin_lock(&efivars->lock);
>  	list_add(&var->list, &efivars->list);
> +	efivar_entry_get(var);
>  	spin_unlock(&efivars->lock);
>  	d_instantiate(dentry, inode);
>  	dget(dentry);
> @@ -993,9 +1060,10 @@ static int efivarfs_unlink(struct inode *dir, struct dentry *dentry)
>  					    0, 0, NULL);
>  
>  	if (status == EFI_SUCCESS || status == EFI_NOT_FOUND) {
> -		list_del(&var->list);
> +		list_del_init(&var->list);
>  		spin_unlock(&efivars->lock);
>  		efivar_unregister(var);
> +		efivar_entry_put(var);
>  		drop_nlink(dentry->d_inode);
>  		dput(dentry);
>  		return 0;
> @@ -1077,6 +1145,7 @@ int efivarfs_fill_super(struct super_block *sb, void *data, int silent)
>  
>  		mutex_lock(&inode->i_mutex);
>  		inode->i_private = entry;
> +		efivar_entry_get(entry);
>  		i_size_write(inode, size+4);
>  		mutex_unlock(&inode->i_mutex);
>  		d_add(dentry, inode);
> @@ -1221,8 +1290,10 @@ static int efi_pstore_write(enum pstore_type_id type,
>  					   0, NULL);
>  	}
>  
> -	if (found)
> -		list_del(&found->list);
> +	if (found) {
> +		list_del_init(&found->list);
> +		efivar_entry_put(found);
> +	}
>  
>  	for (i = 0; i < DUMP_NAME_LEN; i++)
>  		efi_name[i] = name[i];
> @@ -1414,7 +1485,7 @@ static ssize_t efivar_delete(struct file *filp, struct kobject *kobj,
>  		spin_unlock(&efivars->lock);
>  		return -EIO;
>  	}
> -	list_del(&search_efivar->list);
> +	list_del_init(&search_efivar->list);
>  	/* We need to release this lock before unregistering. */
>  	spin_unlock(&efivars->lock);
>  	efivar_unregister(search_efivar);
> @@ -1533,6 +1604,7 @@ efivar_create_sysfs_entry(struct efivars *efivars,
>  
>  	spin_lock(&efivars->lock);
>  	list_add(&new_efivar->list, &efivars->list);
> +	efivar_entry_get(new_efivar);
>  	spin_unlock(&efivars->lock);
>  
>  	return 0;
> @@ -1603,8 +1675,9 @@ void unregister_efivars(struct efivars *efivars)
>  
>  	list_for_each_entry_safe(entry, n, &efivars->list, list) {
>  		spin_lock(&efivars->lock);
> -		list_del(&entry->list);
> +		list_del_init(&entry->list);
>  		spin_unlock(&efivars->lock);
> +		efivar_entry_put(entry);
>  		efivar_unregister(entry);
>  	}
>  	if (efivars->new_var)


-- 
Matt Fleming, Intel Open Source Technology Center

Re: [RFC,PATCH] efivarfs: Don't delete efivar_entry structures on unlink

[Jeremy Kerr]

Hi Matt,

> Isn't there a race here? Can't the efivar_entry be deleted and freed
> before we call efivar_get_entry()?
>
> The problem is you need to be able to ensure the validity of
> inode->i_private, but you can't in the open() function if you haven't
> already taken a reference to the variable. A ref count of some
> description needs to be incremented in efivarfs_fill_super() before the
> efivar_entry pointer is stored in the inode, and while we're holding the
> lock.

Yes, the intention is to bump the refcount any time we store the struct 
efivars_entry somewhere it might be referenced later (ie, in 
inode->i_private in this case). I'll add the ref when we create the 
inodes too.

One of my concerns with the current patch is using the empty list as an 
indicator whether the variable is present in firmware. Alternatively, we 
could do this with a new bool 'deleted' in the strut efivars_entry, 
which is set on unlink(). Any preferences?

Cheers,


Jeremy

Re: [RFC,PATCH] efivarfs: Don't delete efivar_entry structures on unlink

[Matt Fleming]

On Tue, 2013-01-29 at 07:45 +0900, Jeremy Kerr wrote:
> Yes, the intention is to bump the refcount any time we store the struct 
> efivars_entry somewhere it might be referenced later (ie, in 
> inode->i_private in this case). I'll add the ref when we create the 
> inodes too.
> 
> One of my concerns with the current patch is using the empty list as an 
> indicator whether the variable is present in firmware. Alternatively, we 
> could do this with a new bool 'deleted' in the strut efivars_entry, 
> which is set on unlink(). Any preferences?

I think using presence on the efivar list to indicate that a variable
exists in the firmware is a nice and simple solution.

-- 
Matt Fleming, Intel Open Source Technology Center

Re: [RFC,PATCH] efivarfs: Don't delete efivar_entry structures on unlink

[Lingzhu Xiang]

On 01/27/2013 06:52 AM, Jeremy Kerr wrote:
> @@ -1221,8 +1290,10 @@ static int efi_pstore_write(enum pstore_type_id type,
>   					   0, NULL);
>   	}
>   
> -	if (found)
> -		list_del(&found->list);
> +	if (found) {
> +		list_del_init(&found->list);
> +		efivar_entry_put(found);
> +	}
>   
>   	for (i = 0; i < DUMP_NAME_LEN; i++)
>   		efi_name[i] = name[i];

Can't apply against mainline. This part has been removed in commit
96480d9c.

I verify that the open-unlink bug is fixed with this patch (without
the above part) and 3.8-rc5. But now I'm getting extra call traces.

At boot time:
[    0.808347] EFI Variables Facility v0.08 2004-May-17
[    0.809848] ------------[ cut here ]------------
[    0.811048] WARNING: at include/linux/kref.h:42 efivar_create_sysfs_entry+0x1d1/0x1e0()
[    0.813203] Modules linked in:
[    0.813958] Pid: 1, comm: swapper/0 Not tainted 3.8.0-0.rc5.git1.1.efivarfs.open.unlink.fc18.x86_64 #1
[    0.816501] Call Trace:
[    0.817143]  [<ffffffff8105eddf>] warn_slowpath_common+0x7f/0xc0
[    0.818472]  [<ffffffff8105ee3a>] warn_slowpath_null+0x1a/0x20
[    0.819720]  [<ffffffff81545ef1>] efivar_create_sysfs_entry+0x1d1/0x1e0
[    0.821209]  [<ffffffff815464ae>] register_efivars+0xee/0x3b0
[    0.822441]  [<ffffffff81d5970c>] ? dmi_sysfs_register_handle+0x1c0/0x1c0
[    0.823886]  [<ffffffff81d597c6>] efivars_init+0xba/0x108
[    0.825046]  [<ffffffff8100215a>] do_one_initcall+0x12a/0x180
[    0.826273]  [<ffffffff81d1bdbe>] kernel_init_freeable+0x154/0x1de
[    0.827551]  [<ffffffff81d1b614>] ? do_early_param+0x8c/0x8c
[    0.828754]  [<ffffffff816ad600>] ? rest_init+0x140/0x140
[    0.829895]  [<ffffffff816ad60e>] kernel_init+0xe/0xf0
[    0.830991]  [<ffffffff816d47ec>] ret_from_fork+0x7c/0xb0
[    0.832139]  [<ffffffff816ad600>] ? rest_init+0x140/0x140
[    0.833307] ---[ end trace 7f6e14e7c9c8160e ]---
(repeat for each variable)

Later:
[root@qemu-ovmf ~]# umount /sys/firmware/efi/efivars/
[   31.794457] BUG: unable to handle kernel NULL pointer dereference at 000000000000082c
[   31.795025] IP: [<ffffffff81544695>] efivar_entry_put+0x5/0x30
[   31.795025] PGD 0
[   31.795025] Oops: 0002 [#1] SMP
[   31.795025] Modules linked in: vfat fat crc32c_intel ppdev parport_pc i2c_piix4 parport i2c_core virtio_net microcode
[   31.795025] CPU 7
[   31.795025] Pid: 615, comm: umount Tainted: G        W    3.8.0-0.rc5.git1.1.efivarfs.open.unlink.fc18.x86_64 #1
[   31.795025] RIP: 0010:[<ffffffff81544695>]  [<ffffffff81544695>] efivar_entry_put+0x5/0x30
[   31.795025] RSP: 0018:ffff88021253dd30  EFLAGS: 00010292
[   31.795025] RAX: ffffffff81544700 RBX: ffff880212ef6a90 RCX: 0000000000000034
[   31.795025] RDX: 0000000000000001 RSI: 0000000000000001 RDI: 0000000000000000
[   31.795025] RBP: ffff88021253dd48 R08: c038000000000000 R09: 0000000000000002
[   31.795025] R10: 0000000000000000 R11: 0000000000000000 R12: ffff880212ef6c28
[   31.795025] R13: ffffffff8187b920 R14: ffffffff8187b920 R15: ffff880214076680
[   31.795025] FS:  00007ff23e7f9840(0000) GS:ffff88021fce0000(0000) knlGS:0000000000000000
[   31.795025] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[   31.795025] CR2: 000000000000082c CR3: 00000001fe835000 CR4: 00000000000007e0
[   31.795025] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
[   31.795025] DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400
[   31.795025] Process umount (pid: 615, threadinfo ffff88021253c000, task ffff8802125fc2a0)
[   31.795025] Stack:
[   31.795025]  ffffffff8154471d ffff880212ef6a90 ffff880212ef6a90 ffff88021253dd78
[   31.795025]  ffffffff811dacf7 ffff88021253dd78 ffff880212ef6a90 ffff880212ef6b18
[   31.795025]  ffff88020fb1c0f8 ffff88021253dda8 ffffffff811db4f5 ffff880212f17180
[   31.795025] Call Trace:
[   31.795025]  [<ffffffff8154471d>] ? efivarfs_evict_inode+0x1d/0x30
[   31.795025]  [<ffffffff811dacf7>] evict+0xa7/0x1a0
[   31.795025]  [<ffffffff811db4f5>] iput+0x105/0x190
[   31.795025]  [<ffffffff811d55c1>] shrink_dcache_for_umount_subtree+0x111/0x190
[   31.795025]  [<ffffffff816cb4db>] ? _raw_spin_unlock+0x2b/0x40
[   31.795025]  [<ffffffff811d7e03>] shrink_dcache_for_umount+0x33/0x60
[   31.795025]  [<ffffffff811c132c>] generic_shutdown_super+0x2c/0xf0
[   31.795025]  [<ffffffff811c1486>] kill_anon_super+0x16/0x30
[   31.795025]  [<ffffffff811c14c7>] kill_litter_super+0x27/0x30
[   31.795025]  [<ffffffff81543fde>] efivarfs_kill_sb+0xe/0x20
[   31.795025]  [<ffffffff811c18b7>] deactivate_locked_super+0x57/0x80
[   31.795025]  [<ffffffff811c24fe>] deactivate_super+0x4e/0x70
[   31.795025]  [<ffffffff811e1827>] mntput_no_expire+0xd7/0x130
[   31.795025]  [<ffffffff811e2706>] sys_umount+0x76/0x3a0
[   31.795025]  [<ffffffff816d4899>] system_call_fastpath+0x16/0x1b
[   31.795025] Code: 43 20 00 ba 87 81 48 c7 83 d0 01 00 00 00 8a 81 81 48 89 df e8 7d 4f c9 ff 48 89 d8 5b 41 5c 5d c3 0f 1f 44 00 00 66 66 66 66 90 <f0> 83 af 2c 08 00 00 01 0f 94 c0 84 c0 75 0c f3 c3 66 2e 0f 1f
[   31.795025] RIP  [<ffffffff81544695>] efivar_entry_put+0x5/0x30
[   31.795025]  RSP <ffff88021253dd30>
[   31.795025] CR2: 000000000000082c
[   31.876930] ---[ end trace 01acf8f410487409 ]---
Killed