2014-10-05 13:04:18

by Rickard Strandqvist

[permalink] [raw]
Subject: [PATCH v2] lib: string.c: Added a funktion function strzcpy

I've added a strzcpy to make it easier to not do wrong
when you use the function strncpy.

Often one needs the zero padding, but must still have a
guaranteed zero character terminat.
Then you end up writing code like this:
strncpy(to, src, sizeof(to));
to[sizeof(to) - 1] = '\0';

This is solved with strzcpy.

Rickard Strandqvist (1):
lib: string.c: Added a funktion function strzcpy

include/linux/string.h | 1 +
lib/string.c | 31 +++++++++++++++++++++++++++++++
2 files changed, 32 insertions(+)

--
1.7.10.4


2014-10-05 13:04:29

by Rickard Strandqvist

[permalink] [raw]
Subject: [PATCH v2] lib: string.c: Added a funktion function strzcpy

Added a function strzcpy which works the same as strncpy,
but guaranteed to produce the trailing null character.

There are many places in the code where strncpy used although it
must be zero terminated, and switching to strlcpy is not an option
because the string must nonetheless be fyld with zero characters.

Signed-off-by: Rickard Strandqvist <[email protected]>
---
include/linux/string.h | 1 +
lib/string.c | 31 +++++++++++++++++++++++++++++++
2 files changed, 32 insertions(+)

diff --git a/include/linux/string.h b/include/linux/string.h
index d36977e..d789ee5e 100644
--- a/include/linux/string.h
+++ b/include/linux/string.h
@@ -22,6 +22,7 @@ extern char * strcpy(char *,const char *);
#ifndef __HAVE_ARCH_STRNCPY
extern char * strncpy(char *,const char *, __kernel_size_t);
#endif
+extern char *strzcpy(char *, const char *, __kernel_size_t);
#ifndef __HAVE_ARCH_STRLCPY
size_t strlcpy(char *, const char *, size_t);
#endif
diff --git a/lib/string.c b/lib/string.c
index f3c6ff5..582a832 100644
--- a/lib/string.c
+++ b/lib/string.c
@@ -134,6 +134,37 @@ char *strncpy(char *dest, const char *src, size_t count)
EXPORT_SYMBOL(strncpy);
#endif

+/**
+ * strzcpy - Copy a length-limited, C-string
+ * @dest: Where to copy the string to
+ * @src: Where to copy the string from
+ * @count: The maximum number of bytes to copy
+ *
+ * The result is %NUL-terminated,
+ * as long as count is greater than zero.
+ *
+ * In the case where the length of @src is less than that of
+ * count, the remainder of @dest will be padded with %NUL.
+ *
+ */
+char *strzcpy(char *dest, const char *src, size_t count)
+{
+ char *tmp = dest;
+
+ while (count) {
+ if ((*tmp = *src) != 0)
+ src++;
+ tmp++;
+ count--;
+ }
+
+ if (dest != tmp)
+ *--tmp = '\0';
+
+ return dest;
+}
+EXPORT_SYMBOL(strzcpy);
+
#ifndef __HAVE_ARCH_STRLCPY
/**
* strlcpy - Copy a C-string into a sized buffer
--
1.7.10.4

2014-10-15 22:15:18

by Andrew Morton

[permalink] [raw]
Subject: Re: [PATCH v2] lib: string.c: Added a funktion function strzcpy

On Sun, 5 Oct 2014 15:06:17 +0200 Rickard Strandqvist <[email protected]> wrote:

> Added a function strzcpy which works the same as strncpy,
> but guaranteed to produce the trailing null character.
>
> There are many places in the code where strncpy used although it
> must be zero terminated, and switching to strlcpy is not an option
> because the string must nonetheless be fyld with zero characters.

As I mentioned last time, I think this patch would be better if it came
with follow-on patches which convert at least some of those callsites.
As it stands, this function has no callers and hence it won't get
tested. Plus those follow-on patches will demonstrate the value of
this patch and will provide example usages.

2014-10-16 21:09:23

by Rickard Strandqvist

[permalink] [raw]
Subject: Re: [PATCH v2] lib: string.c: Added a funktion function strzcpy

2014-10-16 0:15 GMT+02:00 Andrew Morton <[email protected]>:
> On Sun, 5 Oct 2014 15:06:17 +0200 Rickard Strandqvist <[email protected]> wrote:
>
>> Added a function strzcpy which works the same as strncpy,
>> but guaranteed to produce the trailing null character.
>>
>> There are many places in the code where strncpy used although it
>> must be zero terminated, and switching to strlcpy is not an option
>> because the string must nonetheless be fyld with zero characters.
>
> As I mentioned last time, I think this patch would be better if it came
> with follow-on patches which convert at least some of those callsites.
> As it stands, this function has no callers and hence it won't get
> tested. Plus those follow-on patches will demonstrate the value of
> this patch and will provide example usages.


Hi

Sure I can do that! I have saved some patches just to be able to use
this new feature.
But should I submit everything as one patch then?
Or is there some kind of dependency thing I can use...

I have also e-mailed with Dan about this, he pointed out the same as
some of my tests indicate that strzcpy maybe just should use strncpy
and add a null character instead because strncpy is optimized
depending on the hardware it runs on.
What do you think about that?


Kind regards
Rickard Strandqvist

2014-10-16 21:17:29

by Andrew Morton

[permalink] [raw]
Subject: Re: [PATCH v2] lib: string.c: Added a funktion function strzcpy

On Thu, 16 Oct 2014 23:09:00 +0200 Rickard Strandqvist <[email protected]> wrote:

> 2014-10-16 0:15 GMT+02:00 Andrew Morton <[email protected]>:
> > On Sun, 5 Oct 2014 15:06:17 +0200 Rickard Strandqvist <[email protected]> wrote:
> >
> >> Added a function strzcpy which works the same as strncpy,
> >> but guaranteed to produce the trailing null character.
> >>
> >> There are many places in the code where strncpy used although it
> >> must be zero terminated, and switching to strlcpy is not an option
> >> because the string must nonetheless be fyld with zero characters.
> >
> > As I mentioned last time, I think this patch would be better if it came
> > with follow-on patches which convert at least some of those callsites.
> > As it stands, this function has no callers and hence it won't get
> > tested. Plus those follow-on patches will demonstrate the value of
> > this patch and will provide example usages.
>
>
> Hi
>
> Sure I can do that! I have saved some patches just to be able to use
> this new feature.
> But should I submit everything as one patch then?
> Or is there some kind of dependency thing I can use...

[patch 1/N] lib/string.c: add strzcpy()
[patch 2/N] foo/bar/zot.c: use strzcpy()
[patch 3/N] fooz/barz/zot.c: use strzcpy()
...

> I have also e-mailed with Dan about this, he pointed out the same as
> some of my tests indicate that strzcpy maybe just should use strncpy
> and add a null character instead because strncpy is optimized
> depending on the hardware it runs on.
> What do you think about that?

Sounds like strzcpy() should be used in places where the entire buffer
will be copied out to userspace, or in other situations where we want to
zero it out for security reasons?

2014-10-16 21:29:22

by Rickard Strandqvist

[permalink] [raw]
Subject: Re: [PATCH v2] lib: string.c: Added a funktion function strzcpy

2014-10-16 23:17 GMT+02:00 Andrew Morton <[email protected]>:
> On Thu, 16 Oct 2014 23:09:00 +0200 Rickard Strandqvist <[email protected]> wrote:
>
>> 2014-10-16 0:15 GMT+02:00 Andrew Morton <[email protected]>:
>> > On Sun, 5 Oct 2014 15:06:17 +0200 Rickard Strandqvist <[email protected]> wrote:
>> >
>> >> Added a function strzcpy which works the same as strncpy,
>> >> but guaranteed to produce the trailing null character.
>> >>
>> >> There are many places in the code where strncpy used although it
>> >> must be zero terminated, and switching to strlcpy is not an option
>> >> because the string must nonetheless be fyld with zero characters.
>> >
>> > As I mentioned last time, I think this patch would be better if it came
>> > with follow-on patches which convert at least some of those callsites.
>> > As it stands, this function has no callers and hence it won't get
>> > tested. Plus those follow-on patches will demonstrate the value of
>> > this patch and will provide example usages.
>>
>>
>> Hi
>>
>> Sure I can do that! I have saved some patches just to be able to use
>> this new feature.
>> But should I submit everything as one patch then?
>> Or is there some kind of dependency thing I can use...
>
> [patch 1/N] lib/string.c: add strzcpy()
> [patch 2/N] foo/bar/zot.c: use strzcpy()
> [patch 3/N] fooz/barz/zot.c: use strzcpy()
> ...
>
>> I have also e-mailed with Dan about this, he pointed out the same as
>> some of my tests indicate that strzcpy maybe just should use strncpy
>> and add a null character instead because strncpy is optimized
>> depending on the hardware it runs on.
>> What do you think about that?
>
> Sounds like strzcpy() should be used in places where the entire buffer
> will be copied out to userspace, or in other situations where we want to
> zero it out for security reasons?


Hi

So that's how you write it, ok I will send them this weekend when I
have some more time.

Yes it was the safety aspect I was most yelled at when I start
swapping strncpy with strlcpy, although much of it was justified!

Even Linus was getting into the debate. See more:
https://plus.google.com/111049168280159033135/posts/1amLbuhWbh5


Kind regards
Rickard Strandqvist