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@xxxxxxxxx> wrote:
> From: Alexey Fomenko <ext-alexey.fomenko@xxxxxxxxx>
>
> 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@xxxxxxxxx>
> ---
>  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@xxxxxxxxx>


-- 
With Best Regards,
Andy Shevchenko
--
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/