Re: [PATCH] file2bin: Pass the right values to size and count parameters for fread()

[Lakshmi Ramasubramanian <nramas@linux.microsoft.com>]

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