The following pull request was submitted through Github. It can be accessed and reviewed at: https://github.com/lxc/lxc/pull/2316
This e-mail was sent by the LXC bot, direct replies will not reach the author unless they happen to be subscribed to this list. === Description (from pull-request) === Signed-off-by: Christian Brauner <christian.brau...@ubuntu.com>
From e8d173a61f20317d07160a4daf88bc6bd0582f15 Mon Sep 17 00:00:00 2001 From: Christian Brauner <christian.brau...@ubuntu.com> Date: Fri, 11 May 2018 12:57:51 +0200 Subject: [PATCH 1/3] strlcpy: add strlcpy() implementation Signed-off-by: Christian Brauner <christian.brau...@ubuntu.com> --- configure.ac | 4 ++++ src/include/strlcpy.c | 61 +++++++++++++++++++++++++++++++++++++++++++++++++++ src/include/strlcpy.h | 29 ++++++++++++++++++++++++ src/lxc/Makefile.am | 8 +++++++ 4 files changed, 102 insertions(+) create mode 100644 src/include/strlcpy.c create mode 100644 src/include/strlcpy.h diff --git a/configure.ac b/configure.ac index 2467bb54d..91e51d324 100644 --- a/configure.ac +++ b/configure.ac @@ -620,6 +620,10 @@ AC_CHECK_FUNCS([prlimit64], AM_CONDITIONAL(HAVE_PRLIMIT64, true) AC_DEFINE(HAVE_PRLIMIT64,1,[Have prlimit64]), AM_CONDITIONAL(HAVE_PRLIMIT64, false)) +AC_CHECK_FUNCS([strlcpy], + AM_CONDITIONAL(HAVE_STRLCPY, true) + AC_DEFINE(HAVE_STRLCPY,1,[Have strlcpy]), + AM_CONDITIONAL(HAVE_STRLCPY, false)) # Check for some libraries AX_PTHREAD diff --git a/src/include/strlcpy.c b/src/include/strlcpy.c new file mode 100644 index 000000000..28e16b5c7 --- /dev/null +++ b/src/include/strlcpy.c @@ -0,0 +1,61 @@ +/* liblxcapi + * + * Copyright © 2017 Christian Brauner <christian.brau...@ubuntu.com>. + * Copyright © 2017 Canonical Ltd. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2, as + * published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License along + * with this program; if not, write to the Free Software Foundation, Inc., + * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. + * + * This function has been copied from musl. + */ + +#include <limits.h> +#include <stdint.h> +#include <string.h> + +#define ALIGN (sizeof(size_t) - 1) +#define ONES ((size_t)-1 / UCHAR_MAX) +#define HIGHS (ONES * (UCHAR_MAX / 2 + 1)) +#define HASZERO(x) (((x)-ONES) & ~(x)&HIGHS) + +size_t strlcpy(char *d, const char *s, size_t n) +{ + char *d0 = d; + size_t *wd; + const size_t *ws; + + if (!n--) + goto finish; + + if (((uintptr_t)s & ALIGN) == ((uintptr_t)d & ALIGN)) { + for (; ((uintptr_t)s & ALIGN) && n && (*d = *s); n--, s++, d++) + ; + if (n && *s) { + wd = (void *)d; + ws = (const void *)s; + for (; n >= sizeof(size_t) && !HASZERO(*ws); + n -= sizeof(size_t), ws++, wd++) + *wd = *ws; + d = (void *)wd; + s = (const void *)ws; + } + } + + for (; n && (*d = *s); n--, s++, d++) + ; + + *d = 0; + +finish: + return d - d0 + strlen(s); +} diff --git a/src/include/strlcpy.h b/src/include/strlcpy.h new file mode 100644 index 000000000..1d2729191 --- /dev/null +++ b/src/include/strlcpy.h @@ -0,0 +1,29 @@ +/* liblxcapi + * + * Copyright © 2017 Christian Brauner <christian.brau...@ubuntu.com>. + * Copyright © 2017 Canonical Ltd. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2, as + * published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License along + * with this program; if not, write to the Free Software Foundation, Inc., + * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. + * + * This function has been copied from musl. + */ + +#ifndef _STRLCPY_H +#define _STRLCPY_H + +#include <stdio.h> + +extern size_t strlcpy(char *, const char *, size_t); + +#endif diff --git a/src/lxc/Makefile.am b/src/lxc/Makefile.am index f894b7925..923c43cab 100644 --- a/src/lxc/Makefile.am +++ b/src/lxc/Makefile.am @@ -151,6 +151,10 @@ liblxc_la_SOURCES += ../include/getline.c ../include/getline.h endif endif +if !HAVE_STRLCPY +liblxc_la_SOURCES += ../include/strlcpy.c ../include/strlcpy.h +endif + AM_CFLAGS=-DLXCROOTFSMOUNT=\"$(LXCROOTFSMOUNT)\" \ -DLXCPATH=\"$(LXCPATH)\" \ -DLXC_GLOBAL_CONF=\"$(LXC_GLOBAL_CONF)\" \ @@ -300,6 +304,10 @@ init_lxc_static_SOURCES += ../include/getline.c endif endif +if !HAVE_STRLCPY +init_lxc_static_SOURCES += ../include/strlcpy.c ../include/strlcpy.h +endif + init_lxc_static_LDFLAGS = -all-static init_lxc_static_LDADD = @CAP_LIBS@ init_lxc_static_CFLAGS = $(AM_CFLAGS) -DNO_LXC_CONF From 8ef83215d354120c27434fd76db01351a57389cf Mon Sep 17 00:00:00 2001 From: Christian Brauner <christian.brau...@ubuntu.com> Date: Fri, 11 May 2018 12:58:11 +0200 Subject: [PATCH 2/3] tree-wide: s/strncpy()/strlcpy()/g Signed-off-by: Christian Brauner <christian.brau...@ubuntu.com> --- src/lxc/af_unix.c | 24 ++++++++++++++++++++---- src/lxc/criu.c | 9 +++++++-- src/lxc/log.c | 8 ++++++-- src/lxc/lxccontainer.c | 12 +++++++++--- src/lxc/monitor.c | 10 ++++++---- src/lxc/network.c | 19 ++++++++++++++++--- src/lxc/start.c | 10 ++++++++-- src/lxc/storage/btrfs.c | 34 +++++++++++++++++++++++++--------- src/lxc/storage/storage_utils.c | 12 +++++++++++- 9 files changed, 108 insertions(+), 30 deletions(-) diff --git a/src/lxc/af_unix.c b/src/lxc/af_unix.c index 6debb07f1..4bbc72b47 100644 --- a/src/lxc/af_unix.c +++ b/src/lxc/af_unix.c @@ -36,12 +36,16 @@ #include "log.h" #include "utils.h" +#ifndef HAVE_STRLCPY +#include "include/strlcpy.h" +#endif + lxc_log_define(lxc_af_unix, lxc); int lxc_abstract_unix_open(const char *path, int type, int flags) { int fd, ret; - size_t len; + size_t len, retlen; struct sockaddr_un addr; fd = socket(PF_UNIX, type, 0); @@ -63,8 +67,14 @@ int lxc_abstract_unix_open(const char *path, int type, int flags) errno = ENAMETOOLONG; return -1; } + /* addr.sun_path[0] has already been set to 0 by memset() */ - strncpy(&addr.sun_path[1], &path[1], len); + retlen = strlcpy(&addr.sun_path[1], &path[1], len); + if (retlen >= sizeof(addr.sun_path) - 1) { + close(fd); + errno = ENAMETOOLONG; + return -1; + } ret = bind(fd, (struct sockaddr *)&addr, offsetof(struct sockaddr_un, sun_path) + len + 1); @@ -98,7 +108,7 @@ int lxc_abstract_unix_close(int fd) int lxc_abstract_unix_connect(const char *path) { int fd, ret; - size_t len; + size_t len, retlen; struct sockaddr_un addr; fd = socket(PF_UNIX, SOCK_STREAM, 0); @@ -116,8 +126,14 @@ int lxc_abstract_unix_connect(const char *path) errno = ENAMETOOLONG; return -1; } + /* addr.sun_path[0] has already been set to 0 by memset() */ - strncpy(&addr.sun_path[1], &path[1], strlen(&path[1])); + retlen = strlcpy(&addr.sun_path[1], &path[1], len); + if (retlen >= sizeof(addr.sun_path - 1)) { + close(fd); + errno = ENAMETOOLONG; + return -1; + } ret = connect(fd, (struct sockaddr *)&addr, offsetof(struct sockaddr_un, sun_path) + len + 1); diff --git a/src/lxc/criu.c b/src/lxc/criu.c index f60a6e155..9c70c5921 100644 --- a/src/lxc/criu.c +++ b/src/lxc/criu.c @@ -51,6 +51,10 @@ #include <mntent.h> #endif +#ifndef HAVE_STRLCPY +#include "include/strlcpy.h" +#endif + #define CRIU_VERSION "2.0" #define CRIU_GITID_VERSION "2.0" @@ -536,6 +540,7 @@ static void exec_criu(struct criu_opts *opts) argv = m; lxc_list_for_each(it, &opts->c->lxc_conf->network) { + size_t retlen; char eth[128], *veth; char *fmt; struct lxc_netdev *n = it->elem; @@ -552,9 +557,9 @@ static void exec_criu(struct criu_opts *opts) } if (n->name[0] != '\0') { - if (strlen(n->name) >= sizeof(eth)) + retlen = strlcpy(eth, n->name, sizeof(eth)); + if (retlen >= sizeof(eth)) goto err; - strncpy(eth, n->name, sizeof(eth)); } else { ret = snprintf(eth, sizeof(eth), "eth%d", netnr); if (ret < 0 || ret >= sizeof(eth)) diff --git a/src/lxc/log.c b/src/lxc/log.c index 82ae99119..94b61d432 100644 --- a/src/lxc/log.c +++ b/src/lxc/log.c @@ -46,6 +46,10 @@ #include "utils.h" #include "lxccontainer.h" +#ifndef HAVE_STRLCPY +#include "include/strlcpy.h" +#endif + /* We're logging in seconds and nanoseconds. Assuming that the underlying * datatype is currently at maximum a 64bit integer, we have a date string that * is of maximum length (2^64 - 1) * 2 = (21 + 21) = 42. @@ -690,8 +694,8 @@ extern const char *lxc_log_get_file(void) extern void lxc_log_set_prefix(const char *prefix) { - strncpy(log_prefix, prefix, sizeof(log_prefix)); - log_prefix[sizeof(log_prefix) - 1] = 0; + /* We don't care if thte prefix is truncated. */ + (void)strlcpy(log_prefix, prefix, sizeof(log_prefix)); } extern const char *lxc_log_get_prefix(void) diff --git a/src/lxc/lxccontainer.c b/src/lxc/lxccontainer.c index b396a6d5a..a9041c860 100644 --- a/src/lxc/lxccontainer.c +++ b/src/lxc/lxccontainer.c @@ -87,6 +87,10 @@ #include <mntent.h> #endif +#ifndef HAVE_STRLCPY +#include "include/strlcpy.h" +#endif + /* Define faccessat() if missing from the C library */ #ifndef HAVE_FACCESSAT static int faccessat(int __fd, const char *__file, int __type, int __flag) @@ -728,7 +732,7 @@ static void push_arg(char ***argp, char *arg, int *nargs) static char **split_init_cmd(const char *incmd) { - size_t len; + size_t len, retlen; char *copy, *p; char **argv; int nargs = 0; @@ -739,8 +743,10 @@ static char **split_init_cmd(const char *incmd) len = strlen(incmd) + 1; copy = alloca(len); - strncpy(copy, incmd, len); - copy[len - 1] = '\0'; + retlen = strlcpy(copy, incmd, len); + if (retlen >= len) { + return NULL; + } do { argv = malloc(sizeof(char *)); diff --git a/src/lxc/monitor.c b/src/lxc/monitor.c index 09fb14d42..3aacf22ea 100644 --- a/src/lxc/monitor.c +++ b/src/lxc/monitor.c @@ -49,6 +49,10 @@ #include "state.h" #include "utils.h" +#ifndef HAVE_STRLCPY +#include "include/strlcpy.h" +#endif + lxc_log_define(lxc_monitor, lxc); /* routines used by monitor publishers (containers) */ @@ -131,9 +135,8 @@ void lxc_monitor_send_state(const char *name, lxc_state_t state, const char *lxcpath) { struct lxc_msg msg = {.type = lxc_msg_state, .value = state}; - strncpy(msg.name, name, sizeof(msg.name)); - msg.name[sizeof(msg.name) - 1] = 0; + (void)strlcpy(msg.name, name, sizeof(msg.name)); lxc_monitor_fifo_send(&msg, lxcpath); } @@ -141,9 +144,8 @@ void lxc_monitor_send_exit_code(const char *name, int exit_code, const char *lxcpath) { struct lxc_msg msg = {.type = lxc_msg_exit_code, .value = exit_code}; - strncpy(msg.name, name, sizeof(msg.name)); - msg.name[sizeof(msg.name) - 1] = 0; + (void)strlcpy(msg.name, name, sizeof(msg.name)); lxc_monitor_fifo_send(&msg, lxcpath); } diff --git a/src/lxc/network.c b/src/lxc/network.c index 8345ee394..a7e05280d 100644 --- a/src/lxc/network.c +++ b/src/lxc/network.c @@ -59,6 +59,10 @@ #include <../include/ifaddrs.h> #endif +#ifndef HAVE_STRLCPY +#include "include/strlcpy.h" +#endif + #ifndef IFLA_LINKMODE #define IFLA_LINKMODE 17 #endif @@ -1894,6 +1898,7 @@ static int lxc_ovs_attach_bridge(const char *bridge, const char *nic) int lxc_bridge_attach(const char *bridge, const char *ifname) { int err, fd, index; + size_t retlen; struct ifreq ifr; if (strlen(ifname) >= IFNAMSIZ) @@ -1910,7 +1915,10 @@ int lxc_bridge_attach(const char *bridge, const char *ifname) if (fd < 0) return -errno; - strncpy(ifr.ifr_name, bridge, IFNAMSIZ - 1); + retlen = strlcpy(ifr.ifr_name, bridge, IFNAMSIZ); + if (retlen >= IFNAMSIZ) + return -E2BIG; + ifr.ifr_name[IFNAMSIZ - 1] = '\0'; ifr.ifr_ifindex = index; err = ioctl(fd, SIOCBRADDIF, &ifr); @@ -2113,6 +2121,7 @@ static int lxc_create_network_unpriv_exec(const char *lxcpath, const char *lxcna if (child == 0) { int ret; + size_t retlen; char pidstr[LXC_NUMSTRLEN64]; close(pipefd[0]); @@ -2127,9 +2136,13 @@ static int lxc_create_network_unpriv_exec(const char *lxcpath, const char *lxcna } if (netdev->link[0] != '\0') - strncpy(netdev_link, netdev->link, IFNAMSIZ - 1); + retlen = strlcpy(netdev_link, netdev->link, IFNAMSIZ); else - strncpy(netdev_link, "none", IFNAMSIZ - 1); + retlen = strlcpy(netdev_link, "none", IFNAMSIZ); + if (retlen >= IFNAMSIZ) { + SYSERROR("Invalid network device name"); + _exit(EXIT_FAILURE); + } ret = snprintf(pidstr, LXC_NUMSTRLEN64, "%d", pid); if (ret < 0 || ret >= LXC_NUMSTRLEN64) diff --git a/src/lxc/start.c b/src/lxc/start.c index ce5cb3366..e9b0efe5f 100644 --- a/src/lxc/start.c +++ b/src/lxc/start.c @@ -89,6 +89,10 @@ #include "terminal.h" #include "utils.h" +#ifndef HAVE_STRLCPY +#include "include/strlcpy.h" +#endif + lxc_log_define(lxc_start, lxc); extern void mod_all_rdeps(struct lxc_container *c, bool inc); @@ -410,6 +414,7 @@ static int signal_handler(int fd, uint32_t events, void *data, int lxc_serve_state_clients(const char *name, struct lxc_handler *handler, lxc_state_t state) { + size_t retlen; ssize_t ret; struct lxc_list *cur, *next; struct lxc_state_client *client; @@ -427,8 +432,9 @@ int lxc_serve_state_clients(const char *name, struct lxc_handler *handler, return 0; } - strncpy(msg.name, name, sizeof(msg.name)); - msg.name[sizeof(msg.name) - 1] = 0; + retlen = strlcpy(msg.name, name, sizeof(msg.name)); + if (retlen >= sizeof(msg.name)) + return -E2BIG; lxc_list_for_each_safe(cur, &handler->conf->state_clients, next) { client = cur->elem; diff --git a/src/lxc/storage/btrfs.c b/src/lxc/storage/btrfs.c index 3b6b8d96e..955b6a7b2 100644 --- a/src/lxc/storage/btrfs.c +++ b/src/lxc/storage/btrfs.c @@ -42,6 +42,10 @@ #include "storage.h" #include "utils.h" +#ifndef HAVE_STRLCPY +#include "include/strlcpy.h" +#endif + lxc_log_define(btrfs, lxc); /* @@ -223,6 +227,7 @@ int btrfs_umount(struct lxc_storage *bdev) static int btrfs_subvolume_create(const char *path) { int ret, saved_errno; + size_t retlen; struct btrfs_ioctl_vol_args args; char *p, *newfull; int fd = -1; @@ -248,8 +253,9 @@ static int btrfs_subvolume_create(const char *path) } memset(&args, 0, sizeof(args)); - strncpy(args.name, p + 1, BTRFS_SUBVOL_NAME_MAX); - args.name[BTRFS_SUBVOL_NAME_MAX - 1] = 0; + retlen = strlcpy(args.name, p + 1, BTRFS_SUBVOL_NAME_MAX); + if (retlen >= BTRFS_SUBVOL_NAME_MAX) + return -E2BIG; ret = ioctl(fd, BTRFS_IOC_SUBVOL_CREATE, &args); saved_errno = errno; @@ -303,6 +309,7 @@ int btrfs_same_fs(const char *orig, const char *new) int btrfs_snapshot(const char *orig, const char *new) { + size_t retlen; struct btrfs_ioctl_vol_args_v2 args; char *newdir, *newname; char *newfull = NULL; @@ -328,9 +335,9 @@ int btrfs_snapshot(const char *orig, const char *new) goto out; memset(&args, 0, sizeof(args)); - args.fd = fd; - strncpy(args.name, newname, BTRFS_SUBVOL_NAME_MAX); - args.name[BTRFS_SUBVOL_NAME_MAX - 1] = 0; + retlen = strlcpy(args.name, newname, BTRFS_SUBVOL_NAME_MAX); + if (retlen >= BTRFS_SUBVOL_NAME_MAX) + goto out; ret = ioctl(fddst, BTRFS_IOC_SNAP_CREATE_V2, &args); saved_errno = errno; @@ -498,6 +505,7 @@ bool btrfs_create_snapshot(struct lxc_conf *conf, struct lxc_storage *orig, static int btrfs_do_destroy_subvol(const char *path) { int ret, fd = -1; + size_t retlen; struct btrfs_ioctl_vol_args args; char *p, *newfull = strdup(path); @@ -522,8 +530,12 @@ static int btrfs_do_destroy_subvol(const char *path) } memset(&args, 0, sizeof(args)); - strncpy(args.name, p+1, BTRFS_SUBVOL_NAME_MAX); - args.name[BTRFS_SUBVOL_NAME_MAX-1] = 0; + retlen = strlcpy(args.name, p+1, BTRFS_SUBVOL_NAME_MAX); + if (retlen >= BTRFS_SUBVOL_NAME_MAX) { + free(newfull); + return -E2BIG; + } + ret = ioctl(fd, BTRFS_IOC_SNAP_DESTROY, &args); INFO("btrfs: snapshot destroy ioctl returned %d for %s", ret, path); if (ret < 0 && errno == EPERM) @@ -577,21 +589,25 @@ static bool update_tree_node(struct mytree_node *n, u64 id, u64 parent, { if (id) n->objid = id; + if (parent) n->parentid = parent; + if (name) { n->name = malloc(name_len + 1); if (!n->name) return false; - strncpy(n->name, name, name_len); - n->name[name_len] = '\0'; + + strcpy(n->name, name); } + if (dirname) { n->dirname = malloc(strlen(dirname) + 1); if (!n->dirname) { free(n->name); return false; } + strcpy(n->dirname, dirname); } return true; diff --git a/src/lxc/storage/storage_utils.c b/src/lxc/storage/storage_utils.c index 1a67b6de0..6570bb88e 100644 --- a/src/lxc/storage/storage_utils.c +++ b/src/lxc/storage/storage_utils.c @@ -46,6 +46,10 @@ #include "storage_utils.h" #include "utils.h" +#ifndef HAVE_STRLCPY +#include "include/strlcpy.h" +#endif + #ifndef BLKGETSIZE64 #define BLKGETSIZE64 _IOR(0x12, 114, size_t) #endif @@ -85,8 +89,13 @@ char *dir_new_path(char *src, const char *oldname, const char *name, } while ((p2 = strstr(src, oldname)) != NULL) { + size_t retlen; + /* copy text up to oldname */ - strncpy(p, src, p2 - src); + retlen = strlcpy(p, src, p2 - src); + if (retlen >= p2 - src) + return NULL; + /* move target pointer (p) */ p += p2 - src; /* print new name in place of oldname */ @@ -94,6 +103,7 @@ char *dir_new_path(char *src, const char *oldname, const char *name, /* move src to end of oldname */ src = p2 + l2; } + /* copy the rest of src */ sprintf(p, "%s", src); return ret; From 950b0e08275374bfd8e25049ef825b43e014f5cc Mon Sep 17 00:00:00 2001 From: Christian Brauner <christian.brau...@ubuntu.com> Date: Fri, 11 May 2018 13:02:41 +0200 Subject: [PATCH 3/3] CODING_STYLE: add section about using strlcpy() Signed-off-by: Christian Brauner <christian.brau...@ubuntu.com> --- CODING_STYLE.md | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/CODING_STYLE.md b/CODING_STYLE.md index 24c6d3603..784c5134e 100644 --- a/CODING_STYLE.md +++ b/CODING_STYLE.md @@ -662,3 +662,13 @@ rules to use them: #endif }; ``` + +#### Use `strlcpy()` instead of `strncpy()` + +When copying strings always use `strlcpy()` instead of `strncpy()`. The +advantage of `strlcpy()` is that it will always append a `\0` byte to the +string. + +Unless you have a valid reason to accept truncation you must check whether +truncation has occurred, treat it as an error, and handle the error +appropriately.
_______________________________________________ lxc-devel mailing list lxc-devel@lists.linuxcontainers.org http://lists.linuxcontainers.org/listinfo/lxc-devel