Skip to contentSkip to conversation

Fix array index out of bound exception

PATCH In review Lianbo Jiang 2 participants 4 messages Mar 6, 2018 – Mar 23, 2018

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

PATCH 1/1 Fix array index out of bound exception

A data overflow may lead to a reversal, which may turn a positive number into a large negative number, in this case, the string's length will exceed the array size(for example, eta: -2147483648s), here the array size is defined 16 characters. So, it is nessasary to consider some exceptions.

---
 print_info.c | 21 +++++++++++++--------
 1 file changed, 13 insertions(+), 8 deletions(-)
print_info.c +13 -8
index e0e6a27..09e215a 100644
@@ -16,6 +16,8 @@
 #include "print_info.h"
 #include <time.h>
 #include <string.h>
+#include <stdint.h>
+#include <inttypes.h>
 
 #define PROGRESS_MAXLEN		"50"
 
@@ -352,18 +354,21 @@ static void calc_delta(struct timeval *tv_start, struct timeval *delta)
 }
 
 /* produce less than 12 bytes on msg */
-static int eta_to_human_short (int secs, char* msg)
+static int eta_to_human_short (int64_t secs, char* msg, int maxsize)
 {
 	strcpy(msg, "eta: ");
 	msg += strlen("eta: ");
 	if (secs < 100)
-		sprintf(msg, "%ds", secs);
+		snprintf(msg, maxsize, "%"PRId64"s", secs);
 	else if (secs < 100 * 60)
-		sprintf(msg, "%dm%ds", secs / 60, secs % 60);
+		snprintf(msg, maxsize, "%"PRId64"m""%"PRId64"s",
+			secs / 60, secs % 60);
 	else if (secs < 48 * 3600)
-		sprintf(msg, "%dh%dm", secs / 3600, (secs / 60) % 60);
+		snprintf(msg, maxsize, "%"PRId64"h""%"PRId64"m",
+			secs / 3600, (secs / 60) % 60);
 	else if (secs < 100 * 86400)
-		sprintf(msg, "%dd%dh", secs / 86400, (secs / 3600) % 24);
+		snprintf(msg, maxsize, "%"PRId64"d""%"PRId64"h",
+			secs / 86400, (secs / 3600) % 24);
 	else
 		sprintf(msg, ">2day");
 	return 0;
@@ -379,8 +384,8 @@ print_progress(const char *msg, unsigned long current, unsigned long end, struct
 	static unsigned int lapse = 0;
 	static const char *spinner = "/|\\-";
 	struct timeval delta;
-	double eta;
-	char eta_msg[16] = " ";
+	int64_t eta;
+	char eta_msg[32] = " ";
 
 	if (current < end) {
 		tm = time(NULL);
@@ -395,7 +400,7 @@ print_progress(const char *msg, unsigned long current, unsigned long end, struct
 		calc_delta(start, &delta);
 		eta = delta.tv_sec + delta.tv_usec / 1e6;
 		eta = (100 - progress) * eta / progress;
-		eta_to_human_short(eta, eta_msg);
+		eta_to_human_short(eta, eta_msg, sizeof(eta_msg));
 	}
 	if (flag_ignore_r_char) {
 		PROGRESS_MSG("%-" PROGRESS_MAXLEN "s: [%5.1f %%] %c  %16s\n",
Signature · 7 lines
2.9.5


_______________________________________________
kexec mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/kexec
author
#1 lore raw

Also cc Masaki/Masahiko/piliu.

Thanks.

Lianbo

Quoted 64 lines · Lianbo Jiang

A data overflow may lead to a reversal, which may turn a positive number into a large negative number, in this case, the string's length will exceed the array size(for example, eta: -2147483648s), here the array size is defined 16 characters. So, it is nessasary to consider some exceptions.

---
 print_info.c | 21 +++++++++++++--------
 1 file changed, 13 insertions(+), 8 deletions(-)
print_info.c +12 -7
index e0e6a27..09e215a 100644
@@ -16,6 +16,8 @@
 #include "print_info.h"
 #include <time.h>
 #include <string.h>
+#include <stdint.h>
+#include <inttypes.h>
 
 #define PROGRESS_MAXLEN		"50"
 
@@ -352,18 +354,21 @@ static void calc_delta(struct timeval *tv_start, struct timeval *delta)
 }
 
 /* produce less than 12 bytes on msg */
-static int eta_to_human_short (int secs, char* msg)
+static int eta_to_human_short (int64_t secs, char* msg, int maxsize)
 {
 	strcpy(msg, "eta: ");
 	msg += strlen("eta: ");
 	if (secs < 100)
-		sprintf(msg, "%ds", secs);
+		snprintf(msg, maxsize, "%"PRId64"s", secs);
 	else if (secs < 100 * 60)
-		sprintf(msg, "%dm%ds", secs / 60, secs % 60);
+		snprintf(msg, maxsize, "%"PRId64"m""%"PRId64"s",
+			secs / 60, secs % 60);
 	else if (secs < 48 * 3600)
-		sprintf(msg, "%dh%dm", secs / 3600, (secs / 60) % 60);
+		snprintf(msg, maxsize, "%"PRId64"h""%"PRId64"m",
+			secs / 3600, (secs / 60) % 60);
 	else if (secs < 100 * 86400)
-		sprintf(msg, "%dd%dh", secs / 86400, (secs / 3600) % 24);
+		snprintf(msg, maxsize, "%"PRId64"d""%"PRId64"h",
+			secs / 86400, (secs / 3600) % 24);
 	else
 		sprintf(msg, ">2day");
 	return 0;
@@ -379,8 +384,8 @@ print_progress(const char *msg, unsigned long current, unsigned long end, struct
 	static unsigned int lapse = 0;
 	static const char *spinner = "/|\\-";
 	struct timeval delta;
-	double eta;
-	char eta_msg[16] = " ";
+	int64_t eta;
+	char eta_msg[32] = " ";
 
 	if (current < end) {
 		tm = time(NULL);

print_info.c print_progress(const char *msg, unsigned long current, unsigned long end, struct · line 406

Quoted 8 lines
print_info.c +1 -1
@@ -395,7 +400,7 @@ print_progress(const char *msg, unsigned long current, unsigned long end, struct
 		calc_delta(start, &delta);
 		eta = delta.tv_sec + delta.tv_usec / 1e6;
 		eta = (100 - progress) * eta / progress;
-		eta_to_human_short(eta, eta_msg);
+		eta_to_human_short(eta, eta_msg, sizeof(eta_msg));
 	}
 	if (flag_ignore_r_char) {
 		PROGRESS_MSG("%-" PROGRESS_MAXLEN "s: [%5.1f %%] %c  %16s\n",

_______________________________________________
kexec mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/kexec

#2 lore raw

Hi Lianbo,

Thank you for your patch.
I understand the purpose of the patch.
I'll reply formally in 10 days or so.

Thanks
Tachibana

Quoted 74 lines

-----Original Message-----
From: lijiang [mailto:[email protected]]
Sent: Wednesday, March 07, 2018 10:25 AM
To: [email protected]
Cc: Tachibana Masaki() <[email protected]>; Hayashi Masahiko() <[email protected]>; piliu <[email protected]>
Subject: Re: [PATCH makedumpfile] Fix array index out of bound exception

Also cc Masaki/Masahiko/piliu.

Thanks.

Lianbo
20180306 18:07, Lianbo Jiang :

Quoted 60 lines

A data overflow may lead to a reversal, which may turn a positive number into a large negative number, in this case, the string's length will exceed the array size(for example, eta: -2147483648s), here the array size is defined 16 characters. So, it is nessasary to consider some exceptions.

---
 print_info.c | 21 +++++++++++++--------
 1 file changed, 13 insertions(+), 8 deletions(-)
print_info.c +12 -7
index e0e6a27..09e215a 100644
@@ -16,6 +16,8 @@
 #include "print_info.h"
 #include <time.h>
 #include <string.h>
+#include <stdint.h>
+#include <inttypes.h>
 
 #define PROGRESS_MAXLEN		"50"
 
@@ -352,18 +354,21 @@ static void calc_delta(struct timeval *tv_start, struct timeval *delta)
 }
 
 /* produce less than 12 bytes on msg */
-static int eta_to_human_short (int secs, char* msg)
+static int eta_to_human_short (int64_t secs, char* msg, int maxsize)
 {
 	strcpy(msg, "eta: ");
 	msg += strlen("eta: ");
 	if (secs < 100)
-		sprintf(msg, "%ds", secs);
+		snprintf(msg, maxsize, "%"PRId64"s", secs);
 	else if (secs < 100 * 60)
-		sprintf(msg, "%dm%ds", secs / 60, secs % 60);
+		snprintf(msg, maxsize, "%"PRId64"m""%"PRId64"s",
+			secs / 60, secs % 60);
 	else if (secs < 48 * 3600)
-		sprintf(msg, "%dh%dm", secs / 3600, (secs / 60) % 60);
+		snprintf(msg, maxsize, "%"PRId64"h""%"PRId64"m",
+			secs / 3600, (secs / 60) % 60);
 	else if (secs < 100 * 86400)
-		sprintf(msg, "%dd%dh", secs / 86400, (secs / 3600) % 24);
+		snprintf(msg, maxsize, "%"PRId64"d""%"PRId64"h",
+			secs / 86400, (secs / 3600) % 24);
 	else
 		sprintf(msg, ">2day");
 	return 0;
@@ -379,8 +384,8 @@ print_progress(const char *msg, unsigned long current, unsigned long end, struct
 	static unsigned int lapse = 0;
 	static const char *spinner = "/|\\-";
 	struct timeval delta;
-	double eta;
-	char eta_msg[16] = " ";
+	int64_t eta;
+	char eta_msg[32] = " ";
 
Quoted 12 lines

print_info.c print_progress(const char *msg, unsigned long current, unsigned long end, struct · line 406

Quoted 12 lines
print_info.c +1 -1
 	if (current < end) {
 		tm = time(NULL);
@@ -395,7 +400,7 @@ print_progress(const char *msg, unsigned long current, unsigned long end, struct
 		calc_delta(start, &delta);
 		eta = delta.tv_sec + delta.tv_usec / 1e6;
 		eta = (100 - progress) * eta / progress;
-		eta_to_human_short(eta, eta_msg);
+		eta_to_human_short(eta, eta_msg, sizeof(eta_msg));
 	}
 	if (flag_ignore_r_char) {
 		PROGRESS_MSG("%-" PROGRESS_MAXLEN "s: [%5.1f %%] %c  %16s\n",

_______________________________________________
kexec mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/kexec

#3 lore raw

Hi Lianbo,

Sorry for the late reply.
Thank you for the patch.
I will merge it into V1.6.4.

Thanks
Tachibana

Quoted 99 lines

-----Original Message-----
From: kexec [mailto:[email protected]] On Behalf Of Masaki Tachibana
Sent: Wednesday, March 07, 2018 2:07 PM
To: lijiang <[email protected]>
Cc: [email protected]; Hayashi Masahiko() <[email protected]>; piliu <[email protected]>
Subject: RE: [PATCH makedumpfile] Fix array index out of bound exception

Hi Lianbo,

Thank you for your patch.
I understand the purpose of the patch.
I'll reply formally in 10 days or so.

Thanks
Tachibana

Quoted 83 lines

-----Original Message-----
From: lijiang [mailto:[email protected]]
Sent: Wednesday, March 07, 2018 10:25 AM
To: [email protected]
Cc: Tachibana Masaki() <[email protected]>; Hayashi Masahiko() <[email protected]>; piliu <[email protected]>
Subject: Re: [PATCH makedumpfile] Fix array index out of bound exception

Also cc Masaki/Masahiko/piliu.

Thanks.

Lianbo
20180306 18:07, Lianbo Jiang :

Quoted 69 lines

A data overflow may lead to a reversal, which may turn a positive number into a large negative number, in this case, the string's length will exceed the array size(for example, eta: -2147483648s), here the array size is defined 16 characters. So, it is nessasary to consider some exceptions.

---
 print_info.c | 21 +++++++++++++--------
 1 file changed, 13 insertions(+), 8 deletions(-)
print_info.c +13 -8
index e0e6a27..09e215a 100644
@@ -16,6 +16,8 @@
 #include "print_info.h"
 #include <time.h>
 #include <string.h>
+#include <stdint.h>
+#include <inttypes.h>
 
 #define PROGRESS_MAXLEN		"50"
 
@@ -352,18 +354,21 @@ static void calc_delta(struct timeval *tv_start, struct timeval *delta)
 }
 
 /* produce less than 12 bytes on msg */
-static int eta_to_human_short (int secs, char* msg)
+static int eta_to_human_short (int64_t secs, char* msg, int maxsize)
 {
 	strcpy(msg, "eta: ");
 	msg += strlen("eta: ");
 	if (secs < 100)
-		sprintf(msg, "%ds", secs);
+		snprintf(msg, maxsize, "%"PRId64"s", secs);
 	else if (secs < 100 * 60)
-		sprintf(msg, "%dm%ds", secs / 60, secs % 60);
+		snprintf(msg, maxsize, "%"PRId64"m""%"PRId64"s",
+			secs / 60, secs % 60);
 	else if (secs < 48 * 3600)
-		sprintf(msg, "%dh%dm", secs / 3600, (secs / 60) % 60);
+		snprintf(msg, maxsize, "%"PRId64"h""%"PRId64"m",
+			secs / 3600, (secs / 60) % 60);
 	else if (secs < 100 * 86400)
-		sprintf(msg, "%dd%dh", secs / 86400, (secs / 3600) % 24);
+		snprintf(msg, maxsize, "%"PRId64"d""%"PRId64"h",
+			secs / 86400, (secs / 3600) % 24);
 	else
 		sprintf(msg, ">2day");
 	return 0;
@@ -379,8 +384,8 @@ print_progress(const char *msg, unsigned long current, unsigned long end, struct
 	static unsigned int lapse = 0;
 	static const char *spinner = "/|\\-";
 	struct timeval delta;
-	double eta;
-	char eta_msg[16] = " ";
+	int64_t eta;
+	char eta_msg[32] = " ";
 
 	if (current < end) {
 		tm = time(NULL);
@@ -395,7 +400,7 @@ print_progress(const char *msg, unsigned long current, unsigned long end, struct
 		calc_delta(start, &delta);
 		eta = delta.tv_sec + delta.tv_usec / 1e6;
 		eta = (100 - progress) * eta / progress;
-		eta_to_human_short(eta, eta_msg);
+		eta_to_human_short(eta, eta_msg, sizeof(eta_msg));
 	}
Quoted 8 lines
Quoted 3 lines

print_info.c print_progress(const char *msg, unsigned long current, unsigned long end, struct · line 406

Quoted 3 lines
print_info.c +0 -0
 	if (flag_ignore_r_char) {
 		PROGRESS_MSG("%-" PROGRESS_MAXLEN "s: [%5.1f %%] %c  %16s\n",

_______________________________________________
kexec mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/kexec

_______________________________________________
kexec mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/kexec