* [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®