[U-Boot] [PATCH 1/1] tools: sunxi: avoid possible null pointer dereference
If endptr is NULL we should not dereference it.
The problem was indicated by cppcheck.
Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de --- tools/sunxi-spl-image-builder.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/sunxi-spl-image-builder.c b/tools/sunxi-spl-image-builder.c index d538a38813..0072a64728 100644 --- a/tools/sunxi-spl-image-builder.c +++ b/tools/sunxi-spl-image-builder.c @@ -433,7 +433,7 @@ int main(int argc, char **argv) break; case 'c': info.ecc_strength = strtol(optarg, &endptr, 0); - if (endptr || *endptr == '/') + if (endptr && *endptr == '/') info.ecc_step_size = strtol(endptr + 1, NULL, 0); break; case 'p':
On Thu, May 4, 2017 at 2:41 AM, Heinrich Schuchardt xypron.glpk@gmx.de wrote:
If endptr is NULL we should not dereference it.
The problem was indicated by cppcheck.
Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de
tools/sunxi-spl-image-builder.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/sunxi-spl-image-builder.c b/tools/sunxi-spl-image-builder.c index d538a38813..0072a64728 100644 --- a/tools/sunxi-spl-image-builder.c +++ b/tools/sunxi-spl-image-builder.c @@ -433,7 +433,7 @@ int main(int argc, char **argv) break; case 'c': info.ecc_strength = strtol(optarg, &endptr, 0);
if (endptr || *endptr == '/')
if (endptr && *endptr == '/')
Did the config function as 'if' with single argument, can you check below sample - couldn't reproduce null dereference.
# cppcheck --library=test.cfg tools/sunxi-spl-image-builder.c Checking tools/sunxi-spl-image-builder.c... [tools/sunxi-spl-image-builder.c:286]: (error) Resource leak: src [tools/sunxi-spl-image-builder.c:286]: (error) Resource leak: dst [tools/sunxi-spl-image-builder.c:286]: (error) Resource leak: rnd [tools/sunxi-spl-image-builder.c:263]: (error) Memory leak: buffer # cat test.cfg <?xml version="1.0"?> <def> <function name="if"> <arg nr="1"> <not-null/> </arg> </function> </def>
thanks!
The evaluation of option -c is incorrect:
According to the C99 standard endptr in the first strtol is always set as &endptr is not NULL. So the first part of the or condition is always true. If all digits in optarg are valid endptr will point to the closing \0 and the second strtol will read beyond the end of the string optarg points to.
Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de --- v2: Simplify the logical expression. v1: In the original patch I missed that envptr is always set in strtol and used an unnecessary check if endptr is non-NULL. [PATCH 1/1] tools: sunxi: avoid possible null pointer dereference https://patchwork.ozlabs.org/patch/758224/ --- tools/sunxi-spl-image-builder.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/sunxi-spl-image-builder.c b/tools/sunxi-spl-image-builder.c index d538a38813..a367f11774 100644 --- a/tools/sunxi-spl-image-builder.c +++ b/tools/sunxi-spl-image-builder.c @@ -433,7 +433,7 @@ int main(int argc, char **argv) break; case 'c': info.ecc_strength = strtol(optarg, &endptr, 0); - if (endptr || *endptr == '/') + if (*endptr == '/') info.ecc_step_size = strtol(endptr + 1, NULL, 0); break; case 'p':
On Thu, 4 May 2017 22:26:42 +0200 Heinrich Schuchardt xypron.glpk@gmx.de wrote:
The evaluation of option -c is incorrect:
According to the C99 standard endptr in the first strtol is always set as &endptr is not NULL. So the first part of the or condition is always true. If all digits in optarg are valid endptr will point to the closing \0 and the second strtol will read beyond the end of the string optarg points to.
Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de
Acked-by: Boris Brezillon boris.brezillon@free-electrons.com
v2: Simplify the logical expression. v1: In the original patch I missed that envptr is always set in strtol and used an unnecessary check if endptr is non-NULL. [PATCH 1/1] tools: sunxi: avoid possible null pointer dereference https://patchwork.ozlabs.org/patch/758224/
tools/sunxi-spl-image-builder.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/sunxi-spl-image-builder.c b/tools/sunxi-spl-image-builder.c index d538a38813..a367f11774 100644 --- a/tools/sunxi-spl-image-builder.c +++ b/tools/sunxi-spl-image-builder.c @@ -433,7 +433,7 @@ int main(int argc, char **argv) break; case 'c': info.ecc_strength = strtol(optarg, &endptr, 0);
if (endptr || *endptr == '/')
if (*endptr == '/') info.ecc_step_size = strtol(endptr + 1, NULL, 0); break;
On Thu, May 04, 2017 at 10:26:42PM +0200, xypron.glpk@gmx.de wrote:
The evaluation of option -c is incorrect:
According to the C99 standard endptr in the first strtol is always set as &endptr is not NULL. So the first part of the or condition is always true. If all digits in optarg are valid endptr will point to the closing \0 and the second strtol will read beyond the end of the string optarg points to.
Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de Acked-by: Boris Brezillon boris.brezillon@free-electrons.com
Applied to u-boot/master, thanks!
participants (4)
-
Boris Brezillon -
Heinrich Schuchardt -
Jagan Teki -
Tom Rini