2019-10-08 10:02:30

by Colin King

[permalink] [raw]
Subject: [PATCH][next] efi/tpm: fix sanity check of unsigned tbl_size being less than zero

From: Colin Ian King <[email protected]>

Currently the check for tbl_size being less than zero is always false
because tbl_size is unsigned. Fix this by making it a signed int.

Addresses-Coverity: ("Unsigned compared against 0")
Fixes: e658c82be556 ("efi/tpm: Only set 'efi_tpm_final_log_size' after successful event log parsing")
Signed-off-by: Colin Ian King <[email protected]>
---
drivers/firmware/efi/tpm.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/firmware/efi/tpm.c b/drivers/firmware/efi/tpm.c
index 703469c1ab8e..ebd7977653a8 100644
--- a/drivers/firmware/efi/tpm.c
+++ b/drivers/firmware/efi/tpm.c
@@ -40,7 +40,7 @@ int __init efi_tpm_eventlog_init(void)
{
struct linux_efi_tpm_eventlog *log_tbl;
struct efi_tcg2_final_events_table *final_tbl;
- unsigned int tbl_size;
+ int tbl_size;
int ret = 0;

if (efi.tpm_log == EFI_INVALID_TABLE_ADDR) {
--
2.20.1


Subject: [tip: efi/urgent] efi/tpm: Fix sanity check of unsigned tbl_size being less than zero

The following commit has been merged into the efi/urgent branch of tip:

Commit-ID: be59d57f98065af0b8472f66a0a969207b168680
Gitweb: https://git.kernel.org/tip/be59d57f98065af0b8472f66a0a969207b168680
Author: Colin Ian King <[email protected]>
AuthorDate: Tue, 08 Oct 2019 11:01:53 +01:00
Committer: Ingo Molnar <[email protected]>
CommitterDate: Tue, 08 Oct 2019 13:01:09 +02:00

efi/tpm: Fix sanity check of unsigned tbl_size being less than zero

Currently the check for tbl_size being less than zero is always false
because tbl_size is unsigned. Fix this by making it a signed int.

Addresses-Coverity: ("Unsigned compared against 0")
Signed-off-by: Colin Ian King <[email protected]>
Cc: Ard Biesheuvel <[email protected]>
Cc: Jerry Snitselaar <[email protected]>
Cc: Linus Torvalds <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Thomas Gleixner <[email protected]>
Cc: [email protected]
Cc: [email protected]
Fixes: e658c82be556 ("efi/tpm: Only set 'efi_tpm_final_log_size' after successful event log parsing")
Link: https://lkml.kernel.org/r/[email protected]
Signed-off-by: Ingo Molnar <[email protected]>
---
drivers/firmware/efi/tpm.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/firmware/efi/tpm.c b/drivers/firmware/efi/tpm.c
index 703469c..ebd7977 100644
--- a/drivers/firmware/efi/tpm.c
+++ b/drivers/firmware/efi/tpm.c
@@ -40,7 +40,7 @@ int __init efi_tpm_eventlog_init(void)
{
struct linux_efi_tpm_eventlog *log_tbl;
struct efi_tcg2_final_events_table *final_tbl;
- unsigned int tbl_size;
+ int tbl_size;
int ret = 0;

if (efi.tpm_log == EFI_INVALID_TABLE_ADDR) {

2019-10-08 11:49:33

by Dan Carpenter

[permalink] [raw]
Subject: Re: [PATCH][next] efi/tpm: fix sanity check of unsigned tbl_size being less than zero

On Tue, Oct 08, 2019 at 11:01:53AM +0100, Colin King wrote:
> From: Colin Ian King <[email protected]>
>
> Currently the check for tbl_size being less than zero is always false
> because tbl_size is unsigned. Fix this by making it a signed int.
>
> Addresses-Coverity: ("Unsigned compared against 0")
> Fixes: e658c82be556 ("efi/tpm: Only set 'efi_tpm_final_log_size' after successful event log parsing")
> Signed-off-by: Colin Ian King <[email protected]>
> ---
> drivers/firmware/efi/tpm.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/firmware/efi/tpm.c b/drivers/firmware/efi/tpm.c
> index 703469c1ab8e..ebd7977653a8 100644
> --- a/drivers/firmware/efi/tpm.c
> +++ b/drivers/firmware/efi/tpm.c
> @@ -40,7 +40,7 @@ int __init efi_tpm_eventlog_init(void)
> {
> struct linux_efi_tpm_eventlog *log_tbl;
> struct efi_tcg2_final_events_table *final_tbl;
> - unsigned int tbl_size;
> + int tbl_size;
> int ret = 0;


Do we need to do a "ret = tbl_size;"? Currently we return success.
It's a pitty that tpm2_calc_event_log_size() returns a -1 instead of
-EINVAL.

regards,
dan carpenter

2019-10-08 15:49:15

by Jerry Snitselaar

[permalink] [raw]
Subject: Re: [PATCH][next] efi/tpm: fix sanity check of unsigned tbl_size being less than zero

On Tue Oct 08 19, Colin King wrote:
>From: Colin Ian King <[email protected]>
>
>Currently the check for tbl_size being less than zero is always false
>because tbl_size is unsigned. Fix this by making it a signed int.
>
>Addresses-Coverity: ("Unsigned compared against 0")
>Fixes: e658c82be556 ("efi/tpm: Only set 'efi_tpm_final_log_size' after successful event log parsing")
>Signed-off-by: Colin Ian King <[email protected]>
>---
> drivers/firmware/efi/tpm.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
>diff --git a/drivers/firmware/efi/tpm.c b/drivers/firmware/efi/tpm.c
>index 703469c1ab8e..ebd7977653a8 100644
>--- a/drivers/firmware/efi/tpm.c
>+++ b/drivers/firmware/efi/tpm.c
>@@ -40,7 +40,7 @@ int __init efi_tpm_eventlog_init(void)
> {
> struct linux_efi_tpm_eventlog *log_tbl;
> struct efi_tcg2_final_events_table *final_tbl;
>- unsigned int tbl_size;
>+ int tbl_size;
> int ret = 0;
>
> if (efi.tpm_log == EFI_INVALID_TABLE_ADDR) {
>--
>2.20.1
>

Thanks for catching that. Somehow I dropped it from v2 to v3.

2019-10-08 16:20:13

by Jerry Snitselaar

[permalink] [raw]
Subject: Re: [PATCH][next] efi/tpm: fix sanity check of unsigned tbl_size being less than zero

On Tue Oct 08 19, Dan Carpenter wrote:
>On Tue, Oct 08, 2019 at 11:01:53AM +0100, Colin King wrote:
>> From: Colin Ian King <[email protected]>
>>
>> Currently the check for tbl_size being less than zero is always false
>> because tbl_size is unsigned. Fix this by making it a signed int.
>>
>> Addresses-Coverity: ("Unsigned compared against 0")
>> Fixes: e658c82be556 ("efi/tpm: Only set 'efi_tpm_final_log_size' after successful event log parsing")
>> Signed-off-by: Colin Ian King <[email protected]>
>> ---
>> drivers/firmware/efi/tpm.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/firmware/efi/tpm.c b/drivers/firmware/efi/tpm.c
>> index 703469c1ab8e..ebd7977653a8 100644
>> --- a/drivers/firmware/efi/tpm.c
>> +++ b/drivers/firmware/efi/tpm.c
>> @@ -40,7 +40,7 @@ int __init efi_tpm_eventlog_init(void)
>> {
>> struct linux_efi_tpm_eventlog *log_tbl;
>> struct efi_tcg2_final_events_table *final_tbl;
>> - unsigned int tbl_size;
>> + int tbl_size;
>> int ret = 0;
>
>
>Do we need to do a "ret = tbl_size;"? Currently we return success.
>It's a pitty that tpm2_calc_event_log_size() returns a -1 instead of
>-EINVAL.
>
>regards,
>dan carpenter
>

perhaps "ret = -EINVAL;"? Currently nothing checks the return value of efi_tpm_eventlog_init though.

2019-10-08 16:25:52

by Colin King

[permalink] [raw]
Subject: Re: [PATCH][next] efi/tpm: fix sanity check of unsigned tbl_size being less than zero

On 08/10/2019 17:15, Jerry Snitselaar wrote:
> On Tue Oct 08 19, Dan Carpenter wrote:
>> On Tue, Oct 08, 2019 at 11:01:53AM +0100, Colin King wrote:
>>> From: Colin Ian King <[email protected]>
>>>
>>> Currently the check for tbl_size being less than zero is always false
>>> because tbl_size is unsigned. Fix this by making it a signed int.
>>>
>>> Addresses-Coverity: ("Unsigned compared against 0")
>>> Fixes: e658c82be556 ("efi/tpm: Only set 'efi_tpm_final_log_size'
>>> after successful event log parsing")
>>> Signed-off-by: Colin Ian King <[email protected]>
>>> ---
>>> ?drivers/firmware/efi/tpm.c | 2 +-
>>> ?1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/firmware/efi/tpm.c b/drivers/firmware/efi/tpm.c
>>> index 703469c1ab8e..ebd7977653a8 100644
>>> --- a/drivers/firmware/efi/tpm.c
>>> +++ b/drivers/firmware/efi/tpm.c
>>> @@ -40,7 +40,7 @@ int __init efi_tpm_eventlog_init(void)
>>> ?{
>>> ???? struct linux_efi_tpm_eventlog *log_tbl;
>>> ???? struct efi_tcg2_final_events_table *final_tbl;
>>> -??? unsigned int tbl_size;
>>> +??? int tbl_size;
>>> ???? int ret = 0;
>>
>>
>> Do we need to do a "ret = tbl_size;"?? Currently we return success.
>> It's a pitty that tpm2_calc_event_log_size() returns a -1 instead of
>> -EINVAL.
>>
>> regards,
>> dan carpenter
>>
>
> perhaps "ret = -EINVAL;"? Currently nothing checks the return value of
> efi_tpm_eventlog_init though.

I doubt I'll fix that for my current fix as a V2.