2013-06-06 02:49:35

by Kees Cook

[permalink] [raw]
Subject: [PATCH] doc: avoid strncpy in accounting tool

Avoid strncpy anti-pattern. Use strdup() instead, as already done for
the logfile optarg.

Signed-off-by: Kees Cook <[email protected]>
---
Fix for -mm clean-up-scary-strncpydst-src-strlensrc-uses-fix.patch
---
Documentation/accounting/getdelays.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/Documentation/accounting/getdelays.c b/Documentation/accounting/getdelays.c
index f8ebcde..1db89d3 100644
--- a/Documentation/accounting/getdelays.c
+++ b/Documentation/accounting/getdelays.c
@@ -272,7 +272,7 @@ int main(int argc, char *argv[])
char *logfile = NULL;
int loop = 0;
int containerset = 0;
- char containerpath[1024];
+ char *containerpath = NULL;
int cfd = 0;
int forking = 0;
sigset_t sigset;
@@ -299,7 +299,7 @@ int main(int argc, char *argv[])
break;
case 'C':
containerset = 1;
- strncpy(containerpath, optarg, strlen(optarg) + 1);
+ containerpath = strdup(optarg);
break;
case 'w':
logfile = strdup(optarg);
--
1.7.9.5


--
Kees Cook
Chrome OS Security


2013-06-06 16:52:30

by Andreas Schwab

[permalink] [raw]
Subject: Re: [PATCH] doc: avoid strncpy in accounting tool

Kees Cook <[email protected]> writes:

> Avoid strncpy anti-pattern. Use strdup() instead, as already done for
> the logfile optarg.

There should be no need to copy the string, option arguments are stable.

Andreas.

--
Andreas Schwab, [email protected]
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5
"And now for something completely different."

2013-06-09 01:59:47

by Rob Landley

[permalink] [raw]
Subject: Re: [PATCH] doc: avoid strncpy in accounting tool

On 06/05/2013 09:49:30 PM, Kees Cook wrote:
> Avoid strncpy anti-pattern. Use strdup() instead, as already done for
> the logfile optarg.
>
> Signed-off-by: Kees Cook <[email protected]>
> ---
> Fix for -mm clean-up-scary-strncpydst-src-strlensrc-uses-fix.patch
> ---
> Documentation/accounting/getdelays.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/accounting/getdelays.c
> b/Documentation/accounting/getdelays.c
> index f8ebcde..1db89d3 100644
> --- a/Documentation/accounting/getdelays.c
> +++ b/Documentation/accounting/getdelays.c
> @@ -272,7 +272,7 @@ int main(int argc, char *argv[])
> char *logfile = NULL;
> int loop = 0;
> int containerset = 0;
> - char containerpath[1024];
> + char *containerpath = NULL;
> int cfd = 0;
> int forking = 0;
> sigset_t sigset;
> @@ -299,7 +299,7 @@ int main(int argc, char *argv[])
> break;
> case 'C':
> containerset = 1;
> - strncpy(containerpath, optarg, strlen(optarg) +
> 1);
> + containerpath = strdup(optarg);

*boggle* That an elaborate way of doing a standard strcpy(), isn't it?

(Assuming free() being done by exit() is ok, and that somebody's going
to be modifying this string so just keeping the pointer to optarg might
make ps look weird...)

Acked-by: Rob Landley <[email protected]>

if Andrew hasn't grabbed it already (I'm days behind on email) please
send through [email protected].

Rob-

2013-06-10 21:21:21

by Andrew Morton

[permalink] [raw]
Subject: Re: [PATCH] doc: avoid strncpy in accounting tool

On Wed, 5 Jun 2013 19:49:30 -0700 Kees Cook <[email protected]> wrote:

> Avoid strncpy anti-pattern. Use strdup() instead, as already done for
> the logfile optarg.
>

What Andreas said :)

From: Andrew Morton <[email protected]>
Subject: documentation-accounting-getdelaysc-avoid-strncpy-in-accounting-tool-fix

remove the str[cpy|dup] altogether

Cc: Andreas Schwab <[email protected]>
Cc: Kees Cook <[email protected]>
Cc: Rob Landley <[email protected]>
Signed-off-by: Andrew Morton <[email protected]>
---

Documentation/accounting/getdelays.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff -puN Documentation/accounting/getdelays.c~documentation-accounting-getdelaysc-avoid-strncpy-in-accounting-tool-fix Documentation/accounting/getdelays.c
--- a/Documentation/accounting/getdelays.c~documentation-accounting-getdelaysc-avoid-strncpy-in-accounting-tool-fix
+++ a/Documentation/accounting/getdelays.c
@@ -299,7 +299,7 @@ int main(int argc, char *argv[])
break;
case 'C':
containerset = 1;
- containerpath = strdup(optarg);
+ containerpath = optarg;
break;
case 'w':
logfile = strdup(optarg);
_