[Qemu-devel] [PATCH 11/25] qemu-nbd: Fix and improve input verification

[Max Reitz <mreitz@redhat.com>]

This patch makes sure the result of strtol() does not overflow (by
storing it in long integers instead of plain integers, and by checking
errno), allows the user to specify "--discard on" and
"--detect-zeroes unmap" in any order and strips the trailing \n from two
error messages.

Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 qemu-nbd.c | 40 +++++++++++++++++++++++++++-------------
 1 file changed, 27 insertions(+), 13 deletions(-)

diff --git a/qemu-nbd.c b/qemu-nbd.c
index fd1e0c8..7376a35 100644
--- a/qemu-nbd.c
+++ b/qemu-nbd.c
@@ -51,7 +51,7 @@ static char *srcpath;
 static char *sockpath;
 static int persistent = 0;
 static enum { RUNNING, TERMINATE, TERMINATING, TERMINATED } state;
-static int shared = 1;
+static long shared = 1;
 static int nb_fds;
 
 static void usage(const char *name)
@@ -432,10 +432,10 @@ int main(int argc, char **argv)
     };
     int ch;
     int opt_ind = 0;
-    int li;
+    long li;
     char *end;
     int flags = BDRV_O_RDWR;
-    int partition = -1;
+    long partition = -1;
     int ret = 0;
     int fd;
     bool seen_cache = false;
@@ -510,11 +510,6 @@ int main(int argc, char **argv)
                 errx(EXIT_FAILURE, "Failed to parse detect_zeroes mode: %s", 
                      error_get_pretty(local_err));
             }
-            if (detect_zeroes == BLOCKDEV_DETECT_ZEROES_OPTIONS_UNMAP &&
-                !(flags & BDRV_O_UNMAP)) {
-                errx(EXIT_FAILURE, "setting detect-zeroes to unmap is not allowed "
-                                   "without setting discard operation to unmap"); 
-            }
             break;
         case 'b':
             bindto = optarg;
@@ -530,13 +525,17 @@ int main(int argc, char **argv)
             port = (uint16_t)li;
             break;
         case 'o':
-                dev_offset = strtoll (optarg, &end, 0);
+            errno = 0;
+            dev_offset = strtoll(optarg, &end, 0);
             if (*end) {
                 errx(EXIT_FAILURE, "Invalid offset `%s'", optarg);
             }
             if (dev_offset < 0) {
                 errx(EXIT_FAILURE, "Offset must be positive `%s'", optarg);
             }
+            if (errno) {
+                err(EXIT_FAILURE, "Invalid offset `%s'", optarg);
+            }
             break;
         case 'l':
             if (strstart(optarg, SNAPSHOT_OPT_BASE, NULL)) {
@@ -559,13 +558,13 @@ int main(int argc, char **argv)
                 errx(EXIT_FAILURE, "Invalid partition `%s'", optarg);
             }
             if (partition < 1 || partition > 8) {
-                errx(EXIT_FAILURE, "Invalid partition %d", partition);
+                errx(EXIT_FAILURE, "Invalid partition %s", optarg);
             }
             break;
         case 'k':
             sockpath = optarg;
             if (sockpath[0] != '/') {
-                errx(EXIT_FAILURE, "socket path must be absolute\n");
+                errx(EXIT_FAILURE, "socket path must be absolute");
             }
             break;
         case 'd':
@@ -580,7 +579,12 @@ int main(int argc, char **argv)
                 errx(EXIT_FAILURE, "Invalid shared device number '%s'", optarg);
             }
             if (shared < 1) {
-                errx(EXIT_FAILURE, "Shared device number must be greater than 0\n");
+                errx(EXIT_FAILURE,
+                     "Shared device number must be greater than 0");
+            }
+            if (shared >= INT_MAX) {
+                errx(EXIT_FAILURE,
+                     "Shared device number must be less than %i", INT_MAX);
             }
             break;
         case 'f':
@@ -606,6 +610,12 @@ int main(int argc, char **argv)
         }
     }
 
+    if (detect_zeroes == BLOCKDEV_DETECT_ZEROES_OPTIONS_UNMAP &&
+        !(flags & BDRV_O_UNMAP)) {
+        errx(EXIT_FAILURE, "Setting detect-zeroes to unmap is not allowed "
+                           "without setting discard operation to unmap");
+    }
+
     if ((argc - optind) != 1) {
         errx(EXIT_FAILURE, "Invalid number of argument.\n"
              "Try `%s --help' for more information.",
@@ -730,10 +740,14 @@ int main(int argc, char **argv)
     }
 
     if (partition != -1) {
+        if (dev_offset) {
+            errx(EXIT_FAILURE, "Cannot use both -o and -P at the same time");
+        }
+
         ret = find_partition(blk, partition, &dev_offset, &fd_size);
         if (ret < 0) {
             errno = -ret;
-            err(EXIT_FAILURE, "Could not find partition %d", partition);
+            err(EXIT_FAILURE, "Could not find partition %ld", partition);
         }
     }
 
-- 
2.1.0