Commit 096a8aac6bf4a5a0b2ef812ad76d056bbf3fb2af

Authored by Kees Cook
Committed by Linus Torvalds
1 parent e7152b97f3

clean up scary strncpy(dst, src, strlen(src)) uses

Fix various weird constructions of strncpy(dst, src, strlen(src)).

Length limits should be about the space available in the destination,
not repurposed as a method to either always include or always exclude a
trailing NULL byte.  Either the NULL should always be copied (using
strlcpy), or it should not be copied (using something like memcpy).
Readable code should not depend on the weird behavior of strncpy when it
hits the length limit.  Better to avoid the anti-pattern entirely.

[akpm@linux-foundation.org: revert getdelays.c part due to missing bsd/string.h]
Signed-off-by: Kees Cook <keescook@chromium.org>
Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>	[staging]
Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>	[acpi]
Cc: Martin Schwidefsky <schwidefsky@de.ibm.com>
Cc: Heiko Carstens <heiko.carstens@de.ibm.com>
Cc: Ursula Braun <ursula.braun@de.ibm.com>
Cc: Frank Blaschka <blaschka@linux.vnet.ibm.com>
Cc: Richard Weinberger <richard@nod.at>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>

Showing 4 changed files with 10 additions and 13 deletions Side-by-side Diff

drivers/acpi/sysfs.c
... ... @@ -677,10 +677,9 @@
677 677 else
678 678 sprintf(buffer, "bug%02X", i);
679 679  
680   - name = kzalloc(strlen(buffer) + 1, GFP_KERNEL);
  680 + name = kstrdup(buffer, GFP_KERNEL);
681 681 if (name == NULL)
682 682 goto fail;
683   - strncpy(name, buffer, strlen(buffer) + 1);
684 683  
685 684 sysfs_attr_init(&counter_attrs[i].attr);
686 685 counter_attrs[i].attr.name = name;
drivers/s390/net/qeth_l3_sys.c
... ... @@ -315,10 +315,8 @@
315 315 if (qeth_configure_cq(card, QETH_CQ_ENABLED))
316 316 return -EPERM;
317 317  
318   - for (i = 0; i < 8; i++)
319   - card->options.hsuid[i] = ' ';
320   - card->options.hsuid[8] = '\0';
321   - strncpy(card->options.hsuid, tmp, strlen(tmp));
  318 + snprintf(card->options.hsuid, sizeof(card->options.hsuid),
  319 + "%-8s", tmp);
322 320 ASCEBC(card->options.hsuid, 8);
323 321 if (card->dev)
324 322 memcpy(card->dev->perm_addr, card->options.hsuid, 9);
drivers/staging/tidspbridge/rmgr/drv_interface.c
... ... @@ -421,12 +421,11 @@
421 421 drv_datap->tc_wordswapon = tc_wordswapon;
422 422  
423 423 if (base_img) {
424   - drv_datap->base_img = kmalloc(strlen(base_img) + 1, GFP_KERNEL);
  424 + drv_datap->base_img = kstrdup(base_img, GFP_KERNEL);
425 425 if (!drv_datap->base_img) {
426 426 err = -ENOMEM;
427 427 goto err2;
428 428 }
429   - strncpy(drv_datap->base_img, base_img, strlen(base_img) + 1);
430 429 }
431 430  
432 431 dev_set_drvdata(bridge, drv_datap);
... ... @@ -69,7 +69,7 @@
69 69 struct dentry *parent;
70 70 char *root, *name;
71 71 const char *seg_name;
72   - int len, seg_len;
  72 + int len, seg_len, root_len;
73 73  
74 74 len = 0;
75 75 parent = dentry;
... ... @@ -81,7 +81,8 @@
81 81 }
82 82  
83 83 root = "proc";
84   - len += strlen(root);
  84 + root_len = strlen(root);
  85 + len += root_len;
85 86 name = kmalloc(len + extra + 1, GFP_KERNEL);
86 87 if (name == NULL)
87 88 return NULL;
... ... @@ -91,7 +92,7 @@
91 92 while (parent->d_parent != parent) {
92 93 if (is_pid(parent)) {
93 94 seg_name = "pid";
94   - seg_len = strlen("pid");
  95 + seg_len = strlen(seg_name);
95 96 }
96 97 else {
97 98 seg_name = parent->d_name.name;
98 99  
... ... @@ -100,10 +101,10 @@
100 101  
101 102 len -= seg_len + 1;
102 103 name[len] = '/';
103   - strncpy(&name[len + 1], seg_name, seg_len);
  104 + memcpy(&name[len + 1], seg_name, seg_len);
104 105 parent = parent->d_parent;
105 106 }
106   - strncpy(name, root, strlen(root));
  107 + memcpy(name, root, root_len);
107 108 return name;
108 109 }
109 110