Message ID | 20200312094008.1833929-1-gabravier@gmail.com |
---|---|
State | New |
Headers | show |
Series | gpio-hammer: Avoid potential overflow in main | expand |
czw., 12 mar 2020 o 10:40 Gabriel Ravier <gabravier@gmail.com> napisał(a): > > If '-o' was used more than 64 times in a single invocation of gpio-hammer, > this could lead to an overflow of the 'lines' array. This commit fixes > this by avoiding the overflow and giving a proper diagnostic back to the > user > > Signed-off-by: Gabriel Ravier <gabravier@gmail.com> > --- > tools/gpio/gpio-hammer.c | 19 +++++++++++++++++-- > 1 file changed, 17 insertions(+), 2 deletions(-) > > diff --git a/tools/gpio/gpio-hammer.c b/tools/gpio/gpio-hammer.c > index 0e0060a6e..273d33847 100644 > --- a/tools/gpio/gpio-hammer.c > +++ b/tools/gpio/gpio-hammer.c > @@ -77,7 +77,7 @@ int hammer_device(const char *device_name, unsigned int *lines, int nlines, > > fprintf(stdout, "[%c] ", swirr[j]); > j++; > - if (j == sizeof(swirr)-1) > + if (j == sizeof(swirr) - 1) Please don't try to sneak in unrelated changes into commits. This is of course correct coding-style-wise but send it in a separate patch. > j = 0; > > fprintf(stdout, "["); > @@ -135,7 +135,14 @@ int main(int argc, char **argv) > device_name = optarg; > break; > case 'o': > - lines[i] = strtoul(optarg, NULL, 10); > + /* > + * Avoid overflow. Do not immediately error, we want to > + * be able to accurately report on the amount of times > + *'-o' was given to give an accurate error message > + */ > + if (i < GPIOHANDLES_MAX) > + lines[i] = strtoul(optarg, NULL, 10); > + > i++; > break; > case '?': > @@ -143,6 +150,14 @@ int main(int argc, char **argv) > return -1; > } > } > + > + if (i >= GPIOHANDLES_MAX) { > + fprintf(stderr, > + "Only %d occurences of '-o' are allowed, %d were found\n", > + GPIOHANDLES_MAX, i + 1); > + return -1; > + } > + > nlines = i; > > if (!device_name || !nlines) { > -- > 2.24.1 > Other than that, looks good. Bartosz
Ah, that was accidental. I was applying scripts/Lindent to my code and ended up also having it applied to part of the old code. Didn't think it would hurt, but I guess it makes sense to be this stringent on separating logical changes. Will send a (complete) corrected patch in-reply-to this message.
czw., 12 mar 2020 o 15:21 Gabriel Ravier <gabravier@gmail.com> napisał(a): > > Ah, that was accidental. I was applying scripts/Lindent to my code and > ended up also having it applied to part of the old code. Didn't think it > would hurt, but I guess it makes sense to be this stringent on > separating logical changes. Will send a (complete) corrected patch > in-reply-to this message. > Please don't send patches in response to other threads. Always start a new thread and increment the version in [PATCH vX] tag. Bart
Ah, seems like I didn't read the guide to getting code into the kernel thoroughly enough. Should I send the patch yet again just with a v2 in the subject header or is there no need to bother with that ? On 3/12/20 3:29 PM, Bartosz Golaszewski wrote: > czw., 12 mar 2020 o 15:21 Gabriel Ravier <gabravier@gmail.com> napisał(a): >> Ah, that was accidental. I was applying scripts/Lindent to my code and >> ended up also having it applied to part of the old code. Didn't think it >> would hurt, but I guess it makes sense to be this stringent on >> separating logical changes. Will send a (complete) corrected patch >> in-reply-to this message. >> > Please don't send patches in response to other threads. Always start a > new thread and increment the version in [PATCH vX] tag. > > Bart
czw., 12 mar 2020 o 15:33 Gabriel Ravier <gabravier@gmail.com> napisał(a): > > Ah, seems like I didn't read the guide to getting code into the kernel > thoroughly enough. Should I send the patch yet again just with a v2 in > the subject header or is there no need to bother with that ? > Please do so that patchwork picks it up. Also: please don't top-post on LKML ie. respond below others' text. Bart
On 3/12/20 3:36 PM, Bartosz Golaszewski wrote: > czw., 12 mar 2020 o 15:33 Gabriel Ravier <gabravier@gmail.com> napisał(a): >> Ah, seems like I didn't read the guide to getting code into the kernel >> thoroughly enough. Should I send the patch yet again just with a v2 in >> the subject header or is there no need to bother with that ? >> > Please do so that patchwork picks it up. Also: please don't top-post > on LKML ie. respond below others' text. > > Bart Ok, will send it with s/PATCH/PATCH v2 ASAP. Btw, is the "top-posting" the reason why lkml.org doesn't seem to be able to find my replies to your posts ?
diff --git a/tools/gpio/gpio-hammer.c b/tools/gpio/gpio-hammer.c index 0e0060a6e..273d33847 100644 --- a/tools/gpio/gpio-hammer.c +++ b/tools/gpio/gpio-hammer.c @@ -77,7 +77,7 @@ int hammer_device(const char *device_name, unsigned int *lines, int nlines, fprintf(stdout, "[%c] ", swirr[j]); j++; - if (j == sizeof(swirr)-1) + if (j == sizeof(swirr) - 1) j = 0; fprintf(stdout, "["); @@ -135,7 +135,14 @@ int main(int argc, char **argv) device_name = optarg; break; case 'o': - lines[i] = strtoul(optarg, NULL, 10); + /* + * Avoid overflow. Do not immediately error, we want to + * be able to accurately report on the amount of times + *'-o' was given to give an accurate error message + */ + if (i < GPIOHANDLES_MAX) + lines[i] = strtoul(optarg, NULL, 10); + i++; break; case '?': @@ -143,6 +150,14 @@ int main(int argc, char **argv) return -1; } } + + if (i >= GPIOHANDLES_MAX) { + fprintf(stderr, + "Only %d occurences of '-o' are allowed, %d were found\n", + GPIOHANDLES_MAX, i + 1); + return -1; + } + nlines = i; if (!device_name || !nlines) {
If '-o' was used more than 64 times in a single invocation of gpio-hammer, this could lead to an overflow of the 'lines' array. This commit fixes this by avoiding the overflow and giving a proper diagnostic back to the user Signed-off-by: Gabriel Ravier <gabravier@gmail.com> --- tools/gpio/gpio-hammer.c | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-)