Chuhong Yuan [off-list ref] 于2019年7月30日周二 下午2:39写道：

Kees Cook [off-list ref] 于2019年7月30日周二 下午12:26写道：

quoted

On Mon, Jul 29, 2019 at 11:13:46PM +0800, Chuhong Yuan wrote:

quoted

strncmp(str, const, len) is error-prone.
We had better use newly introduced
str_has_prefix() instead of it.

Wait, stop. :) After Laura called my attention to your conversion series,
mpe pointed out that str_has_prefix() is almost redundant to strstarts()
(from 2009), and the latter has many more users. Let's fix strstarts()
match str_has_prefix()'s return behavior (all the existing callers are
doing boolean tests, so the change in return value won't matter), and
then we can continue with this replacement. (And add some documentation
to Documenation/process/deprecated.rst along with a checkpatch.pl test
maybe too?)

Thanks for your advice!
Does that mean replacing strstarts()'s implementation with
str_has_prefix()'s and then use strstarts() to substitute
strncmp?

I am not very clear about how to add the test into checkpatch.pl.
Should I write a check for this pattern or directly add strncmp into
deprecated_apis?

quoted

Actually I'd focus first on the actually broken cases first (sizeof()
without the "-1", etc):

$ git grep strncmp.*sizeof | grep -v -- '-' | wc -l
17

I expect the "copy/paste" changes could just be a Coccinelle script that
Linus could run to fix all the cases (and should be added to the kernel
source's list of Coccinelle scripts). Especially since the bulk of the
usage pattern are doing literals like this:

Actually I am using a Coccinelle script to detect the cases and
have found 800+ places of strncmp(str, const, len).
But the script still needs some improvement since it has false
negatives and only focuses on detecting, not replacement.
I can upload it after improvement.
In which form should I upload it? In a patch's description or put it
in coccinelle scripts?

quoted

arch/alpha/kernel/setup.c:   if (strncmp(p, "mem=", 4) == 0) {

$ git grep -E 'strncmp.*(sizeof|, *[0-9]*)' | wc -l
2565

And some cases are weirdly backwards:

tools/perf/util/callchain.c:  if (!strncmp(tok, "none", strlen(tok))) {

I find there are cases of this pattern are not wrong.
One example is kernel/irq/debugfs.c: if (!strncmp(buf, "trigger", size)) {

Thus I do not know whether I should include these cases in my script.

quoted

-Kees

I think with the help of Coccinelle script, all strncmp(str, const, len)
can be replaced and these problems will be eliminated. :)

Regards,
Chuhong

quoted

quoted

Signed-off-by: Chuhong Yuan <redacted>
---
 kernel/cgroup/rdma.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/cgroup/rdma.c b/kernel/cgroup/rdma.c
index ae042c347c64..fd12a227f8e4 100644
--- a/kernel/cgroup/rdma.c
+++ b/kernel/cgroup/rdma.c

@@ -379,7 +379,7 @@ static int parse_resource(char *c, int *intval)
                      return -EINVAL;
              return i;
      }
-     if (strncmp(value, RDMACG_MAX_STR, len) == 0) {
+     if (str_has_prefix(value, RDMACG_MAX_STR)) {
              *intval = S32_MAX;
              return i;
      }
--

2.20.1

--
Kees Cook