All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] checkpatch: Added warnings in favor of strscpy().
@ 2019-07-04  5:54 Nitin Gote
  2019-07-04 20:46   ` Joe Perches
  0 siblings, 1 reply; 14+ messages in thread
From: Nitin Gote @ 2019-07-04  5:54 UTC (permalink / raw)
  To: akpm
  Cc: corbet, apw, joe, keescook, linux-doc, linux-kernel,
	kernel-hardening, Nitin Gote

Added warnings in checkpatch.pl script to :

1. Deprecate strcpy() in favor of strscpy().
2. Deprecate strlcpy() in favor of strscpy().
3. Deprecate strncpy() in favor of strscpy() or strscpy_pad().

Updated strncpy() section in Documentation/process/deprecated.rst
to cover strscpy_pad() case.

Signed-off-by: Nitin Gote <nitin.r.gote@intel.com>
---
 This patch is already reviewed by mailing list
 kernel-hardening@lists.openwall.com. Refer below link
 <https://www.openwall.com/lists/kernel-hardening/2019/07/03/4>
Acked-by: Kees Cook <keescook@chromium.org>

 Documentation/process/deprecated.rst | 6 +++---
 scripts/checkpatch.pl                | 5 +++++
 2 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/Documentation/process/deprecated.rst b/Documentation/process/deprecated.rst
index 49e0f64..f564de3 100644
--- a/Documentation/process/deprecated.rst
+++ b/Documentation/process/deprecated.rst
@@ -93,9 +93,9 @@ will be NUL terminated. This can lead to various linear read overflows
 and other misbehavior due to the missing termination. It also NUL-pads the
 destination buffer if the source contents are shorter than the destination
 buffer size, which may be a needless performance penalty for callers using
-only NUL-terminated strings. The safe replacement is :c:func:`strscpy`.
-(Users of :c:func:`strscpy` still needing NUL-padding will need an
-explicit :c:func:`memset` added.)
+only NUL-terminated strings. In this case, the safe replacement is
+:c:func:`strscpy`. If, however, the destination buffer still needs
+NUL-padding, the safe replacement is :c:func:`strscpy_pad`.
 
 If a caller is using non-NUL-terminated strings, :c:func:`strncpy()` can
 still be used, but destinations should be marked with the `__nonstring
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 342c7c7..3d80967 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -595,6 +595,11 @@ our %deprecated_apis = (
 	"rcu_barrier_sched"			=> "rcu_barrier",
 	"get_state_synchronize_sched"		=> "get_state_synchronize_rcu",
 	"cond_synchronize_sched"		=> "cond_synchronize_rcu",
+	"strcpy"				=> "strscpy",
+	"strlcpy"				=> "strscpy",
+	"strncpy"				=> "strscpy, strscpy_pad or for
+	non-NUL-terminated strings, strncpy() can still be used, but
+	destinations should be marked with the __nonstring",
 );
 
 #Create a search pattern for all these strings to speed up a loop below
-- 
2.7.4


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] checkpatch: Added warnings in favor of strscpy().
  2019-07-04  5:54 [PATCH] checkpatch: Added warnings in favor of strscpy() Nitin Gote
@ 2019-07-04 20:46   ` Joe Perches
  0 siblings, 0 replies; 14+ messages in thread
From: Joe Perches @ 2019-07-04 20:46 UTC (permalink / raw)
  To: Nitin Gote, akpm
  Cc: corbet, apw, keescook, linux-doc, linux-kernel, kernel-hardening

On Thu, 2019-07-04 at 11:24 +0530, Nitin Gote wrote:
> Added warnings in checkpatch.pl script to :
> 
> 1. Deprecate strcpy() in favor of strscpy().
> 2. Deprecate strlcpy() in favor of strscpy().
> 3. Deprecate strncpy() in favor of strscpy() or strscpy_pad().
> 
> Updated strncpy() section in Documentation/process/deprecated.rst
> to cover strscpy_pad() case.
> 
> Signed-off-by: Nitin Gote <nitin.r.gote@intel.com>

OK, for whatever reason, this when into a spam folder.

> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
[]
> @@ -595,6 +595,11 @@ our %deprecated_apis = (
>  	"rcu_barrier_sched"			=> "rcu_barrier",
>  	"get_state_synchronize_sched"		=> "get_state_synchronize_rcu",
>  	"cond_synchronize_sched"		=> "cond_synchronize_rcu",
> +	"strcpy"				=> "strscpy",
> +	"strlcpy"				=> "strscpy",
> +	"strncpy"				=> "strscpy, strscpy_pad or for
> +	non-NUL-terminated strings, strncpy() can still be used, but
> +	destinations should be marked with the __nonstring",
>  );

$ git grep -w strcpy | wc -l
2239
$ git grep -w strlcpy | wc -l
1760
$ git grep -w strncpy | wc -l
839

These functions are _really_ commonly used in the kernel.

This should probably be a different %deprecated_string_api
and these should probably not be emitted at WARN level
when using command line option -f/--file but at CHECK level
so that novice script users just don't send bad patches.

Also, perhaps there could be some macro for the relatively
commonly used

	strscpy(foo, bar, sizeof(foo))
and
	strlcpy(foo, bar, sizeof(foo))

so argument 1 doesn't have to be repeated in the sizeof()

Something like:

#define stracpy(to, from)					\
({								\
	size_t size = ARRAY_SIZE(to);				\
	BUILD_BUG_ON(!__same_type(typeof(*to), char));		\
								\
	strscpy(to, from, size);				\
})



^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] checkpatch: Added warnings in favor of strscpy().
@ 2019-07-04 20:46   ` Joe Perches
  0 siblings, 0 replies; 14+ messages in thread
From: Joe Perches @ 2019-07-04 20:46 UTC (permalink / raw)
  To: Nitin Gote, akpm
  Cc: corbet, apw, keescook, linux-doc, linux-kernel, kernel-hardening

On Thu, 2019-07-04 at 11:24 +0530, Nitin Gote wrote:
> Added warnings in checkpatch.pl script to :
> 
> 1. Deprecate strcpy() in favor of strscpy().
> 2. Deprecate strlcpy() in favor of strscpy().
> 3. Deprecate strncpy() in favor of strscpy() or strscpy_pad().
> 
> Updated strncpy() section in Documentation/process/deprecated.rst
> to cover strscpy_pad() case.
> 
> Signed-off-by: Nitin Gote <nitin.r.gote@intel.com>

OK, for whatever reason, this when into a spam folder.

> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
[]
> @@ -595,6 +595,11 @@ our %deprecated_apis = (
>  	"rcu_barrier_sched"			=> "rcu_barrier",
>  	"get_state_synchronize_sched"		=> "get_state_synchronize_rcu",
>  	"cond_synchronize_sched"		=> "cond_synchronize_rcu",
> +	"strcpy"				=> "strscpy",
> +	"strlcpy"				=> "strscpy",
> +	"strncpy"				=> "strscpy, strscpy_pad or for
> +	non-NUL-terminated strings, strncpy() can still be used, but
> +	destinations should be marked with the __nonstring",
>  );

$ git grep -w strcpy | wc -l
2239
$ git grep -w strlcpy | wc -l
1760
$ git grep -w strncpy | wc -l
839

These functions are _really_ commonly used in the kernel.

This should probably be a different %deprecated_string_api
and these should probably not be emitted at WARN level
when using command line option -f/--file but at CHECK level
so that novice script users just don't send bad patches.

Also, perhaps there could be some macro for the relatively
commonly used

	strscpy(foo, bar, sizeof(foo))
and
	strlcpy(foo, bar, sizeof(foo))

so argument 1 doesn't have to be repeated in the sizeof()

Something like:

#define stracpy(to, from)					\
({								\
	size_t size = ARRAY_SIZE(to);				\
	BUILD_BUG_ON(!__same_type(typeof(*to), char));		\
								\
	strscpy(to, from, size);				\
})



^ permalink raw reply	[flat|nested] 14+ messages in thread

* [RFC PATCH] string.h: Add stracpy/stracpy_pad (was: Re: [PATCH] checkpatch: Added warnings in favor of strscpy().)
  2019-07-04 20:46   ` Joe Perches
@ 2019-07-05  0:15     ` Joe Perches
  -1 siblings, 0 replies; 14+ messages in thread
From: Joe Perches @ 2019-07-05  0:15 UTC (permalink / raw)
  To: Nitin Gote, akpm
  Cc: corbet, apw, keescook, linux-doc, linux-kernel, kernel-hardening

On Thu, 2019-07-04 at 13:46 -0700, Joe Perches wrote:
> On Thu, 2019-07-04 at 11:24 +0530, Nitin Gote wrote:
> > Added warnings in checkpatch.pl script to :
> > 
> > 1. Deprecate strcpy() in favor of strscpy().
> > 2. Deprecate strlcpy() in favor of strscpy().
> > 3. Deprecate strncpy() in favor of strscpy() or strscpy_pad().
> > 
> > Updated strncpy() section in Documentation/process/deprecated.rst
> > to cover strscpy_pad() case.

[]

I sent a patch series for some strscpy/strlcpy misuses.

How about adding a macro helper to avoid the misuses like:
---
 include/linux/string.h | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/include/linux/string.h b/include/linux/string.h
index 4deb11f7976b..ef01bd6f19df 100644
--- a/include/linux/string.h
+++ b/include/linux/string.h
@@ -35,6 +35,22 @@ ssize_t strscpy(char *, const char *, size_t);
 /* Wraps calls to strscpy()/memset(), no arch specific code required */
 ssize_t strscpy_pad(char *dest, const char *src, size_t count);
 
+#define stracpy(to, from)					\
+({								\
+	size_t size = ARRAY_SIZE(to);				\
+	BUILD_BUG_ON(!__same_type(typeof(*to), char));		\
+								\
+	strscpy(to, from, size);				\
+})
+
+#define stracpy_pad(to, from)					\
+({								\
+	size_t size = ARRAY_SIZE(to);				\
+	BUILD_BUG_ON(!__same_type(typeof(*to), char));		\
+								\
+	strscpy_pad(to, from, size);				\
+})
+
 #ifndef __HAVE_ARCH_STRCAT
 extern char * strcat(char *, const char *);
 #endif



^ permalink raw reply	[flat|nested] 14+ messages in thread

* [RFC PATCH] string.h: Add stracpy/stracpy_pad (was: Re: [PATCH] checkpatch: Added warnings in favor of strscpy().)
@ 2019-07-05  0:15     ` Joe Perches
  0 siblings, 0 replies; 14+ messages in thread
From: Joe Perches @ 2019-07-05  0:15 UTC (permalink / raw)
  To: Nitin Gote, akpm
  Cc: corbet, apw, keescook, linux-doc, linux-kernel, kernel-hardening

On Thu, 2019-07-04 at 13:46 -0700, Joe Perches wrote:
> On Thu, 2019-07-04 at 11:24 +0530, Nitin Gote wrote:
> > Added warnings in checkpatch.pl script to :
> > 
> > 1. Deprecate strcpy() in favor of strscpy().
> > 2. Deprecate strlcpy() in favor of strscpy().
> > 3. Deprecate strncpy() in favor of strscpy() or strscpy_pad().
> > 
> > Updated strncpy() section in Documentation/process/deprecated.rst
> > to cover strscpy_pad() case.

[]

I sent a patch series for some strscpy/strlcpy misuses.

How about adding a macro helper to avoid the misuses like:
---
 include/linux/string.h | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/include/linux/string.h b/include/linux/string.h
index 4deb11f7976b..ef01bd6f19df 100644
--- a/include/linux/string.h
+++ b/include/linux/string.h
@@ -35,6 +35,22 @@ ssize_t strscpy(char *, const char *, size_t);
 /* Wraps calls to strscpy()/memset(), no arch specific code required */
 ssize_t strscpy_pad(char *dest, const char *src, size_t count);
 
+#define stracpy(to, from)					\
+({								\
+	size_t size = ARRAY_SIZE(to);				\
+	BUILD_BUG_ON(!__same_type(typeof(*to), char));		\
+								\
+	strscpy(to, from, size);				\
+})
+
+#define stracpy_pad(to, from)					\
+({								\
+	size_t size = ARRAY_SIZE(to);				\
+	BUILD_BUG_ON(!__same_type(typeof(*to), char));		\
+								\
+	strscpy_pad(to, from, size);				\
+})
+
 #ifndef __HAVE_ARCH_STRCAT
 extern char * strcat(char *, const char *);
 #endif



^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [RFC PATCH] string.h: Add stracpy/stracpy_pad (was: Re: [PATCH] checkpatch: Added warnings in favor of strscpy().)
  2019-07-05  0:15     ` Joe Perches
  (?)
@ 2019-07-22 17:33     ` Kees Cook
  2019-07-22 17:43         ` Joe Perches
  -1 siblings, 1 reply; 14+ messages in thread
From: Kees Cook @ 2019-07-22 17:33 UTC (permalink / raw)
  To: Joe Perches
  Cc: Nitin Gote, akpm, corbet, apw, linux-doc, linux-kernel,
	kernel-hardening, Rasmus Villemoes

On Thu, Jul 04, 2019 at 05:15:57PM -0700, Joe Perches wrote:
> On Thu, 2019-07-04 at 13:46 -0700, Joe Perches wrote:
> > On Thu, 2019-07-04 at 11:24 +0530, Nitin Gote wrote:
> > > Added warnings in checkpatch.pl script to :
> > > 
> > > 1. Deprecate strcpy() in favor of strscpy().
> > > 2. Deprecate strlcpy() in favor of strscpy().
> > > 3. Deprecate strncpy() in favor of strscpy() or strscpy_pad().
> > > 
> > > Updated strncpy() section in Documentation/process/deprecated.rst
> > > to cover strscpy_pad() case.
> 
> []
> 
> I sent a patch series for some strscpy/strlcpy misuses.
> 
> How about adding a macro helper to avoid the misuses like:
> ---
>  include/linux/string.h | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
> 
> diff --git a/include/linux/string.h b/include/linux/string.h
> index 4deb11f7976b..ef01bd6f19df 100644
> --- a/include/linux/string.h
> +++ b/include/linux/string.h
> @@ -35,6 +35,22 @@ ssize_t strscpy(char *, const char *, size_t);
>  /* Wraps calls to strscpy()/memset(), no arch specific code required */
>  ssize_t strscpy_pad(char *dest, const char *src, size_t count);
>  
> +#define stracpy(to, from)					\
> +({								\
> +	size_t size = ARRAY_SIZE(to);				\
> +	BUILD_BUG_ON(!__same_type(typeof(*to), char));		\
> +								\
> +	strscpy(to, from, size);				\
> +})
> +
> +#define stracpy_pad(to, from)					\
> +({								\
> +	size_t size = ARRAY_SIZE(to);				\
> +	BUILD_BUG_ON(!__same_type(typeof(*to), char));		\
> +								\
> +	strscpy_pad(to, from, size);				\
> +})
> +
>  #ifndef __HAVE_ARCH_STRCAT
>  extern char * strcat(char *, const char *);
>  #endif

This seems like a reasonable addition, yes. I think Coccinelle might
actually be able to find all the existing strscpy(dst, src, sizeof(dst))
cases to jump-start this conversion.

Devil's advocate: this adds yet more string handling functions... will
this cause even more confusion?

-- 
Kees Cook

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [RFC PATCH] string.h: Add stracpy/stracpy_pad (was: Re: [PATCH] checkpatch: Added warnings in favor of strscpy().)
  2019-07-22 17:33     ` Kees Cook
@ 2019-07-22 17:43         ` Joe Perches
  0 siblings, 0 replies; 14+ messages in thread
From: Joe Perches @ 2019-07-22 17:43 UTC (permalink / raw)
  To: Kees Cook
  Cc: Nitin Gote, akpm, corbet, apw, linux-doc, linux-kernel,
	kernel-hardening, Rasmus Villemoes

On Mon, 2019-07-22 at 10:33 -0700, Kees Cook wrote:
> On Thu, Jul 04, 2019 at 05:15:57PM -0700, Joe Perches wrote:
> > On Thu, 2019-07-04 at 13:46 -0700, Joe Perches wrote:
> > > On Thu, 2019-07-04 at 11:24 +0530, Nitin Gote wrote:
> > > > Added warnings in checkpatch.pl script to :
> > > > 
> > > > 1. Deprecate strcpy() in favor of strscpy().
> > > > 2. Deprecate strlcpy() in favor of strscpy().
> > > > 3. Deprecate strncpy() in favor of strscpy() or strscpy_pad().
> > > > 
> > > > Updated strncpy() section in Documentation/process/deprecated.rst
> > > > to cover strscpy_pad() case.
> > 
> > []
> > 
> > I sent a patch series for some strscpy/strlcpy misuses.
> > 
> > How about adding a macro helper to avoid the misuses like:
> > ---
> >  include/linux/string.h | 16 ++++++++++++++++
> >  1 file changed, 16 insertions(+)
> > 
> > diff --git a/include/linux/string.h b/include/linux/string.h
> > index 4deb11f7976b..ef01bd6f19df 100644
> > --- a/include/linux/string.h
> > +++ b/include/linux/string.h
> > @@ -35,6 +35,22 @@ ssize_t strscpy(char *, const char *, size_t);
> >  /* Wraps calls to strscpy()/memset(), no arch specific code required */
> >  ssize_t strscpy_pad(char *dest, const char *src, size_t count);
> >  
> > +#define stracpy(to, from)					\
> > +({								\
> > +	size_t size = ARRAY_SIZE(to);				\
> > +	BUILD_BUG_ON(!__same_type(typeof(*to), char));		\
> > +								\
> > +	strscpy(to, from, size);				\
> > +})
> > +
> > +#define stracpy_pad(to, from)					\
> > +({								\
> > +	size_t size = ARRAY_SIZE(to);				\
> > +	BUILD_BUG_ON(!__same_type(typeof(*to), char));		\
> > +								\
> > +	strscpy_pad(to, from, size);				\
> > +})
> > +
> >  #ifndef __HAVE_ARCH_STRCAT
> >  extern char * strcat(char *, const char *);
> >  #endif
> 
> This seems like a reasonable addition, yes. I think Coccinelle might
> actually be able to find all the existing strscpy(dst, src, sizeof(dst))
> cases to jump-start this conversion.

I did that.  It works.  It's a lot of conversions.

$ cat str.cpy.cocci
@@
expression e1;
expression e2;
@@

- strscpy(e1, e2, sizeof(e1))
+ stracpy(e1, e2)

@@
expression e1;
expression e2;
@@

- strlcpy(e1, e2, sizeof(e1))
+ stracpy(e1, e2)

> Devil's advocate: this adds yet more string handling functions... will
> this cause even more confusion?

Documentation is good.
Actual in-kernel use and examples better.




^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [RFC PATCH] string.h: Add stracpy/stracpy_pad (was: Re: [PATCH] checkpatch: Added warnings in favor of strscpy().)
@ 2019-07-22 17:43         ` Joe Perches
  0 siblings, 0 replies; 14+ messages in thread
From: Joe Perches @ 2019-07-22 17:43 UTC (permalink / raw)
  To: Kees Cook
  Cc: Nitin Gote, akpm, corbet, apw, linux-doc, linux-kernel,
	kernel-hardening, Rasmus Villemoes

On Mon, 2019-07-22 at 10:33 -0700, Kees Cook wrote:
> On Thu, Jul 04, 2019 at 05:15:57PM -0700, Joe Perches wrote:
> > On Thu, 2019-07-04 at 13:46 -0700, Joe Perches wrote:
> > > On Thu, 2019-07-04 at 11:24 +0530, Nitin Gote wrote:
> > > > Added warnings in checkpatch.pl script to :
> > > > 
> > > > 1. Deprecate strcpy() in favor of strscpy().
> > > > 2. Deprecate strlcpy() in favor of strscpy().
> > > > 3. Deprecate strncpy() in favor of strscpy() or strscpy_pad().
> > > > 
> > > > Updated strncpy() section in Documentation/process/deprecated.rst
> > > > to cover strscpy_pad() case.
> > 
> > []
> > 
> > I sent a patch series for some strscpy/strlcpy misuses.
> > 
> > How about adding a macro helper to avoid the misuses like:
> > ---
> >  include/linux/string.h | 16 ++++++++++++++++
> >  1 file changed, 16 insertions(+)
> > 
> > diff --git a/include/linux/string.h b/include/linux/string.h
> > index 4deb11f7976b..ef01bd6f19df 100644
> > --- a/include/linux/string.h
> > +++ b/include/linux/string.h
> > @@ -35,6 +35,22 @@ ssize_t strscpy(char *, const char *, size_t);
> >  /* Wraps calls to strscpy()/memset(), no arch specific code required */
> >  ssize_t strscpy_pad(char *dest, const char *src, size_t count);
> >  
> > +#define stracpy(to, from)					\
> > +({								\
> > +	size_t size = ARRAY_SIZE(to);				\
> > +	BUILD_BUG_ON(!__same_type(typeof(*to), char));		\
> > +								\
> > +	strscpy(to, from, size);				\
> > +})
> > +
> > +#define stracpy_pad(to, from)					\
> > +({								\
> > +	size_t size = ARRAY_SIZE(to);				\
> > +	BUILD_BUG_ON(!__same_type(typeof(*to), char));		\
> > +								\
> > +	strscpy_pad(to, from, size);				\
> > +})
> > +
> >  #ifndef __HAVE_ARCH_STRCAT
> >  extern char * strcat(char *, const char *);
> >  #endif
> 
> This seems like a reasonable addition, yes. I think Coccinelle might
> actually be able to find all the existing strscpy(dst, src, sizeof(dst))
> cases to jump-start this conversion.

I did that.  It works.  It's a lot of conversions.

$ cat str.cpy.cocci
@@
expression e1;
expression e2;
@@

- strscpy(e1, e2, sizeof(e1))
+ stracpy(e1, e2)

@@
expression e1;
expression e2;
@@

- strlcpy(e1, e2, sizeof(e1))
+ stracpy(e1, e2)

> Devil's advocate: this adds yet more string handling functions... will
> this cause even more confusion?

Documentation is good.
Actual in-kernel use and examples better.




^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [RFC PATCH] string.h: Add stracpy/stracpy_pad (was: Re: [PATCH] checkpatch: Added warnings in favor of strscpy().)
  2019-07-22 17:43         ` Joe Perches
@ 2019-07-22 17:58           ` Joe Perches
  -1 siblings, 0 replies; 14+ messages in thread
From: Joe Perches @ 2019-07-22 17:58 UTC (permalink / raw)
  To: Kees Cook
  Cc: Nitin Gote, akpm, corbet, apw, linux-doc, linux-kernel,
	kernel-hardening, Rasmus Villemoes

On Mon, 2019-07-22 at 10:43 -0700, Joe Perches wrote:
> On Mon, 2019-07-22 at 10:33 -0700, Kees Cook wrote:
> > On Thu, Jul 04, 2019 at 05:15:57PM -0700, Joe Perches wrote:
> > > On Thu, 2019-07-04 at 13:46 -0700, Joe Perches wrote:
> > > > On Thu, 2019-07-04 at 11:24 +0530, Nitin Gote wrote:
> > > > > Added warnings in checkpatch.pl script to :
> > > > > 
> > > > > 1. Deprecate strcpy() in favor of strscpy().
> > > > > 2. Deprecate strlcpy() in favor of strscpy().
> > > > > 3. Deprecate strncpy() in favor of strscpy() or strscpy_pad().
> > > > > 
> > > > > Updated strncpy() section in Documentation/process/deprecated.rst
> > > > > to cover strscpy_pad() case.
> > > 
> > > []
> > > 
> > > I sent a patch series for some strscpy/strlcpy misuses.
> > > 
> > > How about adding a macro helper to avoid the misuses like:
> > > ---
> > >  include/linux/string.h | 16 ++++++++++++++++
> > >  1 file changed, 16 insertions(+)
> > > 
> > > diff --git a/include/linux/string.h b/include/linux/string.h
> > > index 4deb11f7976b..ef01bd6f19df 100644
> > > --- a/include/linux/string.h
> > > +++ b/include/linux/string.h
> > > @@ -35,6 +35,22 @@ ssize_t strscpy(char *, const char *, size_t);
> > >  /* Wraps calls to strscpy()/memset(), no arch specific code required */
> > >  ssize_t strscpy_pad(char *dest, const char *src, size_t count);
> > >  
> > > +#define stracpy(to, from)					\
> > > +({								\
> > > +	size_t size = ARRAY_SIZE(to);				\
> > > +	BUILD_BUG_ON(!__same_type(typeof(*to), char));		\
> > > +								\
> > > +	strscpy(to, from, size);				\
> > > +})
> > > +
> > > +#define stracpy_pad(to, from)					\
> > > +({								\
> > > +	size_t size = ARRAY_SIZE(to);				\
> > > +	BUILD_BUG_ON(!__same_type(typeof(*to), char));		\
> > > +								\
> > > +	strscpy_pad(to, from, size);				\
> > > +})
> > > +
> > >  #ifndef __HAVE_ARCH_STRCAT
> > >  extern char * strcat(char *, const char *);
> > >  #endif
> > 
> > This seems like a reasonable addition, yes. I think Coccinelle might
> > actually be able to find all the existing strscpy(dst, src, sizeof(dst))
> > cases to jump-start this conversion.
> 
> I did that.  It works.  It's a lot of conversions.
> 
> $ cat str.cpy.cocci
> @@
> expression e1;
> expression e2;
> @@
> 
> - strscpy(e1, e2, sizeof(e1))
> + stracpy(e1, e2)
> 
> @@
> expression e1;
> expression e2;
> @@
> 
> - strlcpy(e1, e2, sizeof(e1))
> + stracpy(e1, e2)
> 
> > Devil's advocate: this adds yet more string handling functions... will
> > this cause even more confusion?
> 
> Documentation is good.
> Actual in-kernel use and examples better.

btw: I just ran this again and it produces:

$ spatch --in-place -sp-file str.cpy.cocci .
$ git checkout tools/
$ git diff --shortstat
 958 files changed, 2179 insertions(+), 2655 deletions(-)



^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [RFC PATCH] string.h: Add stracpy/stracpy_pad (was: Re: [PATCH] checkpatch: Added warnings in favor of strscpy().)
@ 2019-07-22 17:58           ` Joe Perches
  0 siblings, 0 replies; 14+ messages in thread
From: Joe Perches @ 2019-07-22 17:58 UTC (permalink / raw)
  To: Kees Cook
  Cc: Nitin Gote, akpm, corbet, apw, linux-doc, linux-kernel,
	kernel-hardening, Rasmus Villemoes

On Mon, 2019-07-22 at 10:43 -0700, Joe Perches wrote:
> On Mon, 2019-07-22 at 10:33 -0700, Kees Cook wrote:
> > On Thu, Jul 04, 2019 at 05:15:57PM -0700, Joe Perches wrote:
> > > On Thu, 2019-07-04 at 13:46 -0700, Joe Perches wrote:
> > > > On Thu, 2019-07-04 at 11:24 +0530, Nitin Gote wrote:
> > > > > Added warnings in checkpatch.pl script to :
> > > > > 
> > > > > 1. Deprecate strcpy() in favor of strscpy().
> > > > > 2. Deprecate strlcpy() in favor of strscpy().
> > > > > 3. Deprecate strncpy() in favor of strscpy() or strscpy_pad().
> > > > > 
> > > > > Updated strncpy() section in Documentation/process/deprecated.rst
> > > > > to cover strscpy_pad() case.
> > > 
> > > []
> > > 
> > > I sent a patch series for some strscpy/strlcpy misuses.
> > > 
> > > How about adding a macro helper to avoid the misuses like:
> > > ---
> > >  include/linux/string.h | 16 ++++++++++++++++
> > >  1 file changed, 16 insertions(+)
> > > 
> > > diff --git a/include/linux/string.h b/include/linux/string.h
> > > index 4deb11f7976b..ef01bd6f19df 100644
> > > --- a/include/linux/string.h
> > > +++ b/include/linux/string.h
> > > @@ -35,6 +35,22 @@ ssize_t strscpy(char *, const char *, size_t);
> > >  /* Wraps calls to strscpy()/memset(), no arch specific code required */
> > >  ssize_t strscpy_pad(char *dest, const char *src, size_t count);
> > >  
> > > +#define stracpy(to, from)					\
> > > +({								\
> > > +	size_t size = ARRAY_SIZE(to);				\
> > > +	BUILD_BUG_ON(!__same_type(typeof(*to), char));		\
> > > +								\
> > > +	strscpy(to, from, size);				\
> > > +})
> > > +
> > > +#define stracpy_pad(to, from)					\
> > > +({								\
> > > +	size_t size = ARRAY_SIZE(to);				\
> > > +	BUILD_BUG_ON(!__same_type(typeof(*to), char));		\
> > > +								\
> > > +	strscpy_pad(to, from, size);				\
> > > +})
> > > +
> > >  #ifndef __HAVE_ARCH_STRCAT
> > >  extern char * strcat(char *, const char *);
> > >  #endif
> > 
> > This seems like a reasonable addition, yes. I think Coccinelle might
> > actually be able to find all the existing strscpy(dst, src, sizeof(dst))
> > cases to jump-start this conversion.
> 
> I did that.  It works.  It's a lot of conversions.
> 
> $ cat str.cpy.cocci
> @@
> expression e1;
> expression e2;
> @@
> 
> - strscpy(e1, e2, sizeof(e1))
> + stracpy(e1, e2)
> 
> @@
> expression e1;
> expression e2;
> @@
> 
> - strlcpy(e1, e2, sizeof(e1))
> + stracpy(e1, e2)
> 
> > Devil's advocate: this adds yet more string handling functions... will
> > this cause even more confusion?
> 
> Documentation is good.
> Actual in-kernel use and examples better.

btw: I just ran this again and it produces:

$ spatch --in-place -sp-file str.cpy.cocci .
$ git checkout tools/
$ git diff --shortstat
 958 files changed, 2179 insertions(+), 2655 deletions(-)



^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [RFC PATCH] string.h: Add stracpy/stracpy_pad (was: Re: [PATCH] checkpatch: Added warnings in favor of strscpy().)
  2019-07-22 17:58           ` Joe Perches
  (?)
@ 2019-07-22 18:21           ` Kees Cook
  -1 siblings, 0 replies; 14+ messages in thread
From: Kees Cook @ 2019-07-22 18:21 UTC (permalink / raw)
  To: Joe Perches
  Cc: Nitin Gote, akpm, corbet, apw, linux-doc, linux-kernel,
	kernel-hardening, Rasmus Villemoes

On Mon, Jul 22, 2019 at 10:58:15AM -0700, Joe Perches wrote:
> On Mon, 2019-07-22 at 10:43 -0700, Joe Perches wrote:
> > On Mon, 2019-07-22 at 10:33 -0700, Kees Cook wrote:
> > > On Thu, Jul 04, 2019 at 05:15:57PM -0700, Joe Perches wrote:
> > > > On Thu, 2019-07-04 at 13:46 -0700, Joe Perches wrote:
> > > > > On Thu, 2019-07-04 at 11:24 +0530, Nitin Gote wrote:
> > > > > > Added warnings in checkpatch.pl script to :
> > > > > > 
> > > > > > 1. Deprecate strcpy() in favor of strscpy().
> > > > > > 2. Deprecate strlcpy() in favor of strscpy().
> > > > > > 3. Deprecate strncpy() in favor of strscpy() or strscpy_pad().
> > > > > > 
> > > > > > Updated strncpy() section in Documentation/process/deprecated.rst
> > > > > > to cover strscpy_pad() case.
> > > > 
> > > > []
> > > > 
> > > > I sent a patch series for some strscpy/strlcpy misuses.
> > > > 
> > > > How about adding a macro helper to avoid the misuses like:
> > > > ---
> > > >  include/linux/string.h | 16 ++++++++++++++++
> > > >  1 file changed, 16 insertions(+)
> > > > 
> > > > diff --git a/include/linux/string.h b/include/linux/string.h
> > > > index 4deb11f7976b..ef01bd6f19df 100644
> > > > --- a/include/linux/string.h
> > > > +++ b/include/linux/string.h
> > > > @@ -35,6 +35,22 @@ ssize_t strscpy(char *, const char *, size_t);
> > > >  /* Wraps calls to strscpy()/memset(), no arch specific code required */
> > > >  ssize_t strscpy_pad(char *dest, const char *src, size_t count);
> > > >  
> > > > +#define stracpy(to, from)					\
> > > > +({								\
> > > > +	size_t size = ARRAY_SIZE(to);				\
> > > > +	BUILD_BUG_ON(!__same_type(typeof(*to), char));		\
> > > > +								\
> > > > +	strscpy(to, from, size);				\
> > > > +})
> > > > +
> > > > +#define stracpy_pad(to, from)					\
> > > > +({								\
> > > > +	size_t size = ARRAY_SIZE(to);				\
> > > > +	BUILD_BUG_ON(!__same_type(typeof(*to), char));		\
> > > > +								\
> > > > +	strscpy_pad(to, from, size);				\
> > > > +})
> > > > +
> > > >  #ifndef __HAVE_ARCH_STRCAT
> > > >  extern char * strcat(char *, const char *);
> > > >  #endif
> > > 
> > > This seems like a reasonable addition, yes. I think Coccinelle might
> > > actually be able to find all the existing strscpy(dst, src, sizeof(dst))
> > > cases to jump-start this conversion.
> > 
> > I did that.  It works.  It's a lot of conversions.
> > 
> > $ cat str.cpy.cocci
> > @@
> > expression e1;
> > expression e2;
> > @@
> > 
> > - strscpy(e1, e2, sizeof(e1))
> > + stracpy(e1, e2)
> > 
> > @@
> > expression e1;
> > expression e2;
> > @@
> > 
> > - strlcpy(e1, e2, sizeof(e1))
> > + stracpy(e1, e2)
> > 
> > > Devil's advocate: this adds yet more string handling functions... will
> > > this cause even more confusion?
> > 
> > Documentation is good.
> > Actual in-kernel use and examples better.
> 
> btw: I just ran this again and it produces:
> 
> $ spatch --in-place -sp-file str.cpy.cocci .
> $ git checkout tools/
> $ git diff --shortstat
>  958 files changed, 2179 insertions(+), 2655 deletions(-)

Cool. Well, assuming no one hates this, let's do it. :) Can you send a
more complete patch with docs, etc? Maybe Linus will take it for late
in the next merge window, perhaps?

-- 
Kees Cook

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [RFC PATCH] string.h: Add stracpy/stracpy_pad (was: Re: [PATCH] checkpatch: Added warnings in favor of strscpy().)
  2019-07-22 17:58           ` Joe Perches
  (?)
  (?)
@ 2019-07-22 18:27           ` Matthew Wilcox
  2019-07-22 18:35               ` Joe Perches
  -1 siblings, 1 reply; 14+ messages in thread
From: Matthew Wilcox @ 2019-07-22 18:27 UTC (permalink / raw)
  To: Joe Perches
  Cc: Kees Cook, Nitin Gote, akpm, corbet, apw, linux-doc,
	linux-kernel, kernel-hardening, Rasmus Villemoes

On Mon, Jul 22, 2019 at 10:58:15AM -0700, Joe Perches wrote:
> On Mon, 2019-07-22 at 10:43 -0700, Joe Perches wrote:
> > On Mon, 2019-07-22 at 10:33 -0700, Kees Cook wrote:
> > > On Thu, Jul 04, 2019 at 05:15:57PM -0700, Joe Perches wrote:
> > > > On Thu, 2019-07-04 at 13:46 -0700, Joe Perches wrote:
> > > > > On Thu, 2019-07-04 at 11:24 +0530, Nitin Gote wrote:
> > > > > > Added warnings in checkpatch.pl script to :
> > > > > > 
> > > > > > 1. Deprecate strcpy() in favor of strscpy().
> > > > > > 2. Deprecate strlcpy() in favor of strscpy().
> > > > > > 3. Deprecate strncpy() in favor of strscpy() or strscpy_pad().
> > > > > > 
> > > > > > Updated strncpy() section in Documentation/process/deprecated.rst
> > > > > > to cover strscpy_pad() case.
> > > > 
> > > > []
> > > > 
> > > > I sent a patch series for some strscpy/strlcpy misuses.
> > > > 
> > > > How about adding a macro helper to avoid the misuses like:
> > > > ---
> > > >  include/linux/string.h | 16 ++++++++++++++++
> > > >  1 file changed, 16 insertions(+)
> > > > 
> > > > diff --git a/include/linux/string.h b/include/linux/string.h
> > > > index 4deb11f7976b..ef01bd6f19df 100644
> > > > --- a/include/linux/string.h
> > > > +++ b/include/linux/string.h
> > > > @@ -35,6 +35,22 @@ ssize_t strscpy(char *, const char *, size_t);
> > > >  /* Wraps calls to strscpy()/memset(), no arch specific code required */
> > > >  ssize_t strscpy_pad(char *dest, const char *src, size_t count);
> > > >  
> > > > +#define stracpy(to, from)					\
> > > > +({								\
> > > > +	size_t size = ARRAY_SIZE(to);				\
> > > > +	BUILD_BUG_ON(!__same_type(typeof(*to), char));		\
> > > > +								\
> > > > +	strscpy(to, from, size);				\
> > > > +})

Where does the 'a' in 'stracpy' come from?  Googling around finds other
people using a function called stracpy, but it takes different arguments.
http://stracpy.blogspot.com/ takes a size argument, as does
https://docs.polserver.com/doxygen/html/d5/dce/stracpy_8cpp_source.html

The one in the 'Links' webbrowser (can't find a link to its source) seems
like a strdup clone.


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [RFC PATCH] string.h: Add stracpy/stracpy_pad (was: Re: [PATCH] checkpatch: Added warnings in favor of strscpy().)
  2019-07-22 18:27           ` Matthew Wilcox
@ 2019-07-22 18:35               ` Joe Perches
  0 siblings, 0 replies; 14+ messages in thread
From: Joe Perches @ 2019-07-22 18:35 UTC (permalink / raw)
  To: Matthew Wilcox
  Cc: Kees Cook, Nitin Gote, akpm, corbet, apw, linux-doc,
	linux-kernel, kernel-hardening, Rasmus Villemoes

On Mon, 2019-07-22 at 11:27 -0700, Matthew Wilcox wrote:
> On Mon, Jul 22, 2019 at 10:58:15AM -0700, Joe Perches wrote:
> > On Mon, 2019-07-22 at 10:43 -0700, Joe Perches wrote:
> > > On Mon, 2019-07-22 at 10:33 -0700, Kees Cook wrote:
> > > > On Thu, Jul 04, 2019 at 05:15:57PM -0700, Joe Perches wrote:
> > > > > On Thu, 2019-07-04 at 13:46 -0700, Joe Perches wrote:
[]
> > > > > +#define stracpy(to, from)					\
> > > > > +({								\
> > > > > +	size_t size = ARRAY_SIZE(to);				\
> > > > > +	BUILD_BUG_ON(!__same_type(typeof(*to), char));		\
> > > > > +								\
> > > > > +	strscpy(to, from, size);				\
> > > > > +})
> 
> Where does the 'a' in 'stracpy' come from?

No place in particular.

I used it because dst has to be an 'a'rray rather
than a pointer.



^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [RFC PATCH] string.h: Add stracpy/stracpy_pad (was: Re: [PATCH] checkpatch: Added warnings in favor of strscpy().)
@ 2019-07-22 18:35               ` Joe Perches
  0 siblings, 0 replies; 14+ messages in thread
From: Joe Perches @ 2019-07-22 18:35 UTC (permalink / raw)
  To: Matthew Wilcox
  Cc: Kees Cook, Nitin Gote, akpm, corbet, apw, linux-doc,
	linux-kernel, kernel-hardening, Rasmus Villemoes

On Mon, 2019-07-22 at 11:27 -0700, Matthew Wilcox wrote:
> On Mon, Jul 22, 2019 at 10:58:15AM -0700, Joe Perches wrote:
> > On Mon, 2019-07-22 at 10:43 -0700, Joe Perches wrote:
> > > On Mon, 2019-07-22 at 10:33 -0700, Kees Cook wrote:
> > > > On Thu, Jul 04, 2019 at 05:15:57PM -0700, Joe Perches wrote:
> > > > > On Thu, 2019-07-04 at 13:46 -0700, Joe Perches wrote:
[]
> > > > > +#define stracpy(to, from)					\
> > > > > +({								\
> > > > > +	size_t size = ARRAY_SIZE(to);				\
> > > > > +	BUILD_BUG_ON(!__same_type(typeof(*to), char));		\
> > > > > +								\
> > > > > +	strscpy(to, from, size);				\
> > > > > +})
> 
> Where does the 'a' in 'stracpy' come from?

No place in particular.

I used it because dst has to be an 'a'rray rather
than a pointer.



^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2019-07-22 18:35 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-07-04  5:54 [PATCH] checkpatch: Added warnings in favor of strscpy() Nitin Gote
2019-07-04 20:46 ` Joe Perches
2019-07-04 20:46   ` Joe Perches
2019-07-05  0:15   ` [RFC PATCH] string.h: Add stracpy/stracpy_pad (was: Re: [PATCH] checkpatch: Added warnings in favor of strscpy().) Joe Perches
2019-07-05  0:15     ` Joe Perches
2019-07-22 17:33     ` Kees Cook
2019-07-22 17:43       ` Joe Perches
2019-07-22 17:43         ` Joe Perches
2019-07-22 17:58         ` Joe Perches
2019-07-22 17:58           ` Joe Perches
2019-07-22 18:21           ` Kees Cook
2019-07-22 18:27           ` Matthew Wilcox
2019-07-22 18:35             ` Joe Perches
2019-07-22 18:35               ` Joe Perches

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.