[PATCH] uefi: open efi_runtime driver with flag O_WRONLY | O_RDWR

[Colin Ian King]

On 12/11/14 03:18, Alex Hung wrote:
> Ivan,
> 
> From cking's blog, it replaces "O_NOACCESS 3" with "O_WRONLY | O_RDWR"
> (=3). However, your are replacing "O_RDONLY 0" with "O_WRONLY | O_RDWR".
> Is this really intended?

(O_WRONLY | O_RDWR) (also known as O_NOACCESS but not defined in
fcntl.h) allows us to perform just the ioctl() and disallows us from
accidentally performing reads and writes on the device.  Replacing
O_RDONLY with (O_WRONLY | O_RDWR) is intentional with this patch set as
it stops is from accidentally reading the device and just allows ioctl()
access.

It is obscure. It is a poorly documented in open() - one has to read the
kernel source, but it is a nice little security feature.

Colin

> 
> Cheers,
> Alex Hung
> 
> On 14-11-05 03:29 PM, Ivan Hu wrote:
>> The efi_runtime driver just doing ioctl() calls, using the flag
>> (O_WRONLY | O_RDWR) make sure it allows just ioctl() calls and NO
>> accidental
>> read or writes to the device.
>>
>> http://smackerelofopinion.blogspot.co.uk/2012/01/open-using-owronly-ordwr.html
>>
>>
>> Signed-off-by: Ivan Hu <ivan.hu at canonical.com>
>> ---
>>   src/uefi/uefirtmisc/uefirtmisc.c         |    2 +-
>>   src/uefi/uefirttime/uefirttime.c         |    2 +-
>>   src/uefi/uefirtvariable/uefirtvariable.c |    2 +-
>>   src/uefi/uefivarinfo/uefivarinfo.c       |    2 +-
>>   4 files changed, 4 insertions(+), 4 deletions(-)
>>
>> diff --git a/src/uefi/uefirtmisc/uefirtmisc.c
>> b/src/uefi/uefirtmisc/uefirtmisc.c
>> index bba468e..caafca5 100644
>> --- a/src/uefi/uefirtmisc/uefirtmisc.c
>> +++ b/src/uefi/uefirtmisc/uefirtmisc.c
>> @@ -53,7 +53,7 @@ static int uefirtmisc_init(fwts_framework *fw)
>>           return FWTS_ABORTED;
>>       }
>>   -    fd = open("/dev/efi_runtime", O_RDONLY);
>> +    fd = open("/dev/efi_runtime", O_WRONLY | O_RDWR);
>>       if (fd == -1) {
>>           fwts_log_info(fw, "Cannot open efi_runtime driver. Aborted.");
>>           return FWTS_ABORTED;
>> diff --git a/src/uefi/uefirttime/uefirttime.c
>> b/src/uefi/uefirttime/uefirttime.c
>> index 896e13f..a3125bd 100644
>> --- a/src/uefi/uefirttime/uefirttime.c
>> +++ b/src/uefi/uefirttime/uefirttime.c
>> @@ -175,7 +175,7 @@ static int uefirttime_init(fwts_framework *fw)
>>           return FWTS_ABORTED;
>>       }
>>   -    fd = open("/dev/efi_runtime", O_RDONLY);
>> +    fd = open("/dev/efi_runtime", O_WRONLY | O_RDWR);
>>       if (fd == -1) {
>>           fwts_log_info(fw, "Cannot open efi_runtime driver. Aborted.");
>>           return FWTS_ABORTED;
>> diff --git a/src/uefi/uefirtvariable/uefirtvariable.c
>> b/src/uefi/uefirtvariable/uefirtvariable.c
>> index f0fd0ce..a19f835 100644
>> --- a/src/uefi/uefirtvariable/uefirtvariable.c
>> +++ b/src/uefi/uefirtvariable/uefirtvariable.c
>> @@ -99,7 +99,7 @@ static int uefirtvariable_init(fwts_framework *fw)
>>           return FWTS_ABORTED;
>>       }
>>   -    fd = open("/dev/efi_runtime", O_RDONLY);
>> +    fd = open("/dev/efi_runtime", O_WRONLY | O_RDWR);
>>       if (fd == -1) {
>>           fwts_log_info(fw, "Cannot open efi_runtime driver. Aborted.");
>>           return FWTS_ABORTED;
>> diff --git a/src/uefi/uefivarinfo/uefivarinfo.c
>> b/src/uefi/uefivarinfo/uefivarinfo.c
>> index 41296c6..7310931 100644
>> --- a/src/uefi/uefivarinfo/uefivarinfo.c
>> +++ b/src/uefi/uefivarinfo/uefivarinfo.c
>> @@ -44,7 +44,7 @@ static int uefivarinfo_init(fwts_framework *fw)
>>           return FWTS_ABORTED;
>>       }
>>   -    fd = open("/dev/efi_runtime", O_RDONLY);
>> +    fd = open("/dev/efi_runtime", O_WRONLY | O_RDWR);
>>       if (fd == -1) {
>>           fwts_log_info(fw, "Cannot open efi_runtime driver. Aborted.");
>>           return FWTS_ABORTED;
> 
>