Linux-EFI Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH] efi: mokvar-table: fix some issues in new code
@ 2020-09-24 16:05 Ard Biesheuvel
  2020-09-24 19:12 ` Arvind Sankar
  2020-09-29 16:58 ` Borislav Petkov
  0 siblings, 2 replies; 5+ messages in thread
From: Ard Biesheuvel @ 2020-09-24 16:05 UTC (permalink / raw)
  To: linux-efi; +Cc: Ard Biesheuvel, Lenny Szubowicz, Borislav Petkov, Arvind Sankar

Fix a couple of issues in the new mokvar-table handling code, as
pointed out by Arvind and Boris:
- don't bother checking the end of the physical region against the start
  address of the mokvar table,
- ensure that we enter the loop with err = -EINVAL,
- replace size_t with unsigned long to appease pedantic type equality
  checks.

Cc: Lenny Szubowicz <lszubowi@redhat.com>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Arvind Sankar <nivedita@alum.mit.edu>
Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
 drivers/firmware/efi/mokvar-table.c | 25 +++++++++++--------------
 1 file changed, 11 insertions(+), 14 deletions(-)

diff --git a/drivers/firmware/efi/mokvar-table.c b/drivers/firmware/efi/mokvar-table.c
index b1cd49893d4d..72a9e1736fef 100644
--- a/drivers/firmware/efi/mokvar-table.c
+++ b/drivers/firmware/efi/mokvar-table.c
@@ -98,15 +98,14 @@ static struct kobject *mokvar_kobj;
 void __init efi_mokvar_table_init(void)
 {
 	efi_memory_desc_t md;
-	u64 end_pa;
 	void *va = NULL;
-	size_t cur_offset = 0;
-	size_t offset_limit;
-	size_t map_size = 0;
-	size_t map_size_needed = 0;
-	size_t size;
+	unsigned long cur_offset = 0;
+	unsigned long offset_limit;
+	unsigned long map_size = 0;
+	unsigned long map_size_needed = 0;
+	unsigned long size;
 	struct efi_mokvar_table_entry *mokvar_entry;
-	int err = -EINVAL;
+	int err;
 
 	if (!efi_enabled(EFI_MEMMAP))
 		return;
@@ -122,18 +121,16 @@ void __init efi_mokvar_table_init(void)
 		pr_warn("EFI MOKvar config table is not within the EFI memory map\n");
 		return;
 	}
-	end_pa = efi_mem_desc_end(&md);
-	if (efi.mokvar_table >= end_pa) {
-		pr_err("EFI memory descriptor containing MOKvar config table is invalid\n");
-		return;
-	}
-	offset_limit = end_pa - efi.mokvar_table;
+
+	offset_limit = efi_mem_desc_end(&md) - efi.mokvar_table;
+
 	/*
 	 * Validate the MOK config table. Since there is no table header
 	 * from which we could get the total size of the MOK config table,
 	 * we compute the total size as we validate each variably sized
 	 * entry, remapping as necessary.
 	 */
+	err = -EINVAL;
 	while (cur_offset + sizeof(*mokvar_entry) <= offset_limit) {
 		mokvar_entry = va + cur_offset;
 		map_size_needed = cur_offset + sizeof(*mokvar_entry);
@@ -150,7 +147,7 @@ void __init efi_mokvar_table_init(void)
 				       offset_limit);
 			va = early_memremap(efi.mokvar_table, map_size);
 			if (!va) {
-				pr_err("Failed to map EFI MOKvar config table pa=0x%lx, size=%zu.\n",
+				pr_err("Failed to map EFI MOKvar config table pa=0x%lx, size=%lu.\n",
 				       efi.mokvar_table, map_size);
 				return;
 			}
-- 
2.17.1


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

* Re: [PATCH] efi: mokvar-table: fix some issues in new code
  2020-09-24 16:05 [PATCH] efi: mokvar-table: fix some issues in new code Ard Biesheuvel
@ 2020-09-24 19:12 ` Arvind Sankar
  2020-09-24 19:21   ` Lenny Szubowicz
  2020-09-29 16:58 ` Borislav Petkov
  1 sibling, 1 reply; 5+ messages in thread
From: Arvind Sankar @ 2020-09-24 19:12 UTC (permalink / raw)
  To: Ard Biesheuvel; +Cc: linux-efi, Lenny Szubowicz, Borislav Petkov, Arvind Sankar

On Thu, Sep 24, 2020 at 06:05:46PM +0200, Ard Biesheuvel wrote:
> Fix a couple of issues in the new mokvar-table handling code, as
> pointed out by Arvind and Boris:
> - don't bother checking the end of the physical region against the start
>   address of the mokvar table,
> - ensure that we enter the loop with err = -EINVAL,
> - replace size_t with unsigned long to appease pedantic type equality
>   checks.
> 
> Cc: Lenny Szubowicz <lszubowi@redhat.com>
> Cc: Borislav Petkov <bp@alien8.de>
> Cc: Arvind Sankar <nivedita@alum.mit.edu>
> Signed-off-by: Ard Biesheuvel <ardb@kernel.org>

Reviewed-by: Arvind Sankar <nivedita@alum.mit.edu>

> ---
>  drivers/firmware/efi/mokvar-table.c | 25 +++++++++++--------------
>  1 file changed, 11 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/firmware/efi/mokvar-table.c b/drivers/firmware/efi/mokvar-table.c
> index b1cd49893d4d..72a9e1736fef 100644
> --- a/drivers/firmware/efi/mokvar-table.c
> +++ b/drivers/firmware/efi/mokvar-table.c
> @@ -98,15 +98,14 @@ static struct kobject *mokvar_kobj;
>  void __init efi_mokvar_table_init(void)
>  {
>  	efi_memory_desc_t md;
> -	u64 end_pa;
>  	void *va = NULL;
> -	size_t cur_offset = 0;
> -	size_t offset_limit;
> -	size_t map_size = 0;
> -	size_t map_size_needed = 0;
> -	size_t size;
> +	unsigned long cur_offset = 0;
> +	unsigned long offset_limit;
> +	unsigned long map_size = 0;
> +	unsigned long map_size_needed = 0;
> +	unsigned long size;
>  	struct efi_mokvar_table_entry *mokvar_entry;
> -	int err = -EINVAL;
> +	int err;
>  
>  	if (!efi_enabled(EFI_MEMMAP))
>  		return;
> @@ -122,18 +121,16 @@ void __init efi_mokvar_table_init(void)
>  		pr_warn("EFI MOKvar config table is not within the EFI memory map\n");
>  		return;
>  	}
> -	end_pa = efi_mem_desc_end(&md);
> -	if (efi.mokvar_table >= end_pa) {
> -		pr_err("EFI memory descriptor containing MOKvar config table is invalid\n");
> -		return;
> -	}
> -	offset_limit = end_pa - efi.mokvar_table;
> +
> +	offset_limit = efi_mem_desc_end(&md) - efi.mokvar_table;
> +
>  	/*
>  	 * Validate the MOK config table. Since there is no table header
>  	 * from which we could get the total size of the MOK config table,
>  	 * we compute the total size as we validate each variably sized
>  	 * entry, remapping as necessary.
>  	 */
> +	err = -EINVAL;
>  	while (cur_offset + sizeof(*mokvar_entry) <= offset_limit) {
>  		mokvar_entry = va + cur_offset;
>  		map_size_needed = cur_offset + sizeof(*mokvar_entry);
> @@ -150,7 +147,7 @@ void __init efi_mokvar_table_init(void)
>  				       offset_limit);
>  			va = early_memremap(efi.mokvar_table, map_size);
>  			if (!va) {
> -				pr_err("Failed to map EFI MOKvar config table pa=0x%lx, size=%zu.\n",
> +				pr_err("Failed to map EFI MOKvar config table pa=0x%lx, size=%lu.\n",
>  				       efi.mokvar_table, map_size);
>  				return;
>  			}
> -- 
> 2.17.1
> 

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

* Re: [PATCH] efi: mokvar-table: fix some issues in new code
  2020-09-24 19:12 ` Arvind Sankar
@ 2020-09-24 19:21   ` Lenny Szubowicz
  0 siblings, 0 replies; 5+ messages in thread
From: Lenny Szubowicz @ 2020-09-24 19:21 UTC (permalink / raw)
  To: Arvind Sankar, Ard Biesheuvel; +Cc: linux-efi, Borislav Petkov

On 9/24/20 3:12 PM, Arvind Sankar wrote:
> On Thu, Sep 24, 2020 at 06:05:46PM +0200, Ard Biesheuvel wrote:
>> Fix a couple of issues in the new mokvar-table handling code, as
>> pointed out by Arvind and Boris:
>> - don't bother checking the end of the physical region against the start
>>    address of the mokvar table,
>> - ensure that we enter the loop with err = -EINVAL,
>> - replace size_t with unsigned long to appease pedantic type equality
>>    checks.
>>
>> Cc: Lenny Szubowicz <lszubowi@redhat.com>
>> Cc: Borislav Petkov <bp@alien8.de>
>> Cc: Arvind Sankar <nivedita@alum.mit.edu>
>> Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
> 
> Reviewed-by: Arvind Sankar <nivedita@alum.mit.edu>

Reviewed-by: Lenny Szubowicz <lszubowi@redhat.com>

> 
>> ---
>>   drivers/firmware/efi/mokvar-table.c | 25 +++++++++++--------------
>>   1 file changed, 11 insertions(+), 14 deletions(-)
>>
>> diff --git a/drivers/firmware/efi/mokvar-table.c b/drivers/firmware/efi/mokvar-table.c
>> index b1cd49893d4d..72a9e1736fef 100644
>> --- a/drivers/firmware/efi/mokvar-table.c
>> +++ b/drivers/firmware/efi/mokvar-table.c
>> @@ -98,15 +98,14 @@ static struct kobject *mokvar_kobj;
>>   void __init efi_mokvar_table_init(void)
>>   {
>>   	efi_memory_desc_t md;
>> -	u64 end_pa;
>>   	void *va = NULL;
>> -	size_t cur_offset = 0;
>> -	size_t offset_limit;
>> -	size_t map_size = 0;
>> -	size_t map_size_needed = 0;
>> -	size_t size;
>> +	unsigned long cur_offset = 0;
>> +	unsigned long offset_limit;
>> +	unsigned long map_size = 0;
>> +	unsigned long map_size_needed = 0;
>> +	unsigned long size;
>>   	struct efi_mokvar_table_entry *mokvar_entry;
>> -	int err = -EINVAL;
>> +	int err;
>>   
>>   	if (!efi_enabled(EFI_MEMMAP))
>>   		return;
>> @@ -122,18 +121,16 @@ void __init efi_mokvar_table_init(void)
>>   		pr_warn("EFI MOKvar config table is not within the EFI memory map\n");
>>   		return;
>>   	}
>> -	end_pa = efi_mem_desc_end(&md);
>> -	if (efi.mokvar_table >= end_pa) {
>> -		pr_err("EFI memory descriptor containing MOKvar config table is invalid\n");
>> -		return;
>> -	}
>> -	offset_limit = end_pa - efi.mokvar_table;
>> +
>> +	offset_limit = efi_mem_desc_end(&md) - efi.mokvar_table;
>> +
>>   	/*
>>   	 * Validate the MOK config table. Since there is no table header
>>   	 * from which we could get the total size of the MOK config table,
>>   	 * we compute the total size as we validate each variably sized
>>   	 * entry, remapping as necessary.
>>   	 */
>> +	err = -EINVAL;
>>   	while (cur_offset + sizeof(*mokvar_entry) <= offset_limit) {
>>   		mokvar_entry = va + cur_offset;
>>   		map_size_needed = cur_offset + sizeof(*mokvar_entry);
>> @@ -150,7 +147,7 @@ void __init efi_mokvar_table_init(void)
>>   				       offset_limit);
>>   			va = early_memremap(efi.mokvar_table, map_size);
>>   			if (!va) {
>> -				pr_err("Failed to map EFI MOKvar config table pa=0x%lx, size=%zu.\n",
>> +				pr_err("Failed to map EFI MOKvar config table pa=0x%lx, size=%lu.\n",
>>   				       efi.mokvar_table, map_size);
>>   				return;
>>   			}
>> -- 
>> 2.17.1
>>
> 


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

* Re: [PATCH] efi: mokvar-table: fix some issues in new code
  2020-09-24 16:05 [PATCH] efi: mokvar-table: fix some issues in new code Ard Biesheuvel
  2020-09-24 19:12 ` Arvind Sankar
@ 2020-09-29 16:58 ` Borislav Petkov
  2020-09-29 17:36   ` Ard Biesheuvel
  1 sibling, 1 reply; 5+ messages in thread
From: Borislav Petkov @ 2020-09-29 16:58 UTC (permalink / raw)
  To: Ard Biesheuvel; +Cc: linux-efi, Lenny Szubowicz, Arvind Sankar

On Thu, Sep 24, 2020 at 06:05:46PM +0200, Ard Biesheuvel wrote:
> Fix a couple of issues in the new mokvar-table handling code, as
> pointed out by Arvind and Boris:
> - don't bother checking the end of the physical region against the start
>   address of the mokvar table,
> - ensure that we enter the loop with err = -EINVAL,
> - replace size_t with unsigned long to appease pedantic type equality
>   checks.
> 
> Cc: Lenny Szubowicz <lszubowi@redhat.com>
> Cc: Borislav Petkov <bp@alien8.de>
> Cc: Arvind Sankar <nivedita@alum.mit.edu>
> Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
> ---
>  drivers/firmware/efi/mokvar-table.c | 25 +++++++++++--------------
>  1 file changed, 11 insertions(+), 14 deletions(-)

i386 allmodconfig looks good.

Tested-by: Borislav Petkov <bp@suse.de>

Thx.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

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

* Re: [PATCH] efi: mokvar-table: fix some issues in new code
  2020-09-29 16:58 ` Borislav Petkov
@ 2020-09-29 17:36   ` Ard Biesheuvel
  0 siblings, 0 replies; 5+ messages in thread
From: Ard Biesheuvel @ 2020-09-29 17:36 UTC (permalink / raw)
  To: Borislav Petkov; +Cc: linux-efi, Lenny Szubowicz, Arvind Sankar

On Tue, 29 Sep 2020 at 18:58, Borislav Petkov <bp@alien8.de> wrote:
>
> On Thu, Sep 24, 2020 at 06:05:46PM +0200, Ard Biesheuvel wrote:
> > Fix a couple of issues in the new mokvar-table handling code, as
> > pointed out by Arvind and Boris:
> > - don't bother checking the end of the physical region against the start
> >   address of the mokvar table,
> > - ensure that we enter the loop with err = -EINVAL,
> > - replace size_t with unsigned long to appease pedantic type equality
> >   checks.
> >
> > Cc: Lenny Szubowicz <lszubowi@redhat.com>
> > Cc: Borislav Petkov <bp@alien8.de>
> > Cc: Arvind Sankar <nivedita@alum.mit.edu>
> > Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
> > ---
> >  drivers/firmware/efi/mokvar-table.c | 25 +++++++++++--------------
> >  1 file changed, 11 insertions(+), 14 deletions(-)
>
> i386 allmodconfig looks good.
>
> Tested-by: Borislav Petkov <bp@suse.de>
>

Thanks Boris. I'll have a PR out shortly.

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

end of thread, back to index

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-09-24 16:05 [PATCH] efi: mokvar-table: fix some issues in new code Ard Biesheuvel
2020-09-24 19:12 ` Arvind Sankar
2020-09-24 19:21   ` Lenny Szubowicz
2020-09-29 16:58 ` Borislav Petkov
2020-09-29 17:36   ` Ard Biesheuvel

Linux-EFI Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-efi/0 linux-efi/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-efi linux-efi/ https://lore.kernel.org/linux-efi \
		linux-efi@vger.kernel.org
	public-inbox-index linux-efi

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-efi


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git