diff mbox series

file2bin: Pass the right values to size and count parameters for fread()

Message ID 20201019200526.12678-1-nramas@linux.microsoft.com (mailing list archive)
State New, archived
Headers show
Series file2bin: Pass the right values to size and count parameters for fread() | expand

Commit Message

Lakshmi Ramasubramanian Oct. 19, 2020, 8:05 p.m. UTC
The 2nd parameter to fread() namely "size" specifies the size, in
bytes of each element to be read, and the 3rd parameter namely "count"
specifies the number of elements, each one with a size of "size" bytes.

 size_t fread ( void * ptr, size_t size, size_t count, FILE * stream );

But in the function file2bin() the values passed to "size" and "count"
are reversed causing the function to return an error eventhough the file
was sucdessfully read.

Pass the right values to "size" and "count" parameters for fread() in
the function file2bin().

Signed-off-by: Lakshmi Ramasubramanian <nramas@linux.microsoft.com>
---
 src/evmctl.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Comments

Lakshmi Ramasubramanian Oct. 19, 2020, 8:08 p.m. UTC | #1
On 10/19/20 1:05 PM, Lakshmi Ramasubramanian wrote:
> The 2nd parameter to fread() namely "size" specifies the size, in
> bytes of each element to be read, and the 3rd parameter namely "count"
> specifies the number of elements, each one with a size of "size" bytes.
> 
>   size_t fread ( void * ptr, size_t size, size_t count, FILE * stream );
> 
> But in the function file2bin() the values passed to "size" and "count"
> are reversed causing the function to return an error eventhough the file
> was sucdessfully read.
> 
> Pass the right values to "size" and "count" parameters for fread() in
> the function file2bin().
> 
> Signed-off-by: Lakshmi Ramasubramanian <nramas@linux.microsoft.com>
> ---
>   src/evmctl.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/src/evmctl.c b/src/evmctl.c
> index 7ad1150..d49988e 100644
> --- a/src/evmctl.c
> +++ b/src/evmctl.c
> @@ -221,7 +221,8 @@ static unsigned char *file2bin(const char *file, const char *ext, int *size)
>   		fclose(fp);
>   		return NULL;
>   	}
> -	if (fread(data, len, 1, fp) != len) {
> +
> +	if (fread(data, 1, len, fp) != len) {
>   		log_err("Failed to fread %zu bytes: %s\n", len, name);
>   		fclose(fp);
>   		free(data);
> 

The above patch is for ima-evm-utils. Missed adding "[ima-evm-utils]" in 
the subject.

thanks,
  -lakshmi
Petr Vorel Oct. 19, 2020, 8:23 p.m. UTC | #2
Hi,

> The 2nd parameter to fread() namely "size" specifies the size, in
> bytes of each element to be read, and the 3rd parameter namely "count"
> specifies the number of elements, each one with a size of "size" bytes.

>  size_t fread ( void * ptr, size_t size, size_t count, FILE * stream );

> But in the function file2bin() the values passed to "size" and "count"
> are reversed causing the function to return an error eventhough the file
> was sucdessfully read.

> Pass the right values to "size" and "count" parameters for fread() in
> the function file2bin().

> Signed-off-by: Lakshmi Ramasubramanian <nramas@linux.microsoft.com>
Reviewed-by: Petr Vorel <pvorel@suse.cz>
LGTM.


Kind regards,
Petr

> ---
>  src/evmctl.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)

> diff --git a/src/evmctl.c b/src/evmctl.c
> index 7ad1150..d49988e 100644
> --- a/src/evmctl.c
> +++ b/src/evmctl.c
> @@ -221,7 +221,8 @@ static unsigned char *file2bin(const char *file, const char *ext, int *size)
>  		fclose(fp);
>  		return NULL;
>  	}
> -	if (fread(data, len, 1, fp) != len) {
> +
> +	if (fread(data, 1, len, fp) != len) {
>  		log_err("Failed to fread %zu bytes: %s\n", len, name);
>  		fclose(fp);
>  		free(data);
Mimi Zohar Oct. 19, 2020, 10:12 p.m. UTC | #3
Hi Lakshmi,

On Mon, 2020-10-19 at 13:05 -0700, Lakshmi Ramasubramanian wrote:
> The 2nd parameter to fread() namely "size" specifies the size, in
> bytes of each element to be read, and the 3rd parameter namely "count"
> specifies the number of elements, each one with a size of "size" bytes.
> 
>  size_t fread ( void * ptr, size_t size, size_t count, FILE * stream );
> 
> But in the function file2bin() the values passed to "size" and "count"
> are reversed causing the function to return an error eventhough the file
> was sucdessfully read.
> 
> Pass the right values to "size" and "count" parameters for fread() in
> the function file2bin().
> 
> Signed-off-by: Lakshmi Ramasubramanian <nramas@linux.microsoft.com>
> ---
>  src/evmctl.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/src/evmctl.c b/src/evmctl.c
> index 7ad1150..d49988e 100644
> --- a/src/evmctl.c
> +++ b/src/evmctl.c
> @@ -221,7 +221,8 @@ static unsigned char *file2bin(const char *file, const char *ext, int *size)
>  		fclose(fp);
>  		return NULL;
>  	}
> -	if (fread(data, len, 1, fp) != len) {
> +
> +	if (fread(data, 1, len, fp) != len) {
>  		log_err("Failed to fread %zu bytes: %s\n", len, name);
>  		fclose(fp);
>  		free(data);

Wasn't this problem addressed by Vitaly's patch.  Please look at commit
c89e8508864b ("ima-evm-utils: Fix reading of sigfile").

Mimi
Lakshmi Ramasubramanian Oct. 19, 2020, 10:22 p.m. UTC | #4
On 10/19/20 3:12 PM, Mimi Zohar wrote:
> Hi Lakshmi,
> 
> On Mon, 2020-10-19 at 13:05 -0700, Lakshmi Ramasubramanian wrote:
>> The 2nd parameter to fread() namely "size" specifies the size, in
>> bytes of each element to be read, and the 3rd parameter namely "count"
>> specifies the number of elements, each one with a size of "size" bytes.
>>
>>   size_t fread ( void * ptr, size_t size, size_t count, FILE * stream );
>>
>> But in the function file2bin() the values passed to "size" and "count"
>> are reversed causing the function to return an error eventhough the file
>> was sucdessfully read.
>>
>> Pass the right values to "size" and "count" parameters for fread() in
>> the function file2bin().
>>
>> Signed-off-by: Lakshmi Ramasubramanian <nramas@linux.microsoft.com>
>> ---
>>   src/evmctl.c | 3 ++-
>>   1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/src/evmctl.c b/src/evmctl.c
>> index 7ad1150..d49988e 100644
>> --- a/src/evmctl.c
>> +++ b/src/evmctl.c
>> @@ -221,7 +221,8 @@ static unsigned char *file2bin(const char *file, const char *ext, int *size)
>>   		fclose(fp);
>>   		return NULL;
>>   	}
>> -	if (fread(data, len, 1, fp) != len) {
>> +
>> +	if (fread(data, 1, len, fp) != len) {
>>   		log_err("Failed to fread %zu bytes: %s\n", len, name);
>>   		fclose(fp);
>>   		free(data);
> 
> Wasn't this problem addressed by Vitaly's patch.  Please look at commit
> c89e8508864b ("ima-evm-utils: Fix reading of sigfile").
> 

You are right Mimi. I missed the patch posted by Vitaly. Sorry for the 
duplicate one.

Looks like Vitaly's change hasn't been merged to "master" branch yet in 
https://github.com/pevik/ima-evm-utils

thanks,
  -lakshmi
Mimi Zohar Oct. 19, 2020, 10:30 p.m. UTC | #5
On Mon, 2020-10-19 at 15:22 -0700, Lakshmi Ramasubramanian wrote:
> On 10/19/20 3:12 PM, Mimi Zohar wrote:
> > Hi Lakshmi,
> > 
> > On Mon, 2020-10-19 at 13:05 -0700, Lakshmi Ramasubramanian wrote:
> >> The 2nd parameter to fread() namely "size" specifies the size, in
> >> bytes of each element to be read, and the 3rd parameter namely "count"
> >> specifies the number of elements, each one with a size of "size" bytes.
> >>
> >>   size_t fread ( void * ptr, size_t size, size_t count, FILE * stream );
> >>
> >> But in the function file2bin() the values passed to "size" and "count"
> >> are reversed causing the function to return an error eventhough the file
> >> was sucdessfully read.
> >>
> >> Pass the right values to "size" and "count" parameters for fread() in
> >> the function file2bin().
> >>
> >> Signed-off-by: Lakshmi Ramasubramanian <nramas@linux.microsoft.com>
> >> ---
> >>   src/evmctl.c | 3 ++-
> >>   1 file changed, 2 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/src/evmctl.c b/src/evmctl.c
> >> index 7ad1150..d49988e 100644
> >> --- a/src/evmctl.c
> >> +++ b/src/evmctl.c
> >> @@ -221,7 +221,8 @@ static unsigned char *file2bin(const char *file, const char *ext, int *size)
> >>   		fclose(fp);
> >>   		return NULL;
> >>   	}
> >> -	if (fread(data, len, 1, fp) != len) {
> >> +
> >> +	if (fread(data, 1, len, fp) != len) {
> >>   		log_err("Failed to fread %zu bytes: %s\n", len, name);
> >>   		fclose(fp);
> >>   		free(data);
> > 
> > Wasn't this problem addressed by Vitaly's patch.  Please look at commit
> > c89e8508864b ("ima-evm-utils: Fix reading of sigfile").
> > 
> 
> You are right Mimi. I missed the patch posted by Vitaly. Sorry for the 
> duplicate one.
> 
> Looks like Vitaly's change hasn't been merged to "master" branch yet in 
> https://github.com/pevik/ima-evm-utils

Only after the release would it be in master.   Until then it would be
in next, which it isn't either.   Can I add your Reviewed-by tag on
this patched? 

thanks,

Mimi
Lakshmi Ramasubramanian Oct. 19, 2020, 10:59 p.m. UTC | #6
On 10/19/20 3:30 PM, Mimi Zohar wrote:
> On Mon, 2020-10-19 at 15:22 -0700, Lakshmi Ramasubramanian wrote:
>> On 10/19/20 3:12 PM, Mimi Zohar wrote:
>>> Hi Lakshmi,
>>>
>>> On Mon, 2020-10-19 at 13:05 -0700, Lakshmi Ramasubramanian wrote:
>>>> The 2nd parameter to fread() namely "size" specifies the size, in
>>>> bytes of each element to be read, and the 3rd parameter namely "count"
>>>> specifies the number of elements, each one with a size of "size" bytes.
>>>>
>>>>    size_t fread ( void * ptr, size_t size, size_t count, FILE * stream );
>>>>
>>>> But in the function file2bin() the values passed to "size" and "count"
>>>> are reversed causing the function to return an error eventhough the file
>>>> was sucdessfully read.
>>>>
>>>> Pass the right values to "size" and "count" parameters for fread() in
>>>> the function file2bin().
>>>>
>>>> Signed-off-by: Lakshmi Ramasubramanian <nramas@linux.microsoft.com>
>>>> ---
>>>>    src/evmctl.c | 3 ++-
>>>>    1 file changed, 2 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/src/evmctl.c b/src/evmctl.c
>>>> index 7ad1150..d49988e 100644
>>>> --- a/src/evmctl.c
>>>> +++ b/src/evmctl.c
>>>> @@ -221,7 +221,8 @@ static unsigned char *file2bin(const char *file, const char *ext, int *size)
>>>>    		fclose(fp);
>>>>    		return NULL;
>>>>    	}
>>>> -	if (fread(data, len, 1, fp) != len) {
>>>> +
>>>> +	if (fread(data, 1, len, fp) != len) {
>>>>    		log_err("Failed to fread %zu bytes: %s\n", len, name);
>>>>    		fclose(fp);
>>>>    		free(data);
>>>
>>> Wasn't this problem addressed by Vitaly's patch.  Please look at commit
>>> c89e8508864b ("ima-evm-utils: Fix reading of sigfile").
>>>
>>
>> You are right Mimi. I missed the patch posted by Vitaly. Sorry for the
>> duplicate one.
>>
>> Looks like Vitaly's change hasn't been merged to "master" branch yet in
>> https://github.com/pevik/ima-evm-utils
> 
> Only after the release would it be in master.   Until then it would be
> in next, which it isn't either.   Can I add your Reviewed-by tag on
> this patched?

Sure.

Reviewed-by: Lakshmi Ramasubramanian <nramas@linux.microsoft.com>

  -lakshmi
Petr Vorel Oct. 20, 2020, 6:53 a.m. UTC | #7
Hi Lakshmi,

> > Wasn't this problem addressed by Vitaly's patch.  Please look at commit
> > c89e8508864b ("ima-evm-utils: Fix reading of sigfile").


> You are right Mimi. I missed the patch posted by Vitaly. Sorry for the
> duplicate one.

> Looks like Vitaly's change hasn't been merged to "master" branch yet in
> https://github.com/pevik/ima-evm-utils
FYI: this is my fork, although I update it regularly, next time please check
with the real upstream: https://git.code.sf.net/p/linux-ima/ima-evm-utils

> thanks,
>  -lakshmi

Kind regards,
Petr
diff mbox series

Patch

diff --git a/src/evmctl.c b/src/evmctl.c
index 7ad1150..d49988e 100644
--- a/src/evmctl.c
+++ b/src/evmctl.c
@@ -221,7 +221,8 @@  static unsigned char *file2bin(const char *file, const char *ext, int *size)
 		fclose(fp);
 		return NULL;
 	}
-	if (fread(data, len, 1, fp) != len) {
+
+	if (fread(data, 1, len, fp) != len) {
 		log_err("Failed to fread %zu bytes: %s\n", len, name);
 		fclose(fp);
 		free(data);