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 <[email protected]>
Acked-by: WANG Cong <[email protected]>
---
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);
file = grab_file(filelist, &len);
--
Jesper Juhl <[email protected]> http://www.chaosbits.net/
Don't top-post http://www.catb.org/jargon/html/T/top-post.html
Plain text mails only, please.
Hi,
On Mon, Mar 21, 2011 at 3:17 PM, Jesper Juhl <[email protected]> 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 <[email protected]>
> Acked-by: WANG Cong <[email protected]>
> ---
> ?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 <[email protected]> ? ? ? 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 [email protected]
> More majordomo info at ?http://vger.kernel.org/majordomo-info.html
>