← Commit list

798b94518

Denys Vlasenko
Denys Vlasenko
2017-08-07
SUCCESS Duration: 143s

Subject

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

Binary

No binary (build failed)

Architecture & Compiler Flags

Architecture: x86_64-linux-gnu Compiler: gcc (Ubuntu 11.4.0-1ubuntu1~22.04.3) 11.4.0

—

Build Log

(log yok)

Commit Diff GitHub →

NOFORK_NOEXEC.lst +11 -11libbb/ubi.c +1 -0miscutils/flash_eraseall.c +1 -0miscutils/flash_lock_unlock.c +1 -0miscutils/flashcp.c +1 -0miscutils/ubi_tools.c +39 -24miscutils/ubirename.c +5 -1
NOFORK_NOEXEC.lst+11 -11
@@ -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
libbb/ubi.c+1 -0
@@ -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;
miscutils/flash_eraseall.c+1 -0
@@ -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
miscutils/flash_lock_unlock.c+1 -0
@@ -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
miscutils/flashcp.c+1 -0
@@ -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
miscutils/ubi_tools.c+39 -24
@@ -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);
}
}
miscutils/ubirename.c+5 -1
@@ -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;