diff mbox

[U-Boot] tools: mkimage: Call fclose in error path

Message ID 20161220110145.f3f3io7r4sg5tvtf@lenoch
State Deferred
Delegated to: Tom Rini
Headers show

Commit Message

Ladislav Michl Dec. 20, 2016, 11:01 a.m. UTC 
Hi Michal,

On Tue, Dec 20, 2016 at 09:58:31AM +0100, Michal Simek wrote:
> This patch is fixing missing fclose() calls
> in error patch introduced by:
> "tools: mkimage: Use fstat instead of stat to avoid malicious hacks"
> (sha1: ebe0f53f48e8f9ecc823e533a85b05c13638c350)
> 
> Reported-by: Coverity (CID: 155064, 155065)
> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
> ---
> 
>  tools/zynqimage.c   | 8 ++++++--
>  tools/zynqmpimage.c | 8 ++++++--
>  2 files changed, 12 insertions(+), 4 deletions(-)
> 
> diff --git a/tools/zynqimage.c b/tools/zynqimage.c
> index b47132b02a60..021d2d3fc91f 100644
> --- a/tools/zynqimage.c
> +++ b/tools/zynqimage.c
> @@ -239,11 +239,15 @@ static void zynqimage_parse_initparams(struct zynq_header *zynqhdr,
>  	}
>  
>  	err = fstat(fileno(fp), &path_stat);
> -	if (err)
> +	if (err) {
> +		fclose(fp);
>  		return;
> +	}
>  
> -	if (!S_ISREG(path_stat.st_mode))
> +	if (!S_ISREG(path_stat.st_mode)) {
> +		fclose(fp);
>  		return;
> +	}
>  
>  	do {
>  		r = fscanf(fp, "%x %x", &reginit.address, &reginit.data);
> diff --git a/tools/zynqmpimage.c b/tools/zynqmpimage.c
> index 60d8ed23b4a1..0c9a3daddd6a 100644
> --- a/tools/zynqmpimage.c
> +++ b/tools/zynqmpimage.c
> @@ -251,11 +251,15 @@ static void zynqmpimage_parse_initparams(struct zynqmp_header *zynqhdr,
>  	}
>  
>  	err = fstat(fileno(fp), &path_stat);
> -	if (err)
> +	if (err) {
> +		fclose(fp);
>  		return;
> +	}
>  
> -	if (!S_ISREG(path_stat.st_mode))
> +	if (!S_ISREG(path_stat.st_mode)) {
> +		fclose(fp);
>  		return;
> +	}
>  
>  	do {
>  		r = fscanf(fp, "%x %x", &reginit.address, &reginit.data);

what about something like this?

Best regards,
	ladis (bored waiting for the lunch ;-))

Signed-off-by: Ladislav Michl <ladis@linux-mips.org>
---

Comments

Michal Simek Dec. 20, 2016, 12:50 p.m. UTC | #1 
On 20.12.2016 12:01, Ladislav Michl wrote:
> Hi Michal,
> 
> On Tue, Dec 20, 2016 at 09:58:31AM +0100, Michal Simek wrote:
>> This patch is fixing missing fclose() calls
>> in error patch introduced by:
>> "tools: mkimage: Use fstat instead of stat to avoid malicious hacks"
>> (sha1: ebe0f53f48e8f9ecc823e533a85b05c13638c350)
>>
>> Reported-by: Coverity (CID: 155064, 155065)
>> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
>> ---
>>
>>  tools/zynqimage.c   | 8 ++++++--
>>  tools/zynqmpimage.c | 8 ++++++--
>>  2 files changed, 12 insertions(+), 4 deletions(-)
>>
>> diff --git a/tools/zynqimage.c b/tools/zynqimage.c
>> index b47132b02a60..021d2d3fc91f 100644
>> --- a/tools/zynqimage.c
>> +++ b/tools/zynqimage.c
>> @@ -239,11 +239,15 @@ static void zynqimage_parse_initparams(struct zynq_header *zynqhdr,
>>  	}
>>  
>>  	err = fstat(fileno(fp), &path_stat);
>> -	if (err)
>> +	if (err) {
>> +		fclose(fp);
>>  		return;
>> +	}
>>  
>> -	if (!S_ISREG(path_stat.st_mode))
>> +	if (!S_ISREG(path_stat.st_mode)) {
>> +		fclose(fp);
>>  		return;
>> +	}
>>  
>>  	do {
>>  		r = fscanf(fp, "%x %x", &reginit.address, &reginit.data);
>> diff --git a/tools/zynqmpimage.c b/tools/zynqmpimage.c
>> index 60d8ed23b4a1..0c9a3daddd6a 100644
>> --- a/tools/zynqmpimage.c
>> +++ b/tools/zynqmpimage.c
>> @@ -251,11 +251,15 @@ static void zynqmpimage_parse_initparams(struct zynqmp_header *zynqhdr,
>>  	}
>>  
>>  	err = fstat(fileno(fp), &path_stat);
>> -	if (err)
>> +	if (err) {
>> +		fclose(fp);
>>  		return;
>> +	}
>>  
>> -	if (!S_ISREG(path_stat.st_mode))
>> +	if (!S_ISREG(path_stat.st_mode)) {
>> +		fclose(fp);
>>  		return;
>> +	}
>>  
>>  	do {
>>  		r = fscanf(fp, "%x %x", &reginit.address, &reginit.data);
> 
> what about something like this?

Sure that will work too.

Thanks,
Michal

> 
> Best regards,
> 	ladis (bored waiting for the lunch ;-))
> 
> Signed-off-by: Ladislav Michl <ladis@linux-mips.org>
> ---
> diff --git a/tools/zynqimage.c b/tools/zynqimage.c
> index b47132b02a..026e99c00b 100644
> --- a/tools/zynqimage.c
> +++ b/tools/zynqimage.c
> @@ -228,7 +228,7 @@ static void zynqimage_parse_initparams(struct zynq_header *zynqhdr,
>  	FILE *fp;
>  	struct zynq_reginit reginit;
>  	unsigned int reg_count = 0;
> -	int r, err;
> +	int r;
>  	struct stat path_stat;
>  
>  	/* Expect a table of register-value pairs, e.g. "0x12345678 0x4321" */
> @@ -238,12 +238,10 @@ static void zynqimage_parse_initparams(struct zynq_header *zynqhdr,
>  		exit(1);
>  	}
>  
> -	err = fstat(fileno(fp), &path_stat);
> -	if (err)
> -		return;
> -
> -	if (!S_ISREG(path_stat.st_mode))
> +	if (fstat(fileno(fp), &path_stat) || !S_ISREG(path_stat.st_mode)) {
> +		fclose(fp);
>  		return;
> +	}
>  
>  	do {
>  		r = fscanf(fp, "%x %x", &reginit.address, &reginit.data);
> diff --git a/tools/zynqmpimage.c b/tools/zynqmpimage.c
> index 60d8ed23b4..767e93a2ab 100644
> --- a/tools/zynqmpimage.c
> +++ b/tools/zynqmpimage.c
> @@ -240,7 +240,7 @@ static void zynqmpimage_parse_initparams(struct zynqmp_header *zynqhdr,
>  	FILE *fp;
>  	struct zynqmp_reginit reginit;
>  	unsigned int reg_count = 0;
> -	int r, err;
> +	int r;
>  	struct stat path_stat;
>  
>  	/* Expect a table of register-value pairs, e.g. "0x12345678 0x4321" */
> @@ -250,12 +250,10 @@ static void zynqmpimage_parse_initparams(struct zynqmp_header *zynqhdr,
>  		exit(1);
>  	}
>  
> -	err = fstat(fileno(fp), &path_stat);
> -	if (err)
> -		return;
> -
> -	if (!S_ISREG(path_stat.st_mode))
> +	if (fstat(fileno(fp), &path_stat) || !S_ISREG(path_stat.st_mode)) {
> +		fclose(fp);
>  		return;
> +	}
>  
>  	do {
>  		r = fscanf(fp, "%x %x", &reginit.address, &reginit.data);
>
diff mbox

Patch

diff --git a/tools/zynqimage.c b/tools/zynqimage.c
index b47132b02a..026e99c00b 100644
--- a/tools/zynqimage.c
+++ b/tools/zynqimage.c
@@ -228,7 +228,7 @@  static void zynqimage_parse_initparams(struct zynq_header *zynqhdr,
 	FILE *fp;
 	struct zynq_reginit reginit;
 	unsigned int reg_count = 0;
-	int r, err;
+	int r;
 	struct stat path_stat;
 
 	/* Expect a table of register-value pairs, e.g. "0x12345678 0x4321" */
@@ -238,12 +238,10 @@  static void zynqimage_parse_initparams(struct zynq_header *zynqhdr,
 		exit(1);
 	}
 
-	err = fstat(fileno(fp), &path_stat);
-	if (err)
-		return;
-
-	if (!S_ISREG(path_stat.st_mode))
+	if (fstat(fileno(fp), &path_stat) || !S_ISREG(path_stat.st_mode)) {
+		fclose(fp);
 		return;
+	}
 
 	do {
 		r = fscanf(fp, "%x %x", &reginit.address, &reginit.data);
diff --git a/tools/zynqmpimage.c b/tools/zynqmpimage.c
index 60d8ed23b4..767e93a2ab 100644
--- a/tools/zynqmpimage.c
+++ b/tools/zynqmpimage.c
@@ -240,7 +240,7 @@  static void zynqmpimage_parse_initparams(struct zynqmp_header *zynqhdr,
 	FILE *fp;
 	struct zynqmp_reginit reginit;
 	unsigned int reg_count = 0;
-	int r, err;
+	int r;
 	struct stat path_stat;
 
 	/* Expect a table of register-value pairs, e.g. "0x12345678 0x4321" */
@@ -250,12 +250,10 @@  static void zynqmpimage_parse_initparams(struct zynqmp_header *zynqhdr,
 		exit(1);
 	}
 
-	err = fstat(fileno(fp), &path_stat);
-	if (err)
-		return;
-
-	if (!S_ISREG(path_stat.st_mode))
+	if (fstat(fileno(fp), &path_stat) || !S_ISREG(path_stat.st_mode)) {
+		fclose(fp);
 		return;
+	}
 
 	do {
 		r = fscanf(fp, "%x %x", &reginit.address, &reginit.data);