syscall.c: dump write I/O even if error is returned

It makes sense to try to dump the argument of write syscalls as they can
be readily available despite the fact that error has been returned.

* syscall.c (dumpio): Move check for syserror and check for fd in
read_set to the end of the function.
* tests/read-write.c: Add a check for this behaviour.
* tests/pread64-pwrite64.c: Update expected output.
* NEWS: Mention this.

Suggested-by: J. Bruce Fields <bfields@fieldses.org>
Closes: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=436284

diff --git a/NEWS b/NEWS
index 442e142635b44cd63393afa4247f66a69c098d4e..2816f64e6a731995539c7c5e9273ace23549408f 100644 (file)
--- a/NEWS
+++ b/NEWS
@@ -7,6 +7,8 @@ Noteworthy changes in release ?.?? (????-??-??)
 * Changes in behaviour
   * Angle brackets are now printed as octal number escape sequences
     in the output of paths associated with file descriptors.
+  * Data dump for write syscalls (-e write) is performed regardless
+    of successfulness of these syscall (addresses Debian bug #436284).
 
 * Improvements
   * Implemented delay injection (-e inject=SET:delay_enter= and

diff --git a/syscall.c b/syscall.c
index 363ac74c0d01ce0694f892374bf3226547e4964e..a88e05493d0b3448259f0323b90b1d5f7e4ab5ed 100644 (file)
--- a/syscall.c
+++ b/syscall.c
@@ -434,36 +434,10 @@ decode_syscall_subcall(struct tcb *tcp)
 static void
 dumpio(struct tcb *tcp)
 {
-       if (syserror(tcp))
-               return;
-
        int fd = tcp->u_arg[0];
        if (fd < 0)
                return;
 
-       if (is_number_in_set(fd, read_set)) {
-               switch (tcp->s_ent->sen) {
-               case SEN_read:
-               case SEN_pread:
-               case SEN_recv:
-               case SEN_recvfrom:
-               case SEN_mq_timedreceive:
-                       dumpstr(tcp, tcp->u_arg[1], tcp->u_rval);
-                       return;
-               case SEN_readv:
-               case SEN_preadv:
-               case SEN_preadv2:
-                       dumpiov_upto(tcp, tcp->u_arg[2], tcp->u_arg[1],
-                                    tcp->u_rval);
-                       return;
-               case SEN_recvmsg:
-                       dumpiov_in_msghdr(tcp, tcp->u_arg[1], tcp->u_rval);
-                       return;
-               case SEN_recvmmsg:
-                       dumpiov_in_mmsghdr(tcp, tcp->u_arg[1]);
-                       return;
-               }
-       }
        if (is_number_in_set(fd, write_set)) {
                switch (tcp->s_ent->sen) {
                case SEN_write:
@@ -487,6 +461,33 @@ dumpio(struct tcb *tcp)
                        break;
                }
        }
+
+       if (syserror(tcp))
+               return;
+
+       if (is_number_in_set(fd, read_set)) {
+               switch (tcp->s_ent->sen) {
+               case SEN_read:
+               case SEN_pread:
+               case SEN_recv:
+               case SEN_recvfrom:
+               case SEN_mq_timedreceive:
+                       dumpstr(tcp, tcp->u_arg[1], tcp->u_rval);
+                       return;
+               case SEN_readv:
+               case SEN_preadv:
+               case SEN_preadv2:
+                       dumpiov_upto(tcp, tcp->u_arg[2], tcp->u_arg[1],
+                                    tcp->u_rval);
+                       return;
+               case SEN_recvmsg:
+                       dumpiov_in_msghdr(tcp, tcp->u_arg[1], tcp->u_rval);
+                       return;
+               case SEN_recvmmsg:
+                       dumpiov_in_mmsghdr(tcp, tcp->u_arg[1]);
+                       return;
+               }
+       }
 }
 
 const char *

diff --git a/tests/gen_tests.in b/tests/gen_tests.in
index 72ceca4f36beec9f911077e3f51f748e7b6068c1..a95ff3e272d5ab67ce89c406bced9f1168443e23 100644 (file)
--- a/tests/gen_tests.in
+++ b/tests/gen_tests.in
@@ -295,7 +295,7 @@ quotactl
 quotactl-v     -v -e trace=quotactl
 quotactl-xfs   -e trace=quotactl
 quotactl-xfs-v -v -e trace=quotactl
-read-write     -a15 -eread=0 -ewrite=1 -e trace=read,write -P read-write-tmpfile -P /dev/zero -P /dev/null
+read-write     -a15 -eread=0,5 -ewrite=1,4 -e trace=read,write -P read-write-tmpfile -P /dev/zero -P /dev/null
 readahead      -a1
 readdir        -a16
 readlink       -xx

diff --git a/tests/pread64-pwrite64.c b/tests/pread64-pwrite64.c
index e9d7a645b266a18d3f6a36f7d9e5e0e3663483bc..1adb0812d99885f6d3356ee1b3c4d848d83cde99 100644 (file)
--- a/tests/pread64-pwrite64.c
+++ b/tests/pread64-pwrite64.c
@@ -169,6 +169,7 @@ main(void)
        if (rc != -1)
                perror_msg_and_fail("pwrite64: expected -1, returned %ld", rc);
        tprintf("pwrite64(1, \"\\0\", 1, -3) = -1 EINVAL (%m)\n");
+       dump_str(nil, 1);
 
        rc = pwrite(1, w, w_len, 0);
        if (rc != (int) w_len)

diff --git a/tests/read-write.c b/tests/read-write.c
index ed4c64d43bbaeed44d376a7784f8fbcbcd6bf734..2f7a98a03eecc6a1bd4641b2ad5c3f67736c6f73 100644 (file)
--- a/tests/read-write.c
+++ b/tests/read-write.c
@@ -39,7 +39,15 @@
 static void
 dump_str(const char *str, const unsigned int len)
 {
-       static const char dots[16] = "................";
+       static const char chars[256] =
+               "................................"
+               " !\"#$%&'()*+,-./0123456789:;<=>?"
+               "@ABCDEFGHIJKLMNOPQRSTUVWXYZ[\\]^_"
+               "`abcdefghijklmnopqrstuvwxyz{|}~."
+               "................................"
+               "................................"
+               "................................"
+               "................................";
        unsigned int i;
 
        for (i = 0; i < len; i += 16) {
@@ -47,7 +55,7 @@ dump_str(const char *str, const unsigned int len)
                const char *dump = hexdump_memdup(str + i, n);
 
                tprintf(" | %05x %-49s  %-16.*s |\n",
-                       i, dump, n, dots);
+                       i, dump, n, chars + i);
 
                free((void *) dump);
        }
@@ -59,9 +67,16 @@ print_hex(const char *str, const unsigned int len)
        const unsigned char *ustr = (const unsigned char *) str;
        unsigned int i;
 
+       tprintf("\"");
+
        for (i = 0; i < len; ++i) {
                unsigned int c = ustr[i];
 
+               if (i >= DEFAULT_STRLEN) {
+                       tprintf("\"...");
+                       return;
+               }
+
                switch (c) {
                case '\t':
                        tprintf("\\t"); break;
@@ -77,6 +92,8 @@ print_hex(const char *str, const unsigned int len)
                        tprintf("\\%o", ustr[i]);
                }
        }
+
+       tprintf("\"");
 }
 
 static long
@@ -94,9 +111,12 @@ k_write(unsigned int fd, const void *buf, size_t count)
 }
 
 static void
-test_dump(const unsigned int len)
+test_dump(const unsigned int len, bool err_desc)
 {
        static char *buf;
+       const char *rc_str;
+       int in_fd = err_desc ? 5 : 0;
+       int out_fd = err_desc ? 4 : 1;
 
        if (buf) {
                size_t ps1 = get_page_size() - 1;
@@ -105,28 +125,34 @@ test_dump(const unsigned int len)
                buf = tail_alloc(len);
        }
 
-       long rc = k_read(0, buf, len);
-       if (rc != (int) len)
+       long rc = k_read(in_fd, buf, len);
+       rc_str = sprintrc(rc);
+       if (err_desc ^ (rc != (int) len))
                perror_msg_and_fail("read: expected %d, returned %ld",
-                                   len, rc);
+                                   err_desc ? -1 : (int) len, rc);
 
-       tprintf("%s(%d, \"", "read", 0);
-       print_hex(buf, len);
-       tprintf("\", %d) = %ld\n", len, rc);
-       dump_str(buf, len);
+       tprintf("%s(%d, ", "read", in_fd);
+       if (!err_desc)
+               print_hex(buf, len);
+       else
+               tprintf("%p", buf);
+       tprintf(", %d) = %s\n", len, rc_str);
+       if (!err_desc)
+               dump_str(buf, len);
 
        unsigned int i;
        for (i = 0; i < len; ++i)
                buf[i] = i;
 
-       rc = k_write(1, buf, len);
-       if (rc != (int) len)
+       rc = k_write(out_fd, buf, len);
+       rc_str = sprintrc(rc);
+       if (err_desc ^ (rc != (int) len))
                perror_msg_and_fail("write: expected %d, returned %ld",
-                                   len, rc);
+                                   err_desc ? -1 : (int) len, rc);
 
-       tprintf("%s(%d, \"", "write", 1);
+       tprintf("%s(%d, ", "write", out_fd);
        print_hex(buf, len);
-       tprintf("\", %d) = %ld\n", len, rc);
+       tprintf(", %d) = %s\n", len, rc_str);
        dump_str(buf, len);
 
        if (!len)
@@ -218,9 +244,17 @@ main(void)
        if (open("/dev/null", O_WRONLY) != 1)
                perror_msg_and_fail("open");
 
+       if (open("/dev/zero", O_RDONLY) != 4)
+               perror_msg_and_fail("open");
+
+       if (open("/dev/null", O_WRONLY) != 5)
+               perror_msg_and_fail("open");
+
        unsigned int i;
-       for (i = 0; i <= 32; ++i)
-               test_dump(i);
+       for (i = 0; i <= DEFAULT_STRLEN; ++i)
+               test_dump(i, false);
+
+       test_dump(256, true);
 
        tprintf("+++ exited with 0 +++\n");
        return 0;