| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <AANLkTik=to72=ObiFjOL7zZBnxqbbV2=rsdyLRVYy+_4@mail.gmail.com>
Date: Tue, 10 Aug 2010 16:43:08 +0300
From: Andy Shevchenko <andy.shevchenko@...il.com>
To: Alexey Fomenko <ext-alexey.fomenko@...ia.com>
Cc: "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
Trevor Keith <tsrk@...k.net>,
Rusty Russell <rusty@...tcorp.com.au>,
ext Andrew Morton <akpm@...ux-foundation.org>
Subject: Re: [PATCHv4] modpost: fixed resource leak in report_sec_mismatch()
On Tue, Aug 10, 2010 at 4:09 PM, Alexey Fomenko
<ext-alexey.fomenko@...ia.com> wrote:
> From: Alexey Fomenko <ext-alexey.fomenko@...ia.com>
>
> sec2annotation() returns malloc'ed buffer directly to printf as an
> argument. Patch lets free this buffer after printing. Preventing ops
> while freeing the buffer by changing return const str to return
> strdup empty line.
> Signed-off-by: Alexey Fomenko <ext-alexey.fomenko@...ia.com>
> ---
> scripts/mod/modpost.c | 62
> ++++++++++++++++++++++++++++++++----------------
> 1 files changed, 43 insertions(+), 19 deletions(-)
>
> diff -ur linux-2.6.35/scripts/mod/modpost.c
> linux-2.6.35_b/scripts/mod/modpost.c
> --- linux-2.6.35/scripts/mod/modpost.c 2010-08-10 12:11:03.854528620 +0300
> +++ linux-2.6.35_b/scripts/mod/modpost.c 2010-08-10 14:45:00.294529178 +0300
> @@ -1165,9 +1165,9 @@
> strcat(p, "data ");
> else
> strcat(p, " ");
> - return r; /* we leak her but we do not care */
> + return r;
> } else {
> - return "";
> + return strdup("");
> }
> }
>
> @@ -1195,6 +1195,8 @@
> {
> const char *from, *from_p;
> const char *to, *to_p;
> + char *prl_from;
> + char *prl_to;
>
>
> switch (from_is_func) {
> @@ -1219,16 +1221,21 @@
>
> switch (mismatch->mismatch) {
> case TEXT_TO_ANY_INIT:
> + prl_from = sec2annotation(fromsec);
> + prl_to = sec2annotation(tosec);
> fprintf(stderr,
> "The function %s%s() references\n"
> "the %s %s%s%s.\n"
> "This is often because %s lacks a %s\n"
> "annotation or the annotation of %s is wrong.\n",
> - sec2annotation(fromsec), fromsym,
> - to, sec2annotation(tosec), tosym, to_p,
> - fromsym, sec2annotation(tosec), tosym);
> + prl_from, fromsym,
> + to, prl_to, tosym, to_p,
> + fromsym, prl_to, tosym);
> + free(prl_from);
> + free(prl_to);
> break;
> case DATA_TO_ANY_INIT: {
> + prl_to = sec2annotation(tosec);
> const char *const *s = mismatch->symbol_white_list;
> fprintf(stderr,
> "The variable %s references\n"
> @@ -1236,20 +1243,24 @@
> "If the reference is valid then annotate the\n"
> "variable with __init* or __refdata (see linux/init.h) "
> "or name the variable:\n",
> - fromsym, to, sec2annotation(tosec), tosym, to_p);
> + fromsym, to, prl_to, tosym, to_p);
> while (*s)
> fprintf(stderr, "%s, ", *s++);
> fprintf(stderr, "\n");
> + free(prl_to);
> break;
> }
> case TEXT_TO_ANY_EXIT:
> + prl_to = sec2annotation(tosec);
> fprintf(stderr,
> "The function %s() references a %s in an exit section.\n"
> "Often the %s %s%s has valid usage outside the exit section\n"
> "and the fix is to remove the %sannotation of %s.\n",
> - fromsym, to, to, tosym, to_p, sec2annotation(tosec), tosym);
> + fromsym, to, to, tosym, to_p, prl_to, tosym);
> + free(prl_to);
> break;
> case DATA_TO_ANY_EXIT: {
> + prl_to = sec2annotation(tosec);
> const char *const *s = mismatch->symbol_white_list;
> fprintf(stderr,
> "The variable %s references\n"
> @@ -1257,24 +1268,31 @@
> "If the reference is valid then annotate the\n"
> "variable with __exit* (see linux/init.h) or "
> "name the variable:\n",
> - fromsym, to, sec2annotation(tosec), tosym, to_p);
> + fromsym, to, prl_to, tosym, to_p);
> while (*s)
> fprintf(stderr, "%s, ", *s++);
> fprintf(stderr, "\n");
> + free(prl_to);
> break;
> }
> case XXXINIT_TO_SOME_INIT:
> case XXXEXIT_TO_SOME_EXIT:
> + prl_from = sec2annotation(fromsec);
> + prl_to = sec2annotation(tosec);
> fprintf(stderr,
> "The %s %s%s%s references\n"
> "a %s %s%s%s.\n"
> "If %s is only used by %s then\n"
> "annotate %s with a matching annotation.\n",
> - from, sec2annotation(fromsec), fromsym, from_p,
> - to, sec2annotation(tosec), tosym, to_p,
> + from, prl_from, fromsym, from_p,
> + to, prl_to, tosym, to_p,
> tosym, fromsym, tosym);
> + free(prl_from);
> + free(prl_to);
> break;
> case ANY_INIT_TO_ANY_EXIT:
> + prl_from = sec2annotation(fromsec);
> + prl_to = sec2annotation(tosec);
> fprintf(stderr,
> "The %s %s%s%s references\n"
> "a %s %s%s%s.\n"
> @@ -1283,11 +1301,15 @@
> "uses functionality in the exit path.\n"
> "The fix is often to remove the %sannotation of\n"
> "%s%s so it may be used outside an exit section.\n",
> - from, sec2annotation(fromsec), fromsym, from_p,
> - to, sec2annotation(tosec), tosym, to_p,
> - sec2annotation(tosec), tosym, to_p);
> + from, prl_from, fromsym, from_p,
> + to, prl_to, tosym, to_p,
> + prl_to, tosym, to_p);
> + free(prl_from);
> + free(prl_to);
> break;
> case ANY_EXIT_TO_ANY_INIT:
> + prl_from = sec2annotation(fromsec);
> + prl_to = sec2annotation(tosec);
> fprintf(stderr,
> "The %s %s%s%s references\n"
> "a %s %s%s%s.\n"
> @@ -1296,16 +1318,20 @@
> "uses functionality in the init path.\n"
> "The fix is often to remove the %sannotation of\n"
> "%s%s so it may be used outside an init section.\n",
> - from, sec2annotation(fromsec), fromsym, from_p,
> - to, sec2annotation(tosec), tosym, to_p,
> - sec2annotation(tosec), tosym, to_p);
> + from, prl_from, fromsym, from_p,
> + to, prl_to, tosym, to_p,
> + prl_to, tosym, to_p);
> + free(prl_from);
> + free(prl_to);
> break;
> case EXPORT_TO_INIT_EXIT:
> + prl_to = sec2annotation(tosec);
> fprintf(stderr,
> "The symbol %s is exported and annotated %s\n"
> "Fix this by removing the %sannotation of %s "
> "or drop the export.\n",
> - tosym, sec2annotation(tosec), sec2annotation(tosec), tosym);
> + tosym, prl_to, prl_to, tosym);
> + free(prl_to);
> break;
> }
> fprintf(stderr, "\n");
>
>
Now seems fine to me.
Reviewed-by: Andy Shevchenko <ext-andriy.shevchenko@...ia.com>
--
With Best Regards,
Andy Shevchenko
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists