On Wednesday 20 August 2008 06:34, Cristian Cadar wrote:
>
> Hi Denys, thanks a lot for the quick patch.
> I reran our tool on top, and it did find an error in the patch. The
> problem is that you are using alloca() in getopt32():
>
> 495: if (pargv[0][0] != '-' && pargv[0][0] != '\0') {
> 496: char *pp = alloca(strlen(*pargv) + 2);
>
> which is later dereferenced here on top.c:768, after getopt32() returns:
>
> 766: if (getopt32(argv, "d:n:b", &sinterval, &iterations) & OPT_d) {
> 767: /* Need to limit it to not overflow poll timeout */
> 768: interval = xatou16(sinterval); // -d
>
> After changing alloca to malloc, I didn't find any other problems in
> top.
I am going to apply this patch.
--
vda
diff -d -urpN busybox.8/libbb/getopt32.c busybox.9/libbb/getopt32.c
--- busybox.8/libbb/getopt32.c 2008-08-20 02:17:16.000000000 +0200
+++ busybox.9/libbb/getopt32.c 2008-08-21 01:03:09.000000000 +0200
@@ -155,12 +155,20 @@ Special characters:
Allows any arguments to be given without a dash (./program w x)
as well as with a dash (./program -x).
+ NB: getopt32() will leak a small amount of memory if you use
+ this option! Do not use it if there is a possibility of recursive
+ getopt32() calls.
+
"--" A double dash at the beginning of opt_complementary means the
argv[1] string should always be treated as options, even if it isn't
prefixed with a "-". This is useful for special syntax in applets
such as "ar" and "tar":
tar xvf foo.tar
+ NB: getopt32() will leak a small amount of memory if you use
+ this option! Do not use it if there is a possibility of recursive
+ getopt32() calls.
+
"-N" A dash as the first char in a opt_complementary group followed
by a single digit (0-9) means that at least N non-option
arguments must be present on the command line
@@ -493,7 +501,10 @@ getopt32(char **argv, const char *applet
pargv = argv + 1;
while (*pargv) {
if (pargv[0][0] != '-' && pargv[0][0] != '\0') {
- char *pp = alloca(strlen(*pargv) + 2);
+ /* Can't use alloca: opts with params will
+ * return pointers to stack!
+ * NB: we leak these allocations... */
+ char *pp = xmalloc(strlen(*pargv) + 2);
*pp = '-';
strcpy(pp + 1, *pargv);
*pargv = pp;
_______________________________________________
busybox mailing list
[email protected]
http://busybox.net/cgi-bin/mailman/listinfo/busybox
