* [PATCH mtd-utils] jffsX-utils: fix integer overflow in jffs2dump.c
@ 2024-12-18 11:42 Anton Moryakov
2024-12-19 4:54 ` Zhihao Cheng
0 siblings, 1 reply; 8+ messages in thread
From: Anton Moryakov @ 2024-12-18 11:42 UTC (permalink / raw)
To: chengzhihao1, linux-mtd; +Cc: Anton Moryakov
Report of the static analyzer:
The value of an arithmetic expression 'datsize + oobsize' is a subject to overflow because its operands are not cast to a larger data type before performing arithmetic
Corrections explained:
Fixed potential overflow in arithmetic operation datsize + oobsize
by casting operands to unsigned long long to ensure safe computation.
Triggers found by static analyzer Svace.
Signed-off-by: Anton Moryakov <ant.v.moryakov@gmail.com>
---
jffsX-utils/jffs2dump.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/jffsX-utils/jffs2dump.c b/jffsX-utils/jffs2dump.c
index 30455ea..5be7a5b 100644
--- a/jffsX-utils/jffs2dump.c
+++ b/jffsX-utils/jffs2dump.c
@@ -783,7 +783,7 @@ int main(int argc, char **argv)
read_nocheck (fd, oob, oobsize);
idx += datsize;
imglen -= oobsize;
- len -= datsize + oobsize;
+ len -= (unsigned long long)datsize + (unsigned long long)oobsize;
}
} else {
--
2.30.2
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH mtd-utils] jffsX-utils: fix integer overflow in jffs2dump.c
2024-12-18 11:42 Anton Moryakov
@ 2024-12-19 4:54 ` Zhihao Cheng
0 siblings, 0 replies; 8+ messages in thread
From: Zhihao Cheng @ 2024-12-19 4:54 UTC (permalink / raw)
To: Anton Moryakov, linux-mtd
在 2024/12/18 19:42, Anton Moryakov 写道:
> Report of the static analyzer:
> The value of an arithmetic expression 'datsize + oobsize' is a subject to overflow because its operands are not cast to a larger data type before performing arithmetic
>
> Corrections explained:
> Fixed potential overflow in arithmetic operation datsize + oobsize
> by casting operands to unsigned long long to ensure safe computation.
>
> Triggers found by static analyzer Svace.
>
> Signed-off-by: Anton Moryakov <ant.v.moryakov@gmail.com>
>
> ---
> jffsX-utils/jffs2dump.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/jffsX-utils/jffs2dump.c b/jffsX-utils/jffs2dump.c
> index 30455ea..5be7a5b 100644
> --- a/jffsX-utils/jffs2dump.c
> +++ b/jffsX-utils/jffs2dump.c
> @@ -783,7 +783,7 @@ int main(int argc, char **argv)
> read_nocheck (fd, oob, oobsize);
> idx += datsize;
> imglen -= oobsize;
> - len -= datsize + oobsize;
> + len -= (unsigned long long)datsize + (unsigned long long)oobsize;
The 'datsize' and 'oobsize' are parsed from user arguments, 'datsize'
and 'oobsize' could be negative, so I think we need more checking items.
> }
>
> } else {
>
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH mtd-utils] jffsX-utils: fix integer overflow in jffs2dump.c
@ 2024-12-19 12:49 Anton Moryakov
2024-12-19 13:29 ` Zhihao Cheng
0 siblings, 1 reply; 8+ messages in thread
From: Anton Moryakov @ 2024-12-19 12:49 UTC (permalink / raw)
To: chengzhihao1, linux-mtd; +Cc: Anton Moryakov
Report of the static analyzer:
The value of an arithmetic expression 'datsize + oobsize' is a subject to overflow because its operands are not cast to a larger data type before performing arithmetic
Corrections explained:
Fixed potential overflow in arithmetic operation datsize + oobsize
by casting operands to unsigned long long to ensure safe computation.
And also added a check i think you can just add
if (datsize < 0 || oobsize < 0) excluding negative values for datsize and oobsize
---
jffsX-utils/jffs2dump.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/jffsX-utils/jffs2dump.c b/jffsX-utils/jffs2dump.c
index 30455ea..bee8c59 100644
--- a/jffsX-utils/jffs2dump.c
+++ b/jffsX-utils/jffs2dump.c
@@ -772,6 +772,11 @@ int main(int argc, char **argv)
exit(EXIT_FAILURE);
}
+ if (datsize < 0 || oobsize < 0) {
+ fprintf(stderr, "Error: datsize and oobsize must be non-negative.\n");
+ return -1;
+ }
+
if (datsize && oobsize) {
int idx = 0;
long len = imglen;
@@ -783,7 +788,7 @@ int main(int argc, char **argv)
read_nocheck (fd, oob, oobsize);
idx += datsize;
imglen -= oobsize;
- len -= datsize + oobsize;
+ len -= (unsigned long long)datsize + (unsigned long long)oobsize;
}
} else {
--
2.30.2
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH mtd-utils] jffsX-utils: fix integer overflow in jffs2dump.c
2024-12-19 12:49 Anton Moryakov
@ 2024-12-19 13:29 ` Zhihao Cheng
0 siblings, 0 replies; 8+ messages in thread
From: Zhihao Cheng @ 2024-12-19 13:29 UTC (permalink / raw)
To: Anton Moryakov, linux-mtd
在 2024/12/19 20:49, Anton Moryakov 写道:
> Report of the static analyzer:
> The value of an arithmetic expression 'datsize + oobsize' is a subject to overflow because its operands are not cast to a larger data type before performing arithmetic
>
> Corrections explained:
> Fixed potential overflow in arithmetic operation datsize + oobsize
> by casting operands to unsigned long long to ensure safe computation.
> And also added a check i think you can just add
> if (datsize < 0 || oobsize < 0) excluding negative values for datsize and oobsize
>
> ---
> jffsX-utils/jffs2dump.c | 7 ++++++-
> 1 file changed, 6 insertions(+), 1 deletion(-)
>
How about this:
diff --git a/jffsX-utils/jffs2dump.c b/jffsX-utils/jffs2dump.c
index 30455ea..4bc87c5 100644
--- a/jffsX-utils/jffs2dump.c
+++ b/jffsX-utils/jffs2dump.c
@@ -772,6 +772,14 @@ int main(int argc, char **argv)
exit(EXIT_FAILURE);
}
+ if (datsize < 0 || oobsize < 0 || datsize > imglen ||
+ (long)datsize + oobsize < 0) {
+ fprintf(stderr, "Error: invalid datsize/oobsize.\n");
+ free(data);
+ close (fd);
+ exit(EXIT_FAILURE);
+ }
+
if (datsize && oobsize) {
int idx = 0;
long len = imglen;
@@ -783,7 +791,7 @@ int main(int argc, char **argv)
read_nocheck (fd, oob, oobsize);
idx += datsize;
imglen -= oobsize;
- len -= datsize + oobsize;
+ len -= (long)datsize + oobsize;
}
} else {
> diff --git a/jffsX-utils/jffs2dump.c b/jffsX-utils/jffs2dump.c
> index 30455ea..bee8c59 100644
> --- a/jffsX-utils/jffs2dump.c
> +++ b/jffsX-utils/jffs2dump.c
> @@ -772,6 +772,11 @@ int main(int argc, char **argv)
> exit(EXIT_FAILURE);
> }
>
> + if (datsize < 0 || oobsize < 0) {
> + fprintf(stderr, "Error: datsize and oobsize must be non-negative.\n");
> + return -1;
> + }
> +
> if (datsize && oobsize) {
> int idx = 0;
> long len = imglen;
> @@ -783,7 +788,7 @@ int main(int argc, char **argv)
> read_nocheck (fd, oob, oobsize);
> idx += datsize;
> imglen -= oobsize;
> - len -= datsize + oobsize;
> + len -= (unsigned long long)datsize + (unsigned long long)oobsize;
> }
>
> } else {
>
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH mtd-utils] jffsX-utils: fix integer overflow in jffs2dump.c
@ 2024-12-19 13:46 Anton Moryakov
2024-12-20 1:14 ` Zhihao Cheng
0 siblings, 1 reply; 8+ messages in thread
From: Anton Moryakov @ 2024-12-19 13:46 UTC (permalink / raw)
To: chengzhihao1, linux-mtd; +Cc: Anton Moryakov
Report of the static analyzer:
The value of an arithmetic expression 'datsize + oobsize' is a subject to overflow because its operands are not cast to a larger data type before performing arithmetic.
Corrections explained:
- Added a check to validate datsize and oobsize to ensure they are non-negative and within a safe range.
- Cast datsize and oobsize to long before performing arithmetic to prevent potential integer overflow.
This change ensures safe computation of offsets and prevents undefined behavior caused by overflow.
Signed-off-by: Anton Moryakov <ant.v.moryakov@gmail.com>
---
jffsX-utils/jffs2dump.c | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/jffsX-utils/jffs2dump.c b/jffsX-utils/jffs2dump.c
index 30455ea..5b0e144 100644
--- a/jffsX-utils/jffs2dump.c
+++ b/jffsX-utils/jffs2dump.c
@@ -772,6 +772,13 @@ int main(int argc, char **argv)
exit(EXIT_FAILURE);
}
+ if (datsize < 0 oobsize < 0 datsize > imglen || (long)datsize + oobsize < 0) {
+ fprintf(stderr, "Error: invalid datsize/oobsize.\n");
+ free(data);
+ close (fd);
+ exit(EXIT_FAILURE);
+ }
+
if (datsize && oobsize) {
int idx = 0;
long len = imglen;
@@ -783,7 +790,7 @@ int main(int argc, char **argv)
read_nocheck (fd, oob, oobsize);
idx += datsize;
imglen -= oobsize;
- len -= datsize + oobsize;
+ len -= (long)datsize + oobsize;
}
} else {
--
2.30.2
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH mtd-utils] jffsX-utils: fix integer overflow in jffs2dump.c
2024-12-19 13:46 Anton Moryakov
@ 2024-12-20 1:14 ` Zhihao Cheng
0 siblings, 0 replies; 8+ messages in thread
From: Zhihao Cheng @ 2024-12-20 1:14 UTC (permalink / raw)
To: Anton Moryakov, linux-mtd
在 2024/12/19 21:46, Anton Moryakov 写道:
> Report of the static analyzer:
> The value of an arithmetic expression 'datsize + oobsize' is a subject to overflow because its operands are not cast to a larger data type before performing arithmetic.
>
> Corrections explained:
> - Added a check to validate datsize and oobsize to ensure they are non-negative and within a safe range.
> - Cast datsize and oobsize to long before performing arithmetic to prevent potential integer overflow.
>
> This change ensures safe computation of offsets and prevents undefined behavior caused by overflow.
>
> Signed-off-by: Anton Moryakov <ant.v.moryakov@gmail.com>
> ---
> jffsX-utils/jffs2dump.c | 9 ++++++++-
> 1 file changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/jffsX-utils/jffs2dump.c b/jffsX-utils/jffs2dump.c
> index 30455ea..5b0e144 100644
> --- a/jffsX-utils/jffs2dump.c
> +++ b/jffsX-utils/jffs2dump.c
> @@ -772,6 +772,13 @@ int main(int argc, char **argv)
> exit(EXIT_FAILURE);
> }
>
> + if (datsize < 0 oobsize < 0 datsize > imglen || (long)datsize + oobsize < 0) {
It makes compiling failed, please do the basic tests before sending
patches out.
> + fprintf(stderr, "Error: invalid datsize/oobsize.\n");
> + free(data);
> + close (fd);
> + exit(EXIT_FAILURE);
> + }
> +
> if (datsize && oobsize) {
> int idx = 0;
> long len = imglen;
> @@ -783,7 +790,7 @@ int main(int argc, char **argv)
> read_nocheck (fd, oob, oobsize);
> idx += datsize;
> imglen -= oobsize;
> - len -= datsize + oobsize;
> + len -= (long)datsize + oobsize;
> }
>
> } else {
>
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH mtd-utils] jffsX-utils: fix integer overflow in jffs2dump.c
@ 2024-12-24 12:27 Anton Moryakov
2024-12-24 13:23 ` Zhihao Cheng
0 siblings, 1 reply; 8+ messages in thread
From: Anton Moryakov @ 2024-12-24 12:27 UTC (permalink / raw)
To: chengzhihao1, linux-mtd; +Cc: Anton Moryakov
Report of the static analyzer:
The value of an arithmetic expression 'datsize + oobsize' is a subject to overflow because its operands are not cast to a larger data type before performing arithmetic.
Corrections explained:
- Added a check to validate datsize and oobsize to ensure they are non-negative and within a safe range.
- Cast datsize and oobsize to long before performing arithmetic to prevent potential integer overflow.
This change ensures safe computation of offsets and prevents undefined behavior caused by overflow.
Signed-off-by: Anton Moryakov <ant.v.moryakov@gmail.com>
---
jffsX-utils/jffs2dump.c | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/jffsX-utils/jffs2dump.c b/jffsX-utils/jffs2dump.c
index 30455ea..5b0e144 100644
--- a/jffsX-utils/jffs2dump.c
+++ b/jffsX-utils/jffs2dump.c
@@ -772,6 +772,13 @@ int main(int argc, char **argv)
exit(EXIT_FAILURE);
}
+ if (datsize < 0 || oobsize < 0 || datsize > imglen || (long)datsize + oobsize < 0) {
+ fprintf(stderr, "Error: invalid datsize/oobsize.\n");
+ free(data);
+ close (fd);
+ exit(EXIT_FAILURE);
+ }
+
if (datsize && oobsize) {
int idx = 0;
long len = imglen;
@@ -783,7 +790,7 @@ int main(int argc, char **argv)
read_nocheck (fd, oob, oobsize);
idx += datsize;
imglen -= oobsize;
- len -= datsize + oobsize;
+ len -= (long)datsize + oobsize;
}
} else {
--
2.30.2
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH mtd-utils] jffsX-utils: fix integer overflow in jffs2dump.c
2024-12-24 12:27 [PATCH mtd-utils] jffsX-utils: fix integer overflow in jffs2dump.c Anton Moryakov
@ 2024-12-24 13:23 ` Zhihao Cheng
0 siblings, 0 replies; 8+ messages in thread
From: Zhihao Cheng @ 2024-12-24 13:23 UTC (permalink / raw)
To: Anton Moryakov, linux-mtd
在 2024/12/24 20:27, Anton Moryakov 写道:
> Report of the static analyzer:
> The value of an arithmetic expression 'datsize + oobsize' is a subject to overflow because its operands are not cast to a larger data type before performing arithmetic.
>
> Corrections explained:
> - Added a check to validate datsize and oobsize to ensure they are non-negative and within a safe range.
> - Cast datsize and oobsize to long before performing arithmetic to prevent potential integer overflow.
>
> This change ensures safe computation of offsets and prevents undefined behavior caused by overflow.
>
> Signed-off-by: Anton Moryakov <ant.v.moryakov@gmail.com>
> ---
> jffsX-utils/jffs2dump.c | 9 ++++++++-
> 1 file changed, 8 insertions(+), 1 deletion(-)
>
Reviewed-by: Zhihao Cheng <chengzhihao1@huawei.com>
> diff --git a/jffsX-utils/jffs2dump.c b/jffsX-utils/jffs2dump.c
> index 30455ea..5b0e144 100644
> --- a/jffsX-utils/jffs2dump.c
> +++ b/jffsX-utils/jffs2dump.c
> @@ -772,6 +772,13 @@ int main(int argc, char **argv)
> exit(EXIT_FAILURE);
> }
>
> + if (datsize < 0 || oobsize < 0 || datsize > imglen || (long)datsize + oobsize < 0) {
> + fprintf(stderr, "Error: invalid datsize/oobsize.\n");
> + free(data);
> + close (fd);
> + exit(EXIT_FAILURE);
> + }
> +
> if (datsize && oobsize) {
> int idx = 0;
> long len = imglen;
> @@ -783,7 +790,7 @@ int main(int argc, char **argv)
> read_nocheck (fd, oob, oobsize);
> idx += datsize;
> imglen -= oobsize;
> - len -= datsize + oobsize;
> + len -= (long)datsize + oobsize;
> }
>
> } else {
>
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2024-12-24 13:24 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-12-24 12:27 [PATCH mtd-utils] jffsX-utils: fix integer overflow in jffs2dump.c Anton Moryakov
2024-12-24 13:23 ` Zhihao Cheng
-- strict thread matches above, loose matches on Subject: below --
2024-12-19 13:46 Anton Moryakov
2024-12-20 1:14 ` Zhihao Cheng
2024-12-19 12:49 Anton Moryakov
2024-12-19 13:29 ` Zhihao Cheng
2024-12-18 11:42 Anton Moryakov
2024-12-19 4:54 ` Zhihao Cheng
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).