| Message ID | 20170504202642.5031-1-xypron.glpk@gmx.de |
|---|---|
| State | Accepted |
| Commit | f59a3b21f60190793dc3cee97338c2f6ee9f2336 |
| Delegated to: | Tom Rini |
| Headers | show |
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; > case 'p':
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!
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':
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(-)