[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <AANLkTinD4ze=pZpGGmJ5vor9sbbBfuktAdRqpqiEHZW7@mail.gmail.com>
Date: Mon, 21 Mar 2011 15:35:27 -0400
From: Arnaud Lacombe <lacombar@...il.com>
To: Jesper Juhl <jj@...osbits.net>
Cc: linux-kbuild@...r.kernel.org, linux-kernel@...r.kernel.org,
WANG Cong <xiyou.wangcong@...il.com>,
James Morris <jmorris@...ercode.com.au>,
James Morris <jmorris@...ei.org>,
"David S. Miller" <davem@...hat.com>,
David Miller <davem@...emloft.net>,
Steve French <sfrench@...ibm.com>,
Steve French <smfrench@...il.com>,
Andrew Tridgell <tridge@...ba.org>
Subject: Re: [PATCH][resend] Do not potentially overflow string in sumversion
Hi,
On Mon, Mar 21, 2011 at 3:17 PM, Jesper Juhl <jj@...osbits.net> 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@...osbits.net>
> Acked-by: WANG Cong <xiyou.wangcong@...il.com>
> ---
> 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@...osbits.net> 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@...r.kernel.org
> 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@...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