Re: [PATCH][resend] Do not potentially overflow string in sumversion

[Arnaud Lacombe]

Hi,

On Mon, Mar 21, 2011 at 3:17 PM, Jesper Juhl <jj@xxxxxxxxxxxxx> wrote:
> In scripts/mod/sumversion.c (in get_src_version()) we call
> getenv("MODVERDIR"). This returns a pointer to a string of unknown length.
> This string of unknown length we then pass on as an argument to sprintf()
> and tell it to write the result to 'filelist' which has a, very much
> fixed, size of 'PATH_MAX + 1'. If the string returned by getenv() is too
> long we'll overrun the statically allocated buffer.
> This patch prevents the buffer overrun by using snprintf() and telling it
> to copy a maximum of 'PATH_MAX + 1' bytes (including the terminating \0).
>
> Signed-off-by: Jesper Juhl <jj@xxxxxxxxxxxxx>
> Acked-by: WANG Cong <xiyou.wangcong@xxxxxxxxx>
> ---
>  sumversion.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
>  compile tested only. Patch against Linus' tree as of right now.
>
> diff --git
> a/scripts/mod/sumversion.c
> b/scripts/mod/sumversion.c
> index 9dfcd6d..522c675 100644
> --- a/scripts/mod/sumversion.c
> +++ b/scripts/mod/sumversion.c
> @@ -416,7 +416,7 @@ void get_src_version(const char *modname, char sum[], unsigned sumlen)
>                basename = strrchr(modname, '/') + 1;
>        else
>                basename = modname;
> -       sprintf(filelist, "%s/%.*s.mod", modverdir,
> +       snprintf(filelist, PATH_MAX + 1, "%s/%.*s.mod", modverdir,
>                (int) strlen(basename) - 2, basename);
>
Please, do not use hardcoded size. The `sizeof' operator would be much
more appropriate. Moreover, if you want thing to be done pedantically,
you should also check that you do not get a truncated output. If
snprintf() truncates the output, this should be reported and the error
path should be followed, not to fail silently.

 - Arnaud

>        file = grab_file(filelist, &len);
>
>
> --
> Jesper Juhl <jj@xxxxxxxxxxxxx>       http://www.chaosbits.net/
> Don't top-post http://www.catb.org/jargon/html/T/top-post.html
> Plain text mails only, please.
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
--
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/