[PATCHv2] fixed resource leak in scripts/mod/modpost.c

[PATCHv2] fixed resource leak in scripts/mod/modpost.c

[Alexey Fomenko]

[-- Attachment #1: Type: text/plain, Size: 166 bytes --]

Patch modified for released 2.6.35 kernel. 
sec2annotation returns malloc'ed buffer directly to printf as an
argument. Patch lets free this buffer after printing. 



[-- Attachment #2: 0002-fixed-resource-leak-in-scripts-mod-modpost.c.patch --]
[-- Type: text/x-patch, Size: 4913 bytes --]

From: Alexey Fomenko <ext-alexey.fomenko@nokia.com>
Date: Thu, 05 Aug 2010 17:59:04 +0300
Subject: [PATCHv2] fixed resource leak in scripts/mod/modpost.c


Signed-off-by: Alexey Fomenko <ext-alexey.fomenko@nokia.com>
---
 scripts/mod/modpost.c |   55 ++++++++++++++++++++++++++++++++++++------------
 1 files changed, 40 insertions(+), 14 deletions(-)

diff -u -r 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-02 01:11:14.000000000 +0300
+++ linux-2.6.35_b/scripts/mod/modpost.c	2010-08-05 13:25:58.856270161 +0300
@@ -1195,6 +1195,8 @@
 {
 	const char *from, *from_p;
 	const char *to, *to_p;
+	char *prl_from;
+	char *prl_to;
 
 	switch (from_is_func) {
 	case 0: from = "variable"; from_p = "";   break;
@@ -1218,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"
@@ -1235,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"
@@ -1256,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"
@@ -1282,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,
+		from, prl_from, fromsym, from_p,
+		to, prl_to, tosym, to_p,
 		sec2annotation(tosec), 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"
@@ -1295,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");

Re: [PATCHv2] fixed resource leak in scripts/mod/modpost.c

[Andy Shevchenko]

On Thu, Aug 5, 2010 at 3:09 PM, Alexey Fomenko
<ext-alexey.fomenko@nokia.com> wrote:
> Patch modified for released 2.6.35 kernel.
> sec2annotation returns malloc'ed buffer directly to printf as an
> argument. Patch lets free this buffer after printing.

Needs to be amended.
I told Alexey in private to resend new version of the patch.

-- 
With Best Regards,
Andy Shevchenko

[PATCHv3] fixed resource leak in scripts/mod/modpost.c

[Alexey Fomenko]

From: Alexey Fomenko <ext-alexey.fomenko@nokia.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@nokia.com>
---
 scripts/mod/modpost.c |   58 ++++++++++++++++++++++++++++++++++++------------
 1 files changed, 42 insertions(+), 16 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 12:11:25.174529109 +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,
+		from, prl_from, fromsym, from_p,
+		to, prl_to, tosym, to_p,
 		sec2annotation(tosec), 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");

Re: [PATCHv3] fixed resource leak in scripts/mod/modpost.c

[Andy Shevchenko]

On Tue, Aug 10, 2010 at 1:52 PM, Alexey Fomenko
<ext-alexey.fomenko@nokia.com> wrote:
> From: Alexey Fomenko <ext-alexey.fomenko@nokia.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@nokia.com>
> ---
>  scripts/mod/modpost.c |   58 ++++++++++++++++++++++++++++++++++++------------
>  1 files changed, 42 insertions(+), 16 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 12:11:25.174529109 +0300
> @@ -1165,9 +1165,9 @@
>                        strcat(p, "data ");
>                else
>                        strcat(p, " ");
> -               return r; /* we leak her but we do not care */
> +               return r;
Here is the extra tail whitespace.

> @@ -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,
> +               from, prl_from, fromsym, from_p,
> +               to, prl_to, tosym, to_p,
>                sec2annotation(tosec), tosym, to_p);
^^^^^^^^^^^^^^^^^^^^^^ missed change


Acked-by: Andy Shevchenko <ext-andriy.shevchenko@nokia.com>
(after fixing above issues)

-- 
With Best Regards,
Andy Shevchenko

[PATCHv4] fixed resource leak in scripts/mod/modpost.c

[Alexey Fomenko]

From: Alexey Fomenko <ext-alexey.fomenko@nokia.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@nokia.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,
+		ptl_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");

Re: [PATCHv4] fixed resource leak in scripts/mod/modpost.c

[Andy Shevchenko]

On Tue, Aug 10, 2010 at 3:03 PM, Alexey Fomenko
<ext-alexey.fomenko@nokia.com> wrote:
> From: Alexey Fomenko <ext-alexey.fomenko@nokia.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@nokia.com>

> @@ -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,
> +               ptl_to, tosym, to_p);
Did you ever compile this code?
It looks typo here.

> +               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"

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCHv4] fixed resource leak in scripts/mod/modpost.c

[Alexey Fomenko]

On Tue, 2010-08-10 at 14:21 +0200, ext Andy Shevchenko wrote:
> On Tue, Aug 10, 2010 at 3:03 PM, Alexey Fomenko
> <ext-alexey.fomenko@nokia.com> wrote:
> > From: Alexey Fomenko <ext-alexey.fomenko@nokia.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@nokia.com>
> 
> > @@ -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,
> > +               ptl_to, tosym, to_p);
> Did you ever compile this code?
> It looks typo here.
Ever - yes, last one - no, was in a hurry. Shouldn't have been.
I'll fix it.

> 
> > +               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"
> 

Re: [PATCHv4] modpost: fixed resource leak in report_sec_mismatch()

[Andy Shevchenko]

On Tue, Aug 10, 2010 at 4:09 PM, Alexey Fomenko
<ext-alexey.fomenko@nokia.com> wrote:
> From: Alexey Fomenko <ext-alexey.fomenko@nokia.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@nokia.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@nokia.com>


-- 
With Best Regards,
Andy Shevchenko