Re: [PATCH] Reduce stack usage in module.c

[Randy.Dunlap]

Yum Rayan wrote:
> On Tue, 29 Mar 2005 09:43:12 -0800, Randy.Dunlap <rddunlap@xxxxxxxx> wrote:
>
> > Yum Rayan wrote:
> >
> > > Attempt to reduce stack usage in module.c (linux-2.6.12-rc1-mm3).
> > > Specifically from checkstack.pl
>
> > > Also while at it, fix following in who_is_doing_it(...)
> > > - use only as much memory is needed
> > > - do not write past array index for the boundary case
> >
> > I don't see a boundary case problem with the current code,
> > hence I don't see why the kmalloc(len + 1, GFP_KERNEL) is
> > needed...
>
>
> Let's consider the original code and len = 513
>
>    1399 static void who_is_doing_it(void)
>    1400 {
>    1401         /* Print out all the args. */
>    1402         char args[512];
>    1403         unsigned long i, len = current->mm->arg_end -
> current->mm->arg_start;
>    1404
>    1405         if (len > 512)
>    1406                 len = 512;
>    1407
>    1408         len -= copy_from_user(args, (void
> *)current->mm->arg_start, len);
>    1409
>    1410         for (i = 0; i < len; i++) {
>    1411                 if (args[i] == '\0')
>    1412                         args[i] = ' ';
>    1413         }
>    1414         args[i] = 0;
>    1415         printk("ARGS: %s\n", args);
>    1416 }
>
> After lines 1410 thru 1413, "i" wil be 512. So line 1414 will be
> "args[512] = 0". But args is 512 byte array with last legally
> accessible element at 511?

Yes, it's so obvious (now).  :)

> > File names start one level deeper than wanted.  They should begin
> > with linux/ or a/ or ./ e.g.
> > There are plenty of docs on this, please let me know if you need
> > references to them.
>
>
> Point noted. Will post patch to linux/Documentation/SubmittingPatches,
> hopefully making it more clear. Reworked patch at end of email.

Good idea, thanks.

--
~Randy
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/