[PATCH] Add missing attributes to EFI variable attribute print out from sysfs

[PATCH] Add missing attributes to EFI variable attribute print out from sysfs

[Khalid Aziz]

Some of the EFI variable attributes are missing from print out from
/sys/firmware/efi/vars/*/attributes. This patch adds those in.

Signed-off-by: Khalid Aziz <khalid.aziz@hp.com>
Cc: stable@vger.kernel.org
---
 drivers/firmware/efivars.c |   10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/drivers/firmware/efivars.c b/drivers/firmware/efivars.c
index 47408e8..1e1ac75 100644
--- a/drivers/firmware/efivars.c
+++ b/drivers/firmware/efivars.c
@@ -441,6 +441,16 @@ efivar_attr_read(struct efivar_entry *entry, char *buf)
 		str += sprintf(str, "EFI_VARIABLE_BOOTSERVICE_ACCESS\n");
 	if (var->Attributes & 0x4)
 		str += sprintf(str, "EFI_VARIABLE_RUNTIME_ACCESS\n");
+	if (var->Attributes & 0x8)
+		str += sprintf(str, "EFI_VARIABLE_HARDWARE_ERROR_RECORD\n");
+	if (var->Attributes & 0x10)
+		str += sprintf(str,
+			"EFI_VARIABLE_AUTHENTICATED_WRITE_ACCESS\n");
+	if (var->Attributes & 0x20)
+		str += sprintf(str,
+			"EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS\n");
+	if (var->Attributes & 0x40)
+		str += sprintf(str, "EFI_VARIABLE_APPEND_WRITE\n");
 	return str - buf;
 }
 
-- 
1.7.9.5

==================
Khalid Aziz
khalid.aziz@hp.com

Re: [PATCH] Add missing attributes to EFI variable attribute print out from sysfs

[Kees Cook]

On Fri, Jul 13, 2012 at 10:38 AM, Khalid Aziz <khalid.aziz@hp.com> wrote:
> Some of the EFI variable attributes are missing from print out from
> /sys/firmware/efi/vars/*/attributes. This patch adds those in.
>
> Signed-off-by: Khalid Aziz <khalid.aziz@hp.com>
> Cc: stable@vger.kernel.org

Seems good to me.

Reviewed-by: Kees Cook <keescook@chromium.org>

-- 
Kees Cook
Chrome OS Security

Re: [PATCH] Add missing attributes to EFI variable attribute print out from sysfs

[Matthew Garrett]

On Fri, Jul 13, 2012 at 09:38:27AM -0600, Khalid Aziz wrote:
>  	if (var->Attributes & 0x4)
>  		str += sprintf(str, "EFI_VARIABLE_RUNTIME_ACCESS\n");

I know you're just following the pattern of the existing code, but could 
you change these to be more like

if (var->Attributes & EFI_VARIABLE_RUNTIME_ACCESS)

while you're at it? I think that'd be a worthwhile cleanup.

-- 
Matthew Garrett | mjg59@srcf.ucam.org

Re: [PATCH] Add missing attributes to EFI variable attribute print out from sysfs

[Khalid Aziz]

On 07/13/2012 09:46 AM, Matthew Garrett wrote:
> On Fri, Jul 13, 2012 at 09:38:27AM -0600, Khalid Aziz wrote:
>>   	if (var->Attributes & 0x4)
>>   		str += sprintf(str, "EFI_VARIABLE_RUNTIME_ACCESS\n");
> I know you're just following the pattern of the existing code, but could
> you change these to be more like
>
> if (var->Attributes & EFI_VARIABLE_RUNTIME_ACCESS)
>
> while you're at it? I think that'd be a worthwhile cleanup.
>

That is a good suggestion. I will make the changes and send out updated 
version.

-- 
Khalid Aziz
khalid.aziz@hp.com

Re: [PATCH] Add missing attributes to EFI variable attribute print out from sysfs

[Khalid Aziz]

On 07/13/2012 09:46 AM, Matthew Garrett wrote:

> I know you're just following the pattern of the existing code, but could
> you change these to be more like
>
> if (var->Attributes & EFI_VARIABLE_RUNTIME_ACCESS)
>
> while you're at it? I think that'd be a worthwhile cleanup.
>
I just sent out an updated patch with this change. The constant names
for EFI variable attribute are pretty darn long and cause line wrap
fairly quickly. I would like to rename those constants from EFI_VARIABLE_*
to EFI_VAR_* and possibly shorten the overall name as well:

EFI_VARIABLE_NON_VOLATILE			-> EFI_VAR_NV
EFI_VARIABLE_BOOTSERVICE_ACCESS			-> EFI_VAR_BOOT
EFI_VARIABLE_RUNTIME_ACCESS			-> EFI_VAR_RUNTIME
EFI_VARIABLE_HARDWARE_ERROR_RECORD		-> EFI_VAR_HW_ERROR
EFI_VARIABLE_AUTHENTICATED_WRITE_ACCESS		-> EFI_VAR_AUTH_WRITE
EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS	-> EFI_VAR_TIMED_AUTH_WRITE
EFI_VARIABLE_APPEND_WRITE			-> EFI_VAR_APPEND

Sounds reasonable?

-- 
Khalid Aziz
khalid.aziz@hp.com

Re: [PATCH] Add missing attributes to EFI variable attribute print out from sysfs

[Matthew Garrett]

On Fri, Jul 13, 2012 at 11:52:15AM -0600, Khalid Aziz wrote:
> EFI_VARIABLE_NON_VOLATILE			-> EFI_VAR_NV
> EFI_VARIABLE_BOOTSERVICE_ACCESS			-> EFI_VAR_BOOT
> EFI_VARIABLE_RUNTIME_ACCESS			-> EFI_VAR_RUNTIME
> EFI_VARIABLE_HARDWARE_ERROR_RECORD		-> EFI_VAR_HW_ERROR
> EFI_VARIABLE_AUTHENTICATED_WRITE_ACCESS		-> EFI_VAR_AUTH_WRITE
> EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS	-> EFI_VAR_TIMED_AUTH_WRITE
> EFI_VARIABLE_APPEND_WRITE			-> EFI_VAR_APPEND
> 
> Sounds reasonable?

Sounds great, but sadly they're exposed to userspace so changing them 
would be a problem. Adding aliases would be ugly but workable?

-- 
Matthew Garrett | mjg59@srcf.ucam.org

Re: [PATCH] Add missing attributes to EFI variable attribute print out from sysfs

[Khalid Aziz]

On 07/13/2012 11:54 AM, Matthew Garrett wrote:
> On Fri, Jul 13, 2012 at 11:52:15AM -0600, Khalid Aziz wrote:
>> EFI_VARIABLE_NON_VOLATILE			-> EFI_VAR_NV
>> EFI_VARIABLE_BOOTSERVICE_ACCESS			-> EFI_VAR_BOOT
>> EFI_VARIABLE_RUNTIME_ACCESS			-> EFI_VAR_RUNTIME
>> EFI_VARIABLE_HARDWARE_ERROR_RECORD		-> EFI_VAR_HW_ERROR
>> EFI_VARIABLE_AUTHENTICATED_WRITE_ACCESS		-> EFI_VAR_AUTH_WRITE
>> EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS	-> EFI_VAR_TIMED_AUTH_WRITE
>> EFI_VARIABLE_APPEND_WRITE			-> EFI_VAR_APPEND
>>
>> Sounds reasonable?
> Sounds great, but sadly they're exposed to userspace so changing them
> would be a problem. Adding aliases would be ugly but workable?
>

Ugly but practical. I agree with creating alias. I can work up a patch that
creates the aliases in efi.h, and replaces all uses in kernel with shorter
names. This does not change ABI or API, so existing userspace programs will
continue to work.

-- 
Khalid Aziz
khalid.aziz@hp.com

Re: [PATCH] Add missing attributes to EFI variable attribute print out from sysfs

[Khalid Aziz]

On 07/13/2012 12:04 PM, Mike Waychison wrote:

> You can probably save the wrap by copying out var->Attributes into a local.

Yes, you are right. It will work in this case. I am considering the general
case of very long name for a constant causing issues with line wrap in
more complex code, or just plain being a pain to type such a long name (one
of these constants is ~50 characters).

--
Khalid Aziz
khalid.aziz@hp.com