Skip to contentSkip to conversation

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

PATCH In review Lakshmi Ramasubramanian 3 participants 8 messages Oct 19, 2020 – Oct 20, 2020 Keys-trusted

lore
Share & tools
lore
#0 lore raw

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

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().

---
 src/evmctl.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
src/evmctl.c +2 -1
index 7ad1150..d49988e 100644
@@ -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);
Signature · 1 line
2.28.0
#1 lore raw
Quoted 24 lines · Lakshmi Ramasubramanian

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().

---
  src/evmctl.c | 3 ++-
  1 file changed, 2 insertions(+), 1 deletion(-)
src/evmctl.c +0 -0
index 7ad1150..d49988e 100644

src/evmctl.c static unsigned char *file2bin(const char *file, const char *ext, int *size) · line 228

Quoted 9 lines
src/evmctl.c +2 -1
@@ -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

Reviewed-by on v1 1/1
#2 lore raw
In reply to Lakshmi Ramasubramanian

Hi,

Quoted 6 lines

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 );

Quoted 8 lines

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().

LGTM.

Kind regards,
Petr

Quoted 18 lines
---
 src/evmctl.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
src/evmctl.c +2 -1
index 7ad1150..d49988e 100644
@@ -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);
maintainer
#3 lore raw
In reply to Lakshmi Ramasubramanian

Hi Lakshmi,

Quoted 23 lines · Lakshmi Ramasubramanian

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().

---
 src/evmctl.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
src/evmctl.c +0 -0
index 7ad1150..d49988e 100644

src/evmctl.c static unsigned char *file2bin(const char *file, const char *ext, int *size) · line 228

Quoted 9 lines
src/evmctl.c +2 -1
@@ -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

#4 lore raw
Quoted 30 lines · Mimi Zohar

Hi Lakshmi,

Quoted 27 lines · Lakshmi Ramasubramanian

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().

---
  src/evmctl.c | 3 ++-
  1 file changed, 2 insertions(+), 1 deletion(-)
src/evmctl.c +0 -1
index 7ad1150..d49988e 100644
@@ -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) {
Quoted 9 lines

src/evmctl.c static unsigned char *file2bin(const char *file, const char *ext, int *size) · line 228

Quoted 5 lines
src/evmctl.c +2 -0
+
+	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

maintainer
#5 lore raw
Quoted 41 lines · Lakshmi Ramasubramanian
Quoted 30 lines · Mimi Zohar

Hi Lakshmi,

Quoted 27 lines · Lakshmi Ramasubramanian

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().

---
  src/evmctl.c | 3 ++-
  1 file changed, 2 insertions(+), 1 deletion(-)
src/evmctl.c +0 -1
index 7ad1150..d49988e 100644
@@ -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) {
Quoted 9 lines

src/evmctl.c static unsigned char *file2bin(const char *file, const char *ext, int *size) · line 228

Quoted 5 lines
src/evmctl.c +2 -0
+
+	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").

Quoted 5 lines

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

author Reviewed-by
#6 lore raw
Quoted 45 lines · Mimi Zohar
Quoted 44 lines · Lakshmi Ramasubramanian
Quoted 30 lines · Mimi Zohar

Hi Lakshmi,

Quoted 27 lines · Lakshmi Ramasubramanian

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().

---
   src/evmctl.c | 3 ++-
   1 file changed, 2 insertions(+), 1 deletion(-)
src/evmctl.c +0 -1
index 7ad1150..d49988e 100644
@@ -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) {
Quoted 9 lines

src/evmctl.c static unsigned char *file2bin(const char *file, const char *ext, int *size) · line 228

Quoted 5 lines
src/evmctl.c +2 -0
+
+	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.

Quoted 6 lines
Quoted 2 lines

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.

-lakshmi

#7 lore raw
In reply to Lakshmi Ramasubramanian

Hi Lakshmi,

Quoted 9 lines
Quoted 2 lines

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

Quoted 2 lines

thanks,
-lakshmi

Kind regards,
Petr