mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH] Support UEFI variable append and deleting authenticated variables.
@ 2012-06-25 13:12 Peter Jones
  2012-07-30 10:45 ` Matt Fleming
  0 siblings, 1 reply; 3+ messages in thread
From: Peter Jones @ 2012-06-25 13:12 UTC (permalink / raw)
  To: Matthew Garrett; +Cc: linux-kernel, Peter Jones

This adds support for appending to all UEFI variables, and also for
deleting authentication variables.

Signed-off-by: Peter Jones <pjones@redhat.com>
---
 drivers/firmware/efivars.c |   99 +++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 94 insertions(+), 5 deletions(-)

diff --git a/drivers/firmware/efivars.c b/drivers/firmware/efivars.c
index 47408e8..b12a2f0 100644
--- a/drivers/firmware/efivars.c
+++ b/drivers/firmware/efivars.c
@@ -108,6 +108,27 @@ struct efi_variable {
 	__u32         Attributes;
 } __attribute__((packed));
 
+struct win_certificate {
+	__u32	dwLength;
+	__u16	wRevision;
+	__u16	wCertificateType;
+	__u8	wCertificate[];
+};
+
+struct win_certificate_uefi_guid {
+	struct win_certificate	Hdr;
+	efi_guid_t		CertType;
+};
+
+struct efi_variable_authentication {
+	__u64					MonotonicCount;
+	struct win_certificate_uefi_guid	AuthInfo;
+};
+
+struct efi_variable_authentication_2 {
+	efi_time_t				TimeStamp;
+	struct win_certificate_uefi_guid	AuthInfo;
+};
 
 struct efivar_entry {
 	struct efivars *efivars;
@@ -802,6 +823,54 @@ static struct pstore_info efi_pstore_info = {
 	.erase		= efi_pstore_erase,
 };
 
+static int is_authenticated_delete(struct efi_variable *new_var)
+{
+	/* If we get a set_variable() call that's got an authenticated
+	 * variable attribute set, and its DataSize is the same size as
+	 * the AuthInfo descriptor, then it's really a delete. */
+	if (new_var->Attributes & EFI_VARIABLE_AUTHENTICATED_WRITE_ACCESS) {
+		struct efi_variable_authentication *eva;
+		__u32 size;
+
+		if (new_var->DataSize <
+				sizeof(struct efi_variable_authentication))
+			return 0;
+
+		eva = (struct efi_variable_authentication *)new_var->Data;
+
+		/* 27.2.4 says:
+		 * dwLength: The length of the entire certificate, including
+		 *           the length of the header, in bytes.
+		 */
+		size = sizeof(eva->AuthInfo.CertType) +
+		       eva->AuthInfo.Hdr.dwLength;
+
+		if (size == new_var->DataSize)
+			return 1;
+	} else if (new_var->Attributes
+			& EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS) {
+		struct efi_variable_authentication_2 *eva;
+		__u32 size;
+
+		if (new_var->DataSize <
+				sizeof(struct efi_variable_authentication_2))
+			return 0;
+
+		eva = (struct efi_variable_authentication_2 *)new_var->Data;
+
+		/* 27.2.4 says:
+		 * dwLength: The length of the entire certificate, including
+		 *           the length of the header, in bytes.
+		 */
+		size = sizeof(eva->AuthInfo.CertType) +
+		       eva->AuthInfo.Hdr.dwLength;
+
+		if (size == new_var->DataSize)
+			return 1;
+	}
+	return 0;
+}
+
 static ssize_t efivar_create(struct file *filp, struct kobject *kobj,
 			     struct bin_attribute *bin_attr,
 			     char *buf, loff_t pos, size_t count)
@@ -812,6 +881,8 @@ static ssize_t efivar_create(struct file *filp, struct kobject *kobj,
 	unsigned long strsize1, strsize2;
 	efi_status_t status = EFI_NOT_FOUND;
 	int found = 0;
+	int is_append = 0;
+	int is_delete = 0;
 
 	if (!capable(CAP_SYS_ADMIN))
 		return -EACCES;
@@ -839,11 +910,20 @@ static ssize_t efivar_create(struct file *filp, struct kobject *kobj,
 			break;
 		}
 	}
-	if (found) {
+	if (new_var->Attributes & EFI_VARIABLE_APPEND_WRITE) {
+		if (!found) {
+			spin_unlock(&efivars->lock);
+			return -EINVAL;
+		}
+		is_append = 1;
+	} else if (is_authenticated_delete(new_var)) {
+		is_delete = 1;
+	} else if (found) {
 		spin_unlock(&efivars->lock);
 		return -EINVAL;
 	}
 
+
 	/* now *really* create the variable via EFI */
 	status = efivars->ops->set_variable(new_var->VariableName,
 					    &new_var->VendorGuid,
@@ -857,16 +937,25 @@ static ssize_t efivar_create(struct file *filp, struct kobject *kobj,
 		spin_unlock(&efivars->lock);
 		return -EIO;
 	}
-	spin_unlock(&efivars->lock);
 
 	/* Create the entry in sysfs.  Locking is not required here */
-	status = efivar_create_sysfs_entry(efivars,
+	if (is_delete) {
+		list_del(&search_efivar->list);
+
+		/* We need to release this lock before unregistering. */
+		spin_unlock(&efivars->lock);
+		efivar_unregister(search_efivar);
+	} else if (is_append) {
+		spin_unlock(&efivars->lock);
+	} else {
+		spin_unlock(&efivars->lock);
+		status = efivar_create_sysfs_entry(efivars,
 					   utf16_strsize(new_var->VariableName,
 							 1024),
 					   new_var->VariableName,
 					   &new_var->VendorGuid);
-	if (status) {
-		printk(KERN_WARNING "efivars: variable created, but sysfs entry wasn't.\n");
+		if (status)
+			pr_warn("efivars: variable created, but sysfs entry wasn't.\n");
 	}
 	return count;
 }
-- 
1.7.10.2


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] Support UEFI variable append and deleting authenticated variables.
  2012-06-25 13:12 [PATCH] Support UEFI variable append and deleting authenticated variables Peter Jones
@ 2012-07-30 10:45 ` Matt Fleming
  2012-07-30 14:16   ` Peter Jones
  0 siblings, 1 reply; 3+ messages in thread
From: Matt Fleming @ 2012-07-30 10:45 UTC (permalink / raw)
  To: Peter Jones
  Cc: Matthew Garrett, linux-kernel, linux-efi, Mike Waychison, H. Peter Anvin

On Mon, 2012-06-25 at 09:12 -0400, Peter Jones wrote:
> This adds support for appending to all UEFI variables, and also for
> deleting authentication variables.
> 
> Signed-off-by: Peter Jones <pjones@redhat.com>
> ---
>  drivers/firmware/efivars.c |   99 +++++++++++++++++++++++++++++++++++++++++---
>  1 file changed, 94 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/firmware/efivars.c b/drivers/firmware/efivars.c
> index 47408e8..b12a2f0 100644
> --- a/drivers/firmware/efivars.c
> +++ b/drivers/firmware/efivars.c
> @@ -108,6 +108,27 @@ struct efi_variable {
>  	__u32         Attributes;
>  } __attribute__((packed));
>  
> +struct win_certificate {
> +	__u32	dwLength;
> +	__u16	wRevision;
> +	__u16	wCertificateType;
> +	__u8	wCertificate[];
> +};
> +
> +struct win_certificate_uefi_guid {
> +	struct win_certificate	Hdr;
> +	efi_guid_t		CertType;
> +};
> +
> +struct efi_variable_authentication {
> +	__u64					MonotonicCount;
> +	struct win_certificate_uefi_guid	AuthInfo;
> +};
> +
> +struct efi_variable_authentication_2 {
> +	efi_time_t				TimeStamp;
> +	struct win_certificate_uefi_guid	AuthInfo;
> +};
>  
>  struct efivar_entry {
>  	struct efivars *efivars;
> @@ -802,6 +823,54 @@ static struct pstore_info efi_pstore_info = {
>  	.erase		= efi_pstore_erase,
>  };
>  
> +static int is_authenticated_delete(struct efi_variable *new_var)
> +{
> +	/* If we get a set_variable() call that's got an authenticated
> +	 * variable attribute set, and its DataSize is the same size as
> +	 * the AuthInfo descriptor, then it's really a delete. */

Just FYI, the multi-line comment format used throughout this file is,

	/*
	 * This is a multi-line comment
	 */

and it would be better to not break that convention. Deleting entries in
this way seems counter-intuitive to me. Is there a reason that you can't
just delete authenticated variables with efivar_delete()?

> +	if (new_var->Attributes & EFI_VARIABLE_AUTHENTICATED_WRITE_ACCESS) {
> +		struct efi_variable_authentication *eva;
> +		__u32 size;
> +
> +		if (new_var->DataSize <
> +				sizeof(struct efi_variable_authentication))
> +			return 0;

You could write this as,

		if (new_var->DataSize < sizeof(*eva))

which would mean that you wouldn't have to split it across two lines
like this.

> +		eva = (struct efi_variable_authentication *)new_var->Data;
> +
> +		/* 27.2.4 says:
> +		 * dwLength: The length of the entire certificate, including
> +		 *           the length of the header, in bytes.
> +		 */
> +		size = sizeof(eva->AuthInfo.CertType) +
> +		       eva->AuthInfo.Hdr.dwLength;
> +
> +		if (size == new_var->DataSize)
> +			return 1;
> +	} else if (new_var->Attributes
> +			& EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS) {
> +		struct efi_variable_authentication_2 *eva;
> +		__u32 size;
> +
> +		if (new_var->DataSize <
> +				sizeof(struct efi_variable_authentication_2))
> +			return 0;
> +
> +		eva = (struct efi_variable_authentication_2 *)new_var->Data;
> +
> +		/* 27.2.4 says:
> +		 * dwLength: The length of the entire certificate, including
> +		 *           the length of the header, in bytes.
> +		 */
> +		size = sizeof(eva->AuthInfo.CertType) +
> +		       eva->AuthInfo.Hdr.dwLength;
> +
> +		if (size == new_var->DataSize)
> +			return 1;
> +	}
> +	return 0;
> +}
> +
>  static ssize_t efivar_create(struct file *filp, struct kobject *kobj,
>  			     struct bin_attribute *bin_attr,
>  			     char *buf, loff_t pos, size_t count)
> @@ -812,6 +881,8 @@ static ssize_t efivar_create(struct file *filp, struct kobject *kobj,
>  	unsigned long strsize1, strsize2;
>  	efi_status_t status = EFI_NOT_FOUND;
>  	int found = 0;
> +	int is_append = 0;
> +	int is_delete = 0;
>  
>  	if (!capable(CAP_SYS_ADMIN))
>  		return -EACCES;
> @@ -839,11 +910,20 @@ static ssize_t efivar_create(struct file *filp, struct kobject *kobj,
>  			break;
>  		}
>  	}
> -	if (found) {
> +	if (new_var->Attributes & EFI_VARIABLE_APPEND_WRITE) {
> +		if (!found) {
> +			spin_unlock(&efivars->lock);
> +			return -EINVAL;
> +		}
> +		is_append = 1;
> +	} else if (is_authenticated_delete(new_var)) {
> +		is_delete = 1;
> +	} else if (found) {
>  		spin_unlock(&efivars->lock);
>  		return -EINVAL;
>  	}
>  
> +

Stray newline introduced?

>  	/* now *really* create the variable via EFI */
>  	status = efivars->ops->set_variable(new_var->VariableName,
>  					    &new_var->VendorGuid,
> @@ -857,16 +937,25 @@ static ssize_t efivar_create(struct file *filp, struct kobject *kobj,
>  		spin_unlock(&efivars->lock);
>  		return -EIO;
>  	}
> -	spin_unlock(&efivars->lock);
>  
>  	/* Create the entry in sysfs.  Locking is not required here */
> -	status = efivar_create_sysfs_entry(efivars,
> +	if (is_delete) {
> +		list_del(&search_efivar->list);
> +
> +		/* We need to release this lock before unregistering. */
> +		spin_unlock(&efivars->lock);
> +		efivar_unregister(search_efivar);
> +	} else if (is_append) {
> +		spin_unlock(&efivars->lock);
> +	} else {
> +		spin_unlock(&efivars->lock);
> +		status = efivar_create_sysfs_entry(efivars,
>  					   utf16_strsize(new_var->VariableName,
>  							 1024),
>  					   new_var->VariableName,
>  					   &new_var->VendorGuid);
> -	if (status) {
> -		printk(KERN_WARNING "efivars: variable created, but sysfs entry wasn't.\n");
> +		if (status)
> +			pr_warn("efivars: variable created, but sysfs entry wasn't.\n");
>  	}
>  	return count;
>  }




^ permalink raw reply	[flat|nested] 3+ messages in thread

* [PATCH] Support UEFI variable append and deleting authenticated variables.
  2012-07-30 10:45 ` Matt Fleming
@ 2012-07-30 14:16   ` Peter Jones
  0 siblings, 0 replies; 3+ messages in thread
From: Peter Jones @ 2012-07-30 14:16 UTC (permalink / raw)
  To: linux-efi; +Cc: linux-kernel, Matt Fleming, Matthew Garrett, Peter Jones

This adds support for appending to all UEFI variables, and also for
deleting authentication variables.

(Updated to address mfleming's concerns on 7/30/2012)

Signed-off-by: Peter Jones <pjones@redhat.com>
---
 drivers/firmware/efivars.c |   99 ++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 91 insertions(+), 8 deletions(-)

diff --git a/drivers/firmware/efivars.c b/drivers/firmware/efivars.c
index 47408e8..ff8b524 100644
--- a/drivers/firmware/efivars.c
+++ b/drivers/firmware/efivars.c
@@ -108,6 +108,27 @@ struct efi_variable {
 	__u32         Attributes;
 } __attribute__((packed));
 
+struct win_certificate {
+	__u32	dwLength;
+	__u16	wRevision;
+	__u16	wCertificateType;
+	__u8	wCertificate[];
+};
+
+struct win_certificate_uefi_guid {
+	struct win_certificate	Hdr;
+	efi_guid_t		CertType;
+};
+
+struct efi_variable_authentication {
+	__u64					MonotonicCount;
+	struct win_certificate_uefi_guid	AuthInfo;
+};
+
+struct efi_variable_authentication_2 {
+	efi_time_t				TimeStamp;
+	struct win_certificate_uefi_guid	AuthInfo;
+};
 
 struct efivar_entry {
 	struct efivars *efivars;
@@ -812,6 +833,7 @@ static ssize_t efivar_create(struct file *filp, struct kobject *kobj,
 	unsigned long strsize1, strsize2;
 	efi_status_t status = EFI_NOT_FOUND;
 	int found = 0;
+	int is_append = 0;
 
 	if (!capable(CAP_SYS_ADMIN))
 		return -EACCES;
@@ -839,7 +861,13 @@ static ssize_t efivar_create(struct file *filp, struct kobject *kobj,
 			break;
 		}
 	}
-	if (found) {
+	if (new_var->Attributes & EFI_VARIABLE_APPEND_WRITE) {
+		if (!found) {
+			spin_unlock(&efivars->lock);
+			return -EINVAL;
+		}
+		is_append = 1;
+	} else if (found) {
 		spin_unlock(&efivars->lock);
 		return -EINVAL;
 	}
@@ -857,20 +885,73 @@ static ssize_t efivar_create(struct file *filp, struct kobject *kobj,
 		spin_unlock(&efivars->lock);
 		return -EIO;
 	}
-	spin_unlock(&efivars->lock);
 
 	/* Create the entry in sysfs.  Locking is not required here */
-	status = efivar_create_sysfs_entry(efivars,
+	if (is_append) {
+		spin_unlock(&efivars->lock);
+	} else {
+		spin_unlock(&efivars->lock);
+		status = efivar_create_sysfs_entry(efivars,
 					   utf16_strsize(new_var->VariableName,
 							 1024),
 					   new_var->VariableName,
 					   &new_var->VendorGuid);
-	if (status) {
-		printk(KERN_WARNING "efivars: variable created, but sysfs entry wasn't.\n");
+		if (status)
+			pr_warn("efivars: variable created, but sysfs entry wasn't.\n");
 	}
 	return count;
 }
 
+static int is_authenticated_delete(struct efi_variable *new_var)
+{
+	/*
+	 * If we get a set_variable() call that's got an authenticated
+	 * variable attribute set, and its DataSize is the same size as
+	 * the AuthInfo descriptor, then it's really a delete.
+	 */
+	if (new_var->Attributes & EFI_VARIABLE_AUTHENTICATED_WRITE_ACCESS) {
+		struct efi_variable_authentication *eva;
+		__u32 size;
+
+		if (new_var->DataSize < sizeof(*eva))
+			return 0;
+
+		eva = (struct efi_variable_authentication *)new_var->Data;
+
+		/*
+		 * 27.2.4 says:
+		 * dwLength: The length of the entire certificate, including
+		 *           the length of the header, in bytes.
+		 */
+		size = sizeof(eva->AuthInfo.CertType) +
+		       eva->AuthInfo.Hdr.dwLength;
+
+		if (size == new_var->DataSize)
+			return 1;
+	} else if (new_var->Attributes
+			& EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS) {
+		struct efi_variable_authentication_2 *eva;
+		__u32 size;
+
+		if (new_var->DataSize < sizeof(*eva))
+			return 0;
+
+		eva = (struct efi_variable_authentication_2 *)new_var->Data;
+
+		/*
+		 * 27.2.4 says:
+		 * dwLength: The length of the entire certificate, including
+		 *           the length of the header, in bytes.
+		 */
+		size = sizeof(eva->AuthInfo.CertType) +
+		       eva->AuthInfo.Hdr.dwLength;
+
+		if (size == new_var->DataSize)
+			return 1;
+	}
+	return 0;
+}
+
 static ssize_t efivar_delete(struct file *filp, struct kobject *kobj,
 			     struct bin_attribute *bin_attr,
 			     char *buf, loff_t pos, size_t count)
@@ -906,9 +987,11 @@ static ssize_t efivar_delete(struct file *filp, struct kobject *kobj,
 		spin_unlock(&efivars->lock);
 		return -EINVAL;
 	}
-	/* force the Attributes/DataSize to 0 to ensure deletion */
-	del_var->Attributes = 0;
-	del_var->DataSize = 0;
+	if (!is_authenticated_delete(del_var)) {
+		/* force the Attributes/DataSize to 0 to ensure deletion */
+		del_var->DataSize = 0;
+		del_var->Attributes = 0;
+	}
 
 	status = efivars->ops->set_variable(del_var->VariableName,
 					    &del_var->VendorGuid,
-- 
1.7.10.4


^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2012-07-30 14:16 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-06-25 13:12 [PATCH] Support UEFI variable append and deleting authenticated variables Peter Jones
2012-07-30 10:45 ` Matt Fleming
2012-07-30 14:16   ` Peter Jones

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®