ubi tools: ubiupdatevol supports "-" input and actually respects -s SIZE

Decided to not make any flash applets NOEXEC.
Minor robustifications here and there. Better error messages. Save on strings:

function                                             old     new   delta
ubi_tools_main                                      1235    1288     +53
ubi_get_volid_by_name                                125     133      +8
ubirename_main                                       198     204      +6
get_num_from_file                                     90      94      +4
------------------------------------------------------------------------------
(add/remove: 0/0 grow/shrink: 4/0 up/down: 71/0)               Total: 71 bytes
   text	   data	    bss	    dec	    hex	filename
 915696	    485	   6880	 923061	  e15b5	busybox_old
 915670	    485	   6880	 923035	  e159b	busybox_unstripped

Signed-off-by: Denys Vlasenko <vda.linux@googlemail.com>

diff --git a/NOFORK_NOEXEC.lst b/NOFORK_NOEXEC.lst
index d54c206fe28352e352796381d2294ba6f68f9039..981a10192ff4d2debb97a1ac491a7dbcf4e705f4 100644 (file)
--- a/NOFORK_NOEXEC.lst
+++ b/NOFORK_NOEXEC.lst
@@ -123,10 +123,10 @@ fgconsole - noexec. leaks: get_console_fd_or_die() may open a new fd, or return
 fgrep - longterm runner ("CMD | fgrep ..."  may run indefinitely, better to exec to conserve memory)
 find - noexec. runner
 findfs - suid
-flash_eraseall
-flash_lock
-flash_unlock
-flashcp - needs ^C. flash writing may be slow, better to free memory by execing
+flash_eraseall - could be noexec, but I feel flash ops are risky (prone to hw/fw/sw bugs)
+flash_lock - could be noexec, but I feel flash ops are risky (prone to hw/fw/sw bugs)
+flash_unlock - could be noexec, but I feel flash ops are risky (prone to hw/fw/sw bugs)
+flashcp - needs ^C. could be noexec, but I feel flash ops are risky (prone to hw/fw/sw bugs)
 flock - spawner, changes state (file locks), let's play safe and not be noexec
 fold - noexec. runner
 free - nofork candidate(struct globals, needs to close /proc/meminfo fd)
@@ -366,13 +366,13 @@ tty - NOFORK
 ttysize - NOFORK
 tunctl - noexec
 tune2fs - noexec. leaks: open+xfunc
-ubiattach
-ubidetach
-ubimkvol
-ubirename
-ubirmvol
-ubirsvol
-ubiupdatevol
+ubiattach - could be noexec, but I feel flash ops are risky (prone to hw/fw/sw bugs)
+ubidetach - could be noexec, but I feel flash ops are risky (prone to hw/fw/sw bugs)
+ubimkvol - could be noexec, but I feel flash ops are risky (prone to hw/fw/sw bugs)
+ubirename - could be noexec, but I feel flash ops are risky (prone to hw/fw/sw bugs)
+ubirmvol - could be noexec, but I feel flash ops are risky (prone to hw/fw/sw bugs)
+ubirsvol - could be noexec, but I feel flash ops are risky (prone to hw/fw/sw bugs)
+ubiupdatevol - could be noexec, but I feel flash ops are risky (prone to hw/fw/sw bugs)
 udhcpc - daemon
 udhcpd - daemon
 udpsvd - daemon

diff --git a/libbb/ubi.c b/libbb/ubi.c
index 34595d79790c409d21c8714658760e83c522f15b..a90016acf4a80685ed71e617ab346cec50da8d2d 100644 (file)
--- a/libbb/ubi.c
+++ b/libbb/ubi.c
@@ -35,6 +35,7 @@ int FAST_FUNC ubi_get_volid_by_name(unsigned ubi_devnum, const char *vol_name)
                if (open_read_close(fname, buf, sizeof(buf)) <= 0)
                        continue;
 
+               buf[UBI_MAX_VOLUME_NAME] = '\0';
                strchrnul(buf, '\n')[0] = '\0';
                if (strcmp(vol_name, buf) == 0)
                        return i;

diff --git a/miscutils/flash_eraseall.c b/miscutils/flash_eraseall.c
index af9ebea244d908e6c2e621d64926f4a709e67ff0..3ddd9dd990bd6f2d562217dc95d0b4f32e9dd521 100644 (file)
--- a/miscutils/flash_eraseall.c
+++ b/miscutils/flash_eraseall.c
@@ -17,6 +17,7 @@
 //config:      This utility is used to erase the whole MTD device.
 
 //applet:IF_FLASH_ERASEALL(APPLET(flash_eraseall, BB_DIR_USR_SBIN, BB_SUID_DROP))
+/* not NOEXEC: if flash operation stalls, use less memory in "hung" process */
 
 //kbuild:lib-$(CONFIG_FLASH_ERASEALL) += flash_eraseall.o
 

diff --git a/miscutils/flash_lock_unlock.c b/miscutils/flash_lock_unlock.c
index 374eed5f6c0e5b095298fcc9d13b3e698e9fd16f..6f2c049f4a29bc5339fc48b7fba8a2549412356b 100644 (file)
--- a/miscutils/flash_lock_unlock.c
+++ b/miscutils/flash_lock_unlock.c
@@ -20,6 +20,7 @@
 //                       APPLET_ODDNAME:name          main               location         suid_type     help
 //applet:IF_FLASH_LOCK(  APPLET_ODDNAME(flash_lock,   flash_lock_unlock, BB_DIR_USR_SBIN, BB_SUID_DROP, flash_lock))
 //applet:IF_FLASH_UNLOCK(APPLET_ODDNAME(flash_unlock, flash_lock_unlock, BB_DIR_USR_SBIN, BB_SUID_DROP, flash_unlock))
+/* not NOEXEC: if flash operation stalls, use less memory in "hung" process */
 
 //kbuild:lib-$(CONFIG_FLASH_LOCK) += flash_lock_unlock.o
 //kbuild:lib-$(CONFIG_FLASH_UNLOCK) += flash_lock_unlock.o

diff --git a/miscutils/flashcp.c b/miscutils/flashcp.c
index d4ac62df4611ac404960375e9361e2fec1329703..c10b96ee8e23c7a6a1995ee3c0175ac74626b3c9 100644 (file)
--- a/miscutils/flashcp.c
+++ b/miscutils/flashcp.c
@@ -14,6 +14,7 @@
 //config:      This utility is used to copy images into a MTD device.
 
 //applet:IF_FLASHCP(APPLET(flashcp, BB_DIR_USR_SBIN, BB_SUID_DROP))
+/* not NOEXEC: if flash operation stalls, use less memory in "hung" process */
 
 //kbuild:lib-$(CONFIG_FLASHCP) += flashcp.o
 

diff --git a/miscutils/ubi_tools.c b/miscutils/ubi_tools.c
index 494718ccfc7654775dad2105cb51e053c8627634..123551e94fdd27f8ecc42ea10c8611c5fbee7d00 100644 (file)
--- a/miscutils/ubi_tools.c
+++ b/miscutils/ubi_tools.c
@@ -52,6 +52,7 @@
 //applet:IF_UBIRMVOL(    APPLET_ODDNAME(ubirmvol,  ubi_tools, BB_DIR_USR_SBIN, BB_SUID_DROP, ubirmvol))
 //applet:IF_UBIRSVOL(    APPLET_ODDNAME(ubirsvol,  ubi_tools, BB_DIR_USR_SBIN, BB_SUID_DROP, ubirsvol))
 //applet:IF_UBIUPDATEVOL(APPLET_ODDNAME(ubiupdatevol, ubi_tools, BB_DIR_USR_SBIN, BB_SUID_DROP, ubiupdatevol))
+/* not NOEXEC: if flash operation stalls, use less memory in "hung" process */
 
 //kbuild:lib-$(CONFIG_UBIATTACH) += ubi_tools.o
 //kbuild:lib-$(CONFIG_UBIDETACH) += ubi_tools.o
@@ -83,16 +84,16 @@
 #define do_rsvol  (ENABLE_UBIRSVOL     && (UBI_APPLET_CNT == 1 || applet_name[4] == 's'))
 #define do_update (ENABLE_UBIUPDATEVOL && (UBI_APPLET_CNT == 1 || applet_name[4] == 'p'))
 
-static unsigned get_num_from_file(const char *path, unsigned max, const char *errmsg)
+static unsigned get_num_from_file(const char *path, unsigned max)
 {
        char buf[sizeof(long long)*3];
        unsigned long long num;
 
        if (open_read_close(path, buf, sizeof(buf)) < 0)
-               bb_perror_msg_and_die(errmsg, path);
+               bb_perror_msg_and_die("can't open '%s'", path);
        /* It can be \n terminated, xatoull won't work well */
        if (sscanf(buf, "%llu", &num) != 1 || num > max)
-               bb_error_msg_and_die(errmsg, path);
+               bb_error_msg_and_die("number in '%s' is malformed or too large", path);
        return num;
 }
 
@@ -226,10 +227,10 @@ int ubi_tools_main(int argc UNUSED_PARAM, char **argv)
                        p = path_sys_class_ubi_ubi + sprintf(path_sys_class_ubi_ubi, "%u/", num);
 
                        strcpy(p, "avail_eraseblocks");
-                       leb_avail = get_num_from_file(path, UINT_MAX, "Can't get available eraseblocks from '%s'");
+                       leb_avail = get_num_from_file(path, UINT_MAX);
 
                        strcpy(p, "eraseblock_size");
-                       leb_size = get_num_from_file(path, MAX_SANE_ERASEBLOCK, "Can't get eraseblock size from '%s'");
+                       leb_size = get_num_from_file(path, MAX_SANE_ERASEBLOCK);
 
                        size_bytes = leb_avail * (unsigned long long)leb_size;
                        //if (size_bytes <= 0)
@@ -241,16 +242,19 @@ int ubi_tools_main(int argc UNUSED_PARAM, char **argv)
                if (!(opts & OPTION_N))
                        bb_error_msg_and_die("name not specified");
 
+               /* the structure is memset(0) above */
                mkvol_req.vol_id = vol_id;
                mkvol_req.vol_type = UBI_DYNAMIC_VOLUME;
                if ((opts & OPTION_t) && type[0] == 's')
                        mkvol_req.vol_type = UBI_STATIC_VOLUME;
                mkvol_req.alignment = alignment;
                mkvol_req.bytes = size_bytes; /* signed int64_t */
-               strncpy(mkvol_req.name, vol_name, UBI_MAX_VOLUME_NAME);
-               mkvol_req.name_len = strlen(vol_name);
+               /* strnlen avoids overflow of 16-bit field (paranoia) */
+               mkvol_req.name_len = strnlen(vol_name, UBI_MAX_VOLUME_NAME+1);
                if (mkvol_req.name_len > UBI_MAX_VOLUME_NAME)
                        bb_error_msg_and_die("volume name too long: '%s'", vol_name);
+               /* this is safe: .name[] is UBI_MAX_VOLUME_NAME+1 bytes */
+               strcpy(mkvol_req.name, vol_name);
 
                xioctl(fd, UBI_IOCMKVOL, &mkvol_req);
        } else
@@ -315,38 +319,49 @@ int ubi_tools_main(int argc UNUSED_PARAM, char **argv)
                else {
                        unsigned ubinum, volnum;
                        unsigned leb_size;
-                       ssize_t len;
-                       char *input_data;
+                       char *buf;
 
                        /* Assume that device is in normal format. */
                        /* Removes need for scanning sysfs tree as full libubi does. */
                        if (sscanf(ubi_ctrl, "/dev/ubi%u_%u", &ubinum, &volnum) != 2)
-                               bb_error_msg_and_die("wrong format of UBI device name");
+                               bb_error_msg_and_die("UBI device name '%s' is not /dev/ubiN_M", ubi_ctrl);
 
                        sprintf(path_sys_class_ubi_ubi, "%u_%u/usable_eb_size", ubinum, volnum);
-                       leb_size = get_num_from_file(path, MAX_SANE_ERASEBLOCK, "Can't get usable eraseblock size from '%s'");
+                       leb_size = get_num_from_file(path, MAX_SANE_ERASEBLOCK);
 
-                       if (!(opts & OPTION_t)) {
-                               if (!*argv)
-                                       bb_show_usage();
+                       if (!*argv)
+                               bb_show_usage();
+                       if (NOT_LONE_DASH(*argv)) /* mtd-utils supports "-" as stdin */
                                xmove_fd(xopen(*argv, O_RDONLY), STDIN_FILENO);
-                               if (!(opts & OPTION_s)) {
-                                       struct stat st;
-                                       xfstat(STDIN_FILENO, &st, *argv);
-                                       size_bytes = st.st_size;
-                               }
+
+                       if (!(opts & OPTION_s)) {
+                               struct stat st;
+                               xfstat(STDIN_FILENO, &st, *argv);
+                               size_bytes = st.st_size;
                        }
 
                        bytes64 = size_bytes;
                        /* this ioctl expects signed int64_t* parameter */
                        xioctl(fd, UBI_IOCVOLUP, &bytes64);
 
-                       input_data = xmalloc(leb_size);
-                       while ((len = full_read(STDIN_FILENO, input_data, leb_size)) > 0) {
-                               xwrite(fd, input_data, len);
+                       /* can't use bb_copyfd_exact_size(): copy in blocks of exactly leb_size */
+                       buf = xmalloc(leb_size);
+                       while (size_bytes != 0) {
+                               int len = full_read(STDIN_FILENO, buf, leb_size);
+                               if (len <= 0) {
+                                       if (len < 0)
+                                               bb_perror_msg_and_die("read error from '%s'", *argv);
+                                       break;
+                               }
+                               if ((unsigned)len > size_bytes) {
+                                       /* for this case: "ubiupdatevol -s 1024000 $UBIDEV /dev/urandom" */
+                                       len = size_bytes;
+                               }
+                               xwrite(fd, buf, len);
+                               size_bytes -= len;
                        }
-                       if (len < 0)
-                               bb_perror_msg_and_die("UBI volume update failed");
+                       if (ENABLE_FEATURE_CLEAN_UP)
+                               free(buf);
                }
        }
 

diff --git a/miscutils/ubirename.c b/miscutils/ubirename.c
index 786c4b9fa455879721b434516150e2f924d78c23..ecc8fe137f9a41c17a8e5f97065b91318546c117 100644 (file)
--- a/miscutils/ubirename.c
+++ b/miscutils/ubirename.c
@@ -14,6 +14,7 @@
 //config:      Utility to rename UBI volumes
 
 //applet:IF_UBIRENAME(APPLET(ubirename, BB_DIR_USR_SBIN, BB_SUID_DROP))
+/* not NOEXEC: if flash operation stalls, use less memory in "hung" process */
 
 //kbuild:lib-$(CONFIG_UBIRENAME) += ubirename.o
 
@@ -80,9 +81,12 @@ int ubirename_main(int argc, char **argv)
        argv += 2;
        while (argv[0]) {
                rnvol->ents[n].vol_id = ubi_get_volid_by_name(ubi_devnum, argv[0]);
-               rnvol->ents[n].name_len = strlen(argv[1]);
+
+               /* strnlen avoids overflow of 16-bit field (paranoia) */
+               rnvol->ents[n].name_len = strnlen(argv[1], sizeof(rnvol->ents[n].name));
                if (rnvol->ents[n].name_len >= sizeof(rnvol->ents[n].name))
                        bb_error_msg_and_die("new name '%s' is too long", argv[1]);
+
                strcpy(rnvol->ents[n].name, argv[1]);
                n++;
                argv += 2;