[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Mon, 1 Jul 2019 08:42:39 +0000
From: "Gote, Nitin R" <nitin.r.gote@...el.com>
To: Kees Cook <keescook@...omium.org>
CC: "jannh@...gle.com" <jannh@...gle.com>,
"kernel-hardening@...ts.openwall.com" <kernel-hardening@...ts.openwall.com>
Subject: RE: [PATCH] checkpatch: Added warnings in favor of strscpy().
Hi Kees,
As per my understanding, I have updated strncpy() section in Documentation/process/deprecated.rst for strscpy_pad() case. Other two cases of strncpy() are already explained.
Also updated checkpatch for __nonstring case.
Could you please give your inputs on below diff changes ? If this looks good, I will send the patch.
Diff changes :
diff --git a/Documentation/process/deprecated.rst b/Documentation/process/deprecated.rst
index 49e0f64..6ab05ac 100644
--- a/Documentation/process/deprecated.rst
+++ b/Documentation/process/deprecated.rst
@@ -102,6 +102,9 @@ still be used, but destinations should be marked with the `__nonstring
<https://gcc.gnu.org/onlinedocs/gcc/Common-Variable-Attributes.html>`_
attribute to avoid future compiler warnings.
+If a caller is using NUL-terminated strings, and destination needing
+trailing NUL, then the safe replace is :c:func:`strscpy_pad()`.
+
strlcpy()
---------
:c:func:`strlcpy` reads the entire source buffer first, possibly exceeding
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 342c7c7..d3c0587 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -595,6 +595,10 @@ 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",
);
Thanks and Regards,
Nitin Gote
-----Original Message-----
From: Kees Cook [mailto:keescook@...omium.org]
Sent: Friday, June 28, 2019 8:16 PM
To: Gote, Nitin R <nitin.r.gote@...el.com>
Cc: jannh@...gle.com; kernel-hardening@...ts.openwall.com
Subject: Re: [PATCH] checkpatch: Added warnings in favor of strscpy().
On Fri, Jun 28, 2019 at 05:25:48PM +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().
>
> Signed-off-by: Nitin Gote <nitin.r.gote@...el.com>
Excellent, yes. Can you also add a bit to the strncpy() section in Documentation/process/deprecated.rst so that all three cases of strncpy() are explained:
- strncpy() into NUL-terminated target should use strscpy()
- strncpy() into NUL-terminated target needing trailing NUL: strscpy_pad()
- strncpy() into non-NUL-terminated target should have target marked
with __nonstring.
(and probably mention the __nonstring case in checkpatch too)
-Kees
> ---
> scripts/checkpatch.pl | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index
> 342c7c7..bb0fa11 100755
> --- a/scripts/checkpatch.pl
> +++ b/scripts/checkpatch.pl
> @@ -595,6 +595,9 @@ 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 or strscpy_pad",
> );
>
> #Create a search pattern for all these strings to speed up a loop
> below
> --
> 2.7.4
>
--
Kees Cook
Powered by blists - more mailing lists
Confused about mailing lists and their use?
Read about mailing lists on Wikipedia
and check out these
guidelines on proper formatting of your messages.
