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.

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>

diff --git a/Documentation/accounting/getdelays.c b/Documentation/accounting/getdelays.c
index f8ebcde43b174640ae1ab1d8ac47426a10688614..5e4773dc3e5ff8cc25e3221f13cda0deffc4214b 100644 (file)
--- a/Documentation/accounting/getdelays.c
+++ b/Documentation/accounting/getdelays.c
@@ -23,6 +23,7 @@
 #include <sys/socket.h>
 #include <sys/wait.h>
 #include <signal.h>
+#include <bsd/string.h>
 
 #include <linux/genetlink.h>
 #include <linux/taskstats.h>
@@ -299,7 +300,7 @@ int main(int argc, char *argv[])
                        break;
                case 'C':
                        containerset = 1;
-                       strncpy(containerpath, optarg, strlen(optarg) + 1);
+                       strlcpy(containerpath, optarg, sizeof(containerpath));
                        break;
                case 'w':
                        logfile = strdup(optarg);

diff --git a/drivers/acpi/sysfs.c b/drivers/acpi/sysfs.c
index 5c5d1624fa2c261c8b6dca89a34156268a91d799..05306a59aedc18b45dd89f636d907def41d67d11 100644 (file)
--- a/drivers/acpi/sysfs.c
+++ b/drivers/acpi/sysfs.c
@@ -677,10 +677,9 @@ void acpi_irq_stats_init(void)
                else
                        sprintf(buffer, "bug%02X", i);
 
-               name = kzalloc(strlen(buffer) + 1, GFP_KERNEL);
+               name = kstrdup(buffer, GFP_KERNEL);
                if (name == NULL)
                        goto fail;
-               strncpy(name, buffer, strlen(buffer) + 1);
 
                sysfs_attr_init(&counter_attrs[i].attr);
                counter_attrs[i].attr.name = name;

diff --git a/drivers/s390/net/qeth_l3_sys.c b/drivers/s390/net/qeth_l3_sys.c
index e70af2406ff9da388828660bb0bed2b73bf5cb71..d1c8025b0b037605c73f331b6ebd3178feed2767 100644 (file)
--- a/drivers/s390/net/qeth_l3_sys.c
+++ b/drivers/s390/net/qeth_l3_sys.c
@@ -315,10 +315,8 @@ static ssize_t qeth_l3_dev_hsuid_store(struct device *dev,
        if (qeth_configure_cq(card, QETH_CQ_ENABLED))
                return -EPERM;
 
-       for (i = 0; i < 8; i++)
-               card->options.hsuid[i] = ' ';
-       card->options.hsuid[8] = '\0';
-       strncpy(card->options.hsuid, tmp, strlen(tmp));
+       snprintf(card->options.hsuid, sizeof(card->options.hsuid),
+                "%-8s", tmp);
        ASCEBC(card->options.hsuid, 8);
        if (card->dev)
                memcpy(card->dev->perm_addr, card->options.hsuid, 9);

diff --git a/drivers/staging/tidspbridge/rmgr/drv_interface.c b/drivers/staging/tidspbridge/rmgr/drv_interface.c
index 9c020562c84637ca85d8d9e5ffe768c8b7809440..6d04eb48bfbce9362862d47e28b6d3d1a0b84be0 100644 (file)
--- a/drivers/staging/tidspbridge/rmgr/drv_interface.c
+++ b/drivers/staging/tidspbridge/rmgr/drv_interface.c
@@ -421,12 +421,11 @@ static int omap3_bridge_startup(struct platform_device *pdev)
        drv_datap->tc_wordswapon = tc_wordswapon;
 
        if (base_img) {
-               drv_datap->base_img = kmalloc(strlen(base_img) + 1, GFP_KERNEL);
+               drv_datap->base_img = kstrdup(base_img, GFP_KERNEL);
                if (!drv_datap->base_img) {
                        err = -ENOMEM;
                        goto err2;
                }
-               strncpy(drv_datap->base_img, base_img, strlen(base_img) + 1);
        }
 
        dev_set_drvdata(bridge, drv_datap);

diff --git a/fs/hppfs/hppfs.c b/fs/hppfs/hppfs.c
index fc90ab11c34049e6431e0b9dcf220040cfbbb146..4338ff32959d4e9e3414bd9ed184d4d47b5a53fe 100644 (file)
--- a/fs/hppfs/hppfs.c
+++ b/fs/hppfs/hppfs.c
@@ -69,7 +69,7 @@ static char *dentry_name(struct dentry *dentry, int extra)
        struct dentry *parent;
        char *root, *name;
        const char *seg_name;
-       int len, seg_len;
+       int len, seg_len, root_len;
 
        len = 0;
        parent = dentry;
@@ -81,7 +81,8 @@ static char *dentry_name(struct dentry *dentry, int extra)
        }
 
        root = "proc";
-       len += strlen(root);
+       root_len = strlen(root);
+       len += root_len;
        name = kmalloc(len + extra + 1, GFP_KERNEL);
        if (name == NULL)
                return NULL;
@@ -91,7 +92,7 @@ static char *dentry_name(struct dentry *dentry, int extra)
        while (parent->d_parent != parent) {
                if (is_pid(parent)) {
                        seg_name = "pid";
-                       seg_len = strlen("pid");
+                       seg_len = strlen(seg_name);
                }
                else {
                        seg_name = parent->d_name.name;
@@ -100,10 +101,10 @@ static char *dentry_name(struct dentry *dentry, int extra)
 
                len -= seg_len + 1;
                name[len] = '/';
-               strncpy(&name[len + 1], seg_name, seg_len);
+               memcpy(&name[len + 1], seg_name, seg_len);
                parent = parent->d_parent;
        }
-       strncpy(name, root, strlen(root));
+       memcpy(name, root, root_len);
        return name;
 }