[PATCH blktests v2 0/3] Add SCSI generic test group

[PATCH blktests v2 0/3] Add SCSI generic test group

[Johannes Thumshirn]

Add a test group for the SCSI generic driver and one syzcaller
reproducer for this group.

The reprodcuer is distributed as a C program, so the makefile is
amended to build C files to be used in the test.

Changes to v1:
* Stripped left over TODO comment
* Modified reproducer to accept a device name
* Fixed Makefile so it can build more than one target

Johannes Thumshirn (3):
  Add ability to build test-cases
  tests/sg: add SCSI generic test grouop
  sg/001: add regression test for syzcaller generated GPF in sg_read
    path

 Makefile         |  26 +++-
 common/sg        |  41 ++++++
 src/.gitignore   |   1 +
 src/Makefile     |  16 ++
 src/sg-001.c     | 438 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 tests/sg/001     |  47 ++++++
 tests/sg/001.out |   2 +
 tests/sg/group   |  28 ++++
 8 files changed, 598 insertions(+), 1 deletion(-)
 create mode 100644 common/sg
 create mode 100644 src/.gitignore
 create mode 100644 src/Makefile
 create mode 100644 src/sg-001.c
 create mode 100755 tests/sg/001
 create mode 100644 tests/sg/001.out
 create mode 100644 tests/sg/group

-- 
2.12.0

[PATCH blktests v2 1/3] Add ability to build test-cases

[Johannes Thumshirn]

Add the ability to build test cases from C files. This is handy for
things like syzcaller reproducers and all other kinds of test
binaries.

Signed-off-by: Johannes Thumshirn <jthumshirn@suse.de>
---
 Makefile       |  26 +++-
 src/.gitignore |   1 +
 src/Makefile   |  16 +++
 src/sg-001.c   | 438 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 480 insertions(+), 1 deletion(-)
 create mode 100644 src/.gitignore
 create mode 100644 src/Makefile
 create mode 100644 src/sg-001.c

diff --git a/Makefile b/Makefile
index 3a0f0100232c..a70623dd4a52 100644
--- a/Makefile
+++ b/Makefile
@@ -1,7 +1,31 @@
+ifeq ("$(origin V)", "command line")
+	BUILD_VERBOSE = $(V)
+else
+	BUILD_VERBOSE = 0
+endif
+ifndef BUILD_VERBOSE
+	BUILD_VERBOSE = 0
+endif
+
+ifeq ($(BUILD_VERBOSE),1)
+	Q =
+else
+	Q = @
+endif
+
+MAKEOPTS = --no-print-directory Q=$(Q)
+
+SUBDIRS = src
+default:
+	$(Q)$(MAKE) $(MAKEOPTS) -C $(SUBDIRS)
+
+clean:
+	$(Q)$(MAKE) $(MAKEOPTS) -C $(SUBDIRS) clean
+
 all:
 	@echo "Please read README.md"
 
 shellcheck:
 	shellcheck -x -f gcc check new common/* tests/*/[0-9]*[0-9]
 
-.PHONY: all shellcheck
+.PHONY: all shellcheck clean
diff --git a/src/.gitignore b/src/.gitignore
new file mode 100644
index 000000000000..f543ddb9280f
--- /dev/null
+++ b/src/.gitignore
@@ -0,0 +1 @@
+sg-001
diff --git a/src/Makefile b/src/Makefile
new file mode 100644
index 000000000000..d05b625cc7f8
--- /dev/null
+++ b/src/Makefile
@@ -0,0 +1,16 @@
+CC = gcc
+CFLAGS = -O2
+
+TARGETS = sg-001
+
+FILES = $(TARGETS:=.c)
+
+all: $(TARGETS)
+
+$(TARGETS): $(FILES)
+	@echo "    [CC]    $@"
+	$(Q)$(CC) $@.c -o $@ $(CFLAGS)
+
+clean:
+	@echo "    [CLEAN]  $(notdir $(CURDIR))"
+	$(Q)rm -f $(TARGETS) *.o
diff --git a/src/sg-001.c b/src/sg-001.c
new file mode 100644
index 000000000000..1dabba7d5ae8
--- /dev/null
+++ b/src/sg-001.c
@@ -0,0 +1,438 @@
+// autogenerated by syzkaller (http://github.com/google/syzkaller)
+
+#ifndef __NR_read
+#define __NR_read 0
+#endif
+#ifndef __NR_mmap
+#define __NR_mmap 9
+#endif
+#ifndef __NR_syz_open_dev
+#define __NR_syz_open_dev 1000002
+#endif
+#ifndef __NR_write
+#define __NR_write 1
+#endif
+
+#define _GNU_SOURCE
+
+#include <sys/ioctl.h>
+#include <sys/mman.h>
+#include <sys/mount.h>
+#include <sys/prctl.h>
+#include <sys/resource.h>
+#include <sys/socket.h>
+#include <sys/stat.h>
+#include <sys/syscall.h>
+#include <sys/time.h>
+#include <sys/types.h>
+#include <sys/wait.h>
+
+#include <arpa/inet.h>
+#include <linux/capability.h>
+#include <linux/if.h>
+#include <linux/if_ether.h>
+#include <linux/if_tun.h>
+#include <linux/ip.h>
+#include <linux/kvm.h>
+#include <linux/sched.h>
+#include <linux/tcp.h>
+#include <net/if_arp.h>
+
+#include <assert.h>
+#include <dirent.h>
+#include <errno.h>
+#include <fcntl.h>
+#include <grp.h>
+#include <pthread.h>
+#include <setjmp.h>
+#include <signal.h>
+#include <stdarg.h>
+#include <stdbool.h>
+#include <stddef.h>
+#include <stdint.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <unistd.h>
+
+const int kFailStatus = 67;
+const int kErrorStatus = 68;
+const int kRetryStatus = 69;
+
+const char *dev_sg;
+
+__attribute__((noreturn)) void doexit(int status)
+{
+  volatile unsigned i;
+  syscall(__NR_exit_group, status);
+  for (i = 0;; i++) {
+  }
+}
+
+__attribute__((noreturn)) void fail(const char* msg, ...)
+{
+  int e = errno;
+  fflush(stdout);
+  va_list args;
+  va_start(args, msg);
+  vfprintf(stderr, msg, args);
+  va_end(args);
+  fprintf(stderr, " (errno %d)\n", e);
+  doexit((e == ENOMEM || e == EAGAIN) ? kRetryStatus : kFailStatus);
+}
+
+__attribute__((noreturn)) void exitf(const char* msg, ...)
+{
+  int e = errno;
+  fflush(stdout);
+  va_list args;
+  va_start(args, msg);
+  vfprintf(stderr, msg, args);
+  va_end(args);
+  fprintf(stderr, " (errno %d)\n", e);
+  doexit(kRetryStatus);
+}
+
+static int flag_debug;
+
+void debug(const char* msg, ...)
+{
+  if (!flag_debug)
+    return;
+  va_list args;
+  va_start(args, msg);
+  vfprintf(stdout, msg, args);
+  va_end(args);
+  fflush(stdout);
+}
+
+__thread int skip_segv;
+__thread jmp_buf segv_env;
+
+static void segv_handler(int sig, siginfo_t* info, void* uctx)
+{
+  uintptr_t addr = (uintptr_t)info->si_addr;
+  const uintptr_t prog_start = 1 << 20;
+  const uintptr_t prog_end = 100 << 20;
+  if (__atomic_load_n(&skip_segv, __ATOMIC_RELAXED) &&
+      (addr < prog_start || addr > prog_end)) {
+    debug("SIGSEGV on %p, skipping\n", addr);
+    _longjmp(segv_env, 1);
+  }
+  debug("SIGSEGV on %p, exiting\n", addr);
+  doexit(sig);
+  for (;;) {
+  }
+}
+
+static void install_segv_handler()
+{
+  struct sigaction sa;
+  memset(&sa, 0, sizeof(sa));
+  sa.sa_sigaction = segv_handler;
+  sa.sa_flags = SA_NODEFER | SA_SIGINFO;
+  sigaction(SIGSEGV, &sa, NULL);
+  sigaction(SIGBUS, &sa, NULL);
+}
+
+#define NONFAILING(...)                                                \
+  {                                                                    \
+    __atomic_fetch_add(&skip_segv, 1, __ATOMIC_SEQ_CST);               \
+    if (_setjmp(segv_env) == 0) {                                      \
+      __VA_ARGS__;                                                     \
+    }                                                                  \
+    __atomic_fetch_sub(&skip_segv, 1, __ATOMIC_SEQ_CST);               \
+  }
+
+#define BITMASK_LEN(type, bf_len) (type)((1ull << (bf_len)) - 1)
+
+#define BITMASK_LEN_OFF(type, bf_off, bf_len)                          \
+  (type)(BITMASK_LEN(type, (bf_len)) << (bf_off))
+
+#define STORE_BY_BITMASK(type, addr, val, bf_off, bf_len)              \
+  if ((bf_off) == 0 && (bf_len) == 0) {                                \
+    *(type*)(addr) = (type)(val);                                      \
+  } else {                                                             \
+    type new_val = *(type*)(addr);                                     \
+    new_val &= ~BITMASK_LEN_OFF(type, (bf_off), (bf_len));             \
+    new_val |= ((type)(val)&BITMASK_LEN(type, (bf_len))) << (bf_off);  \
+    *(type*)(addr) = new_val;                                          \
+  }
+
+struct csum_inet {
+  uint32_t acc;
+};
+
+void csum_inet_init(struct csum_inet* csum)
+{
+  csum->acc = 0;
+}
+
+void csum_inet_update(struct csum_inet* csum, const uint8_t* data,
+                      size_t length)
+{
+  if (length == 0)
+    return;
+
+  size_t i;
+  for (i = 0; i < length - 1; i += 2)
+    csum->acc += *(uint16_t*)&data[i];
+
+  if (length & 1)
+    csum->acc += (uint16_t)data[length - 1];
+
+  while (csum->acc > 0xffff)
+    csum->acc = (csum->acc & 0xffff) + (csum->acc >> 16);
+}
+
+uint16_t csum_inet_digest(struct csum_inet* csum)
+{
+  return ~csum->acc;
+}
+
+static uintptr_t syz_open_dev(uintptr_t a0, uintptr_t a1, uintptr_t a2)
+{
+  if (a0 == 0xc || a0 == 0xb) {
+    char buf[128];
+    sprintf(buf, "/dev/%s/%d:%d", a0 == 0xc ? "char" : "block",
+            (uint8_t)a1, (uint8_t)a2);
+    return open(buf, O_RDWR, 0);
+  } else {
+    char buf[1024];
+    char* hash;
+    NONFAILING(strncpy(buf, (char*)a0, sizeof(buf)));
+    buf[sizeof(buf) - 1] = 0;
+    while ((hash = strchr(buf, '#'))) {
+      *hash = '0' + (char)(a1 % 10);
+      a1 /= 10;
+    }
+    return open(buf, a2, 0);
+  }
+}
+
+static uintptr_t execute_syscall(int nr, uintptr_t a0, uintptr_t a1,
+                                 uintptr_t a2, uintptr_t a3,
+                                 uintptr_t a4, uintptr_t a5,
+                                 uintptr_t a6, uintptr_t a7,
+                                 uintptr_t a8)
+{
+  switch (nr) {
+  default:
+    return syscall(nr, a0, a1, a2, a3, a4, a5);
+  case __NR_syz_open_dev:
+    return syz_open_dev(a0, a1, a2);
+  }
+}
+
+static void setup_main_process()
+{
+  struct sigaction sa;
+  memset(&sa, 0, sizeof(sa));
+  sa.sa_handler = SIG_IGN;
+  syscall(SYS_rt_sigaction, 0x20, &sa, NULL, 8);
+  syscall(SYS_rt_sigaction, 0x21, &sa, NULL, 8);
+  install_segv_handler();
+
+  char tmpdir_template[] = "./syzkaller.XXXXXX";
+  char* tmpdir = mkdtemp(tmpdir_template);
+  if (!tmpdir)
+    fail("failed to mkdtemp");
+  if (chmod(tmpdir, 0777))
+    fail("failed to chmod");
+  if (chdir(tmpdir))
+    fail("failed to chdir");
+}
+
+static void loop();
+
+static void sandbox_common()
+{
+  prctl(PR_SET_PDEATHSIG, SIGKILL, 0, 0, 0);
+  setpgrp();
+  setsid();
+
+  struct rlimit rlim;
+  rlim.rlim_cur = rlim.rlim_max = 128 << 20;
+  setrlimit(RLIMIT_AS, &rlim);
+  rlim.rlim_cur = rlim.rlim_max = 1 << 20;
+  setrlimit(RLIMIT_FSIZE, &rlim);
+  rlim.rlim_cur = rlim.rlim_max = 1 << 20;
+  setrlimit(RLIMIT_STACK, &rlim);
+  rlim.rlim_cur = rlim.rlim_max = 0;
+  setrlimit(RLIMIT_CORE, &rlim);
+
+  unshare(CLONE_NEWNS);
+  unshare(CLONE_NEWIPC);
+  unshare(CLONE_IO);
+}
+
+static int do_sandbox_none(int executor_pid, bool enable_tun)
+{
+  int pid = fork();
+  if (pid)
+    return pid;
+
+  sandbox_common();
+
+  loop();
+  doexit(1);
+}
+
+static void remove_dir(const char* dir)
+{
+  DIR* dp;
+  struct dirent* ep;
+  int iter = 0;
+retry:
+  dp = opendir(dir);
+  if (dp == NULL) {
+    if (errno == EMFILE) {
+      exitf("opendir(%s) failed due to NOFILE, exiting");
+    }
+    exitf("opendir(%s) failed", dir);
+  }
+  while ((ep = readdir(dp))) {
+    if (strcmp(ep->d_name, ".") == 0 || strcmp(ep->d_name, "..") == 0)
+      continue;
+    char filename[FILENAME_MAX];
+    snprintf(filename, sizeof(filename), "%s/%s", dir, ep->d_name);
+    struct stat st;
+    if (lstat(filename, &st))
+      exitf("lstat(%s) failed", filename);
+    if (S_ISDIR(st.st_mode)) {
+      remove_dir(filename);
+      continue;
+    }
+    int i;
+    for (i = 0;; i++) {
+      debug("unlink(%s)\n", filename);
+      if (unlink(filename) == 0)
+        break;
+      if (errno == EROFS) {
+        debug("ignoring EROFS\n");
+        break;
+      }
+      if (errno != EBUSY || i > 100)
+        exitf("unlink(%s) failed", filename);
+      debug("umount(%s)\n", filename);
+      if (umount2(filename, MNT_DETACH))
+        exitf("umount(%s) failed", filename);
+    }
+  }
+  closedir(dp);
+  int i;
+  for (i = 0;; i++) {
+    debug("rmdir(%s)\n", dir);
+    if (rmdir(dir) == 0)
+      break;
+    if (i < 100) {
+      if (errno == EROFS) {
+        debug("ignoring EROFS\n");
+        break;
+      }
+      if (errno == EBUSY) {
+        debug("umount(%s)\n", dir);
+        if (umount2(dir, MNT_DETACH))
+          exitf("umount(%s) failed", dir);
+        continue;
+      }
+      if (errno == ENOTEMPTY) {
+        if (iter < 100) {
+          iter++;
+          goto retry;
+        }
+      }
+    }
+    exitf("rmdir(%s) failed", dir);
+  }
+}
+
+static uint64_t current_time_ms()
+{
+  struct timespec ts;
+
+  if (clock_gettime(CLOCK_MONOTONIC, &ts))
+    fail("clock_gettime failed");
+  return (uint64_t)ts.tv_sec * 1000 + (uint64_t)ts.tv_nsec / 1000000;
+}
+
+static void test();
+
+void loop()
+{
+  int iter;
+  for (iter = 0;; iter++) {
+    char cwdbuf[256];
+    sprintf(cwdbuf, "./%d", iter);
+    if (mkdir(cwdbuf, 0777))
+      fail("failed to mkdir");
+    int pid = fork();
+    if (pid < 0)
+      fail("clone failed");
+    if (pid == 0) {
+      prctl(PR_SET_PDEATHSIG, SIGKILL, 0, 0, 0);
+      setpgrp();
+      if (chdir(cwdbuf))
+        fail("failed to chdir");
+      test();
+      doexit(0);
+    }
+    int status = 0;
+    uint64_t start = current_time_ms();
+    for (;;) {
+      int res = waitpid(-1, &status, __WALL | WNOHANG);
+      if (res == pid)
+        break;
+      usleep(1000);
+      if (current_time_ms() - start > 5 * 1000) {
+        kill(-pid, SIGKILL);
+        kill(pid, SIGKILL);
+        while (waitpid(-1, &status, __WALL) != pid) {
+        }
+        break;
+      }
+    }
+    remove_dir(cwdbuf);
+  }
+}
+
+long r[15];
+void test()
+{
+  memset(r, -1, sizeof(r));
+  r[0] = execute_syscall(__NR_mmap, 0x20000000ul, 0x5000ul, 0x3ul,
+                         0x32ul, 0xfffffffffffffffful, 0x0ul, 0, 0, 0);
+  NONFAILING(memcpy((void*)0x20000000,
+//                    "\x2f\x64\x65\x76\x2f\x73\x67\x23\x00", 9));
+                    dev_sg, strlen(dev_sg)));
+  r[2] = execute_syscall(__NR_syz_open_dev, 0x20000000ul, 0x0ul, 0x2ul,
+                         0, 0, 0, 0, 0, 0);
+  NONFAILING(*(uint64_t*)0x20001fdc = (uint64_t)0x0);
+  NONFAILING(*(uint64_t*)0x20001fe4 = (uint64_t)0x0);
+  NONFAILING(*(uint16_t*)0x20001fec = (uint16_t)0x3);
+  NONFAILING(*(uint16_t*)0x20001fee = (uint16_t)0x4);
+  NONFAILING(*(uint32_t*)0x20001ff0 = (uint32_t)0x9);
+  NONFAILING(*(uint64_t*)0x20001ff4 = (uint64_t)0x0);
+  NONFAILING(*(uint64_t*)0x20001ffc = (uint64_t)0x2710);
+  NONFAILING(*(uint16_t*)0x20002004 = (uint16_t)0x7);
+  NONFAILING(*(uint16_t*)0x20002006 = (uint16_t)0x7);
+  NONFAILING(*(uint32_t*)0x20002008 = (uint32_t)0x8);
+  r[13] = execute_syscall(__NR_write, r[2], 0x20001fdcul, 0x30ul, 0, 0,
+                          0, 0, 0, 0);
+  r[14] = execute_syscall(__NR_read, r[2], 0x20002f36ul, 0x0ul, 0, 0, 0,
+                          0, 0, 0);
+}
+int main(int argc, char **argv)
+{
+  if (argc == 2)
+    dev_sg = strdup(argv[1]);
+  else
+    dev_sg = "/dev/sg0";
+
+  setup_main_process();
+  int pid = do_sandbox_none(0, false);
+  int status = 0;
+  while (waitpid(pid, &status, __WALL) != pid) {
+  }
+  return 0;
+}
-- 
2.12.0

[PATCH blktests v2 2/3] tests/sg: add SCSI generic test grouop

[Johannes Thumshirn]

Add a test group for tests of the SCSI generic driver and and
functions common to the SCSI generic driver and it's test cases.

Signed-off-by: Johannes Thumshirn <jthumshirn@suse.de>
---
 common/sg      | 41 +++++++++++++++++++++++++++++++++++++++++
 tests/sg/group | 28 ++++++++++++++++++++++++++++
 2 files changed, 69 insertions(+)
 create mode 100644 common/sg
 create mode 100644 tests/sg/group

diff --git a/common/sg b/common/sg
new file mode 100644
index 000000000000..123735178d96
--- /dev/null
+++ b/common/sg
@@ -0,0 +1,41 @@
+#!/bin/bash
+#
+# SCSI generic helper functions.
+#
+# Copyright (C) 2017 Johannes Thumshirn
+#
+# This program is free software: you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation, either version 3 of the License, or
+# (at your option) any later version.
+#
+# 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, see <http://www.gnu.org/licenses/>.
+
+_have_scsi_generic() {
+	_have_module sg
+}
+
+_test_dev_is_scsi() {
+
+	local DEV="$1"
+
+	if [[ -d "/sys/block/$DEV/device/scsi_device" ]]; then
+		return 0
+	else
+		return 1
+	fi
+}
+
+_get_sg_from_blockdev()
+{
+	local sg_path="/sys/block/$1/device/scsi_generic/"
+	local sg_dev=$(ls $sg_path | grep -E 'sg[0-9]+') 
+
+	echo "$sg_dev"
+}
diff --git a/tests/sg/group b/tests/sg/group
new file mode 100644
index 000000000000..3121d799fe25
--- /dev/null
+++ b/tests/sg/group
@@ -0,0 +1,28 @@
+#!/bin/bash
+#
+# Regression tests for SCSI generic device
+#
+# Copyright (C) 2017 Johannes Thumshirn <jthumshirn@suse.de>
+#
+# This program is free software: you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation, either version 3 of the License, or
+# (at your option) any later version.
+#
+# 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, see <http://www.gnu.org/licenses/>.
+
+. common/sg
+
+group_requires() {
+	_have_scsi_generic
+}
+
+group_device_requires() {
+	_test_dev_is_scsi
+}
-- 
2.12.0

[PATCH blktests v2 3/3] sg/001: add regression test for syzcaller generated GPF in sg_read path

[Johannes Thumshirn]

Add a regression test for commit 48ae8484e9fc ("scsi: sg: don't return
bogus Sg_requests"). This is a general protection fault triggered by
syzcaller via issuing bogus read(2)s on the /dev/sg devices.

Signed-off-by: Johannes Thumshirn <jthumshirn@suse.de>
---
 tests/sg/001     | 47 +++++++++++++++++++++++++++++++++++++++++++++++
 tests/sg/001.out |  2 ++
 2 files changed, 49 insertions(+)
 create mode 100755 tests/sg/001
 create mode 100644 tests/sg/001.out

diff --git a/tests/sg/001 b/tests/sg/001
new file mode 100755
index 000000000000..86430409b6a3
--- /dev/null
+++ b/tests/sg/001
@@ -0,0 +1,47 @@
+#!/bin/bash
+#
+# Regression test for commit 48ae8484e9fc ("scsi: sg: don't return bogus
+# Sg_requests")
+#
+# Copyright (C) 2017 Johannes Thumshirn <jthumshirn@suse.de>
+#
+# This program is free software: you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation, either version 3 of the License, or
+# (at your option) any later version.
+#
+# 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, see <http://www.gnu.org/licenses/>.
+
+. common/sg
+. common/scsi_debug
+
+DESCRIPTION="try triggering a kernel GPF with 0 byte SG reads"
+QUICK=1
+
+requires() {
+	_have_program src/sg-001 \
+	    && _have_scsi_debug \
+	    && _have_scsi_generic
+}
+
+
+test() {
+	echo "Running ${TEST_NAME}"
+
+	if ! _get_scsi_debug_dev; then
+	    return 1
+	fi
+
+	SG_DEV=$(_get_sg_from_blockdev "$SCSI_DEBUG_NAME")
+	timeout -s INT 10s ./src/sg-001 "$SG_DEV"
+
+	_put_scsi_debug_dev
+
+	echo "Test complete"
+}
diff --git a/tests/sg/001.out b/tests/sg/001.out
new file mode 100644
index 000000000000..beb4c437dd28
--- /dev/null
+++ b/tests/sg/001.out
@@ -0,0 +1,2 @@
+Running sg/001
+Test complete
-- 
2.12.0

Re: [PATCH blktests v2 3/3] sg/001: add regression test for syzcaller generated GPF in sg_read path

[Omar Sandoval]

On Fri, May 19, 2017 at 03:55:31PM +0200, Johannes Thumshirn wrote:
> Add a regression test for commit 48ae8484e9fc ("scsi: sg: don't return
> bogus Sg_requests"). This is a general protection fault triggered by
> syzcaller via issuing bogus read(2)s on the /dev/sg devices.
> 
> Signed-off-by: Johannes Thumshirn <jthumshirn@suse.de>
> ---
>  tests/sg/001     | 47 +++++++++++++++++++++++++++++++++++++++++++++++
>  tests/sg/001.out |  2 ++
>  2 files changed, 49 insertions(+)
>  create mode 100755 tests/sg/001
>  create mode 100644 tests/sg/001.out
> 
> diff --git a/tests/sg/001 b/tests/sg/001
> new file mode 100755
> index 000000000000..86430409b6a3
> --- /dev/null
> +++ b/tests/sg/001
> @@ -0,0 +1,47 @@
> +#!/bin/bash
> +#
> +# Regression test for commit 48ae8484e9fc ("scsi: sg: don't return bogus
> +# Sg_requests")
> +#
> +# Copyright (C) 2017 Johannes Thumshirn <jthumshirn@suse.de>
> +#
> +# This program is free software: you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation, either version 3 of the License, or
> +# (at your option) any later version.
> +#
> +# 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, see <http://www.gnu.org/licenses/>.
> +
> +. common/sg
> +. common/scsi_debug
> +
> +DESCRIPTION="try triggering a kernel GPF with 0 byte SG reads"
> +QUICK=1
> +
> +requires() {
> +	_have_program src/sg-001 \
> +	    && _have_scsi_debug \
> +	    && _have_scsi_generic
> +}
> +
> +
> +test() {
> +	echo "Running ${TEST_NAME}"
> +
> +	if ! _get_scsi_debug_dev; then
> +	    return 1
> +	fi
> +
> +	SG_DEV=$(_get_sg_from_blockdev "$SCSI_DEBUG_NAME")
> +	timeout -s INT 10s ./src/sg-001 "$SG_DEV"
> +
> +	_put_scsi_debug_dev
> +
> +	echo "Test complete"
> +}

This looks much better, thanks! One question for you: is there any value
in running this on specific test devices (i.e., changing test() to
test_device() and using "$TEST_DEV" instead of a scsi-debug device), or
would it be a waste of time since it's just exercising generic code?

Re: [PATCH blktests v2 3/3] sg/001: add regression test for syzcaller generated GPF in sg_read path

[Johannes Thumshirn]

On 05/22/2017 07:59 PM, Omar Sandoval wrote:
> This looks much better, thanks! One question for you: is there any value
> in running this on specific test devices (i.e., changing test() to
> test_device() and using "$TEST_DEV" instead of a scsi-debug device), or
> would it be a waste of time since it's just exercising generic code?

That's just generic code. All I need is a SCSI device so I get a /dev/sg
device node.

One could do a check if $TEST_DEV is a SCSI device and have a fall-back
to scsi_debug if it isn't, but I'm not sure if this isn't just a waste
of time.

Byte,
	Johannes

-- 
Johannes Thumshirn                                          Storage
jthumshirn@suse.de                                +49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850

Re: [PATCH blktests v2 3/3] sg/001: add regression test for syzcaller generated GPF in sg_read path

[Jens Axboe]

On 05/23/2017 12:58 AM, Johannes Thumshirn wrote:
> On 05/22/2017 07:59 PM, Omar Sandoval wrote:
>> This looks much better, thanks! One question for you: is there any value
>> in running this on specific test devices (i.e., changing test() to
>> test_device() and using "$TEST_DEV" instead of a scsi-debug device), or
>> would it be a waste of time since it's just exercising generic code?
> 
> That's just generic code. All I need is a SCSI device so I get a /dev/sg
> device node.
> 
> One could do a check if $TEST_DEV is a SCSI device and have a fall-back
> to scsi_debug if it isn't, but I'm not sure if this isn't just a waste
> of time.

Add some code to the framework that allows you to get the corresponding
SG device for a SCSI block device? Make that part of the prepare, skip
the test if the block device isn't a SCSI dev.


-- 
Jens Axboe

Re: [PATCH blktests v2 3/3] sg/001: add regression test for syzcaller generated GPF in sg_read path

[Johannes Thumshirn]

On 05/23/2017 04:15 PM, Jens Axboe wrote:
> Add some code to the framework that allows you to get the corresponding
> SG device for a SCSI block device? Make that part of the prepare, skip
> the test if the block device isn't a SCSI dev.

Well the code is already there (in patch 2/3).

I'll pack it into the prepare stage.

-- 
Johannes Thumshirn                                          Storage
jthumshirn@suse.de                                +49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850

Re: [PATCH blktests v2 3/3] sg/001: add regression test for syzcaller generated GPF in sg_read path

[Jens Axboe]

On 05/23/2017 08:25 AM, Johannes Thumshirn wrote:
> On 05/23/2017 04:15 PM, Jens Axboe wrote:
>> Add some code to the framework that allows you to get the corresponding
>> SG device for a SCSI block device? Make that part of the prepare, skip
>> the test if the block device isn't a SCSI dev.
> 
> Well the code is already there (in patch 2/3).
> 
> I'll pack it into the prepare stage.

I tried to look up that commit:

 48ae8484e9fc ("scsi: sg: don't return bogus Sg_requests")

but that isn't in Linus' tree. Even searched for just the title, still
didn't find anything.

I'm assuming this is a bug in the sg.c driver, in which case the 2/3
prep and real test case looks fine. For generic device testing, we
should just use SG_IO and not bother with sg.c at all. The world would
be a better place if we could just get rid of sg.c...

-- 
Jens Axboe

Re: [PATCH blktests v2 3/3] sg/001: add regression test for syzcaller generated GPF in sg_read path

[Johannes Thumshirn]

On 05/23/2017 04:39 PM, Jens Axboe wrote:
> I tried to look up that commit:
> 
>  48ae8484e9fc ("scsi: sg: don't return bogus Sg_requests")
> 
> but that isn't in Linus' tree. Even searched for just the title, still
> didn't find anything.

It's queued up in Martin's tree [1].

> 
> I'm assuming this is a bug in the sg.c driver, in which case the 2/3
> prep and real test case looks fine. For generic device testing, we
> should just use SG_IO and not bother with sg.c at all. The world would
> be a better place if we could just get rid of sg.c...

Agreed. Yes the bug is in the sg.c driver and we did have quite some of
these lately thanks to the syzcaller folks.

My intention with these tests was to have a place where we can throw in
the syzcaller reproducers and run it in nicely Qemu.

[1]
https://git.kernel.org/pub/scm/linux/kernel/git/mkp/scsi.git/commit/?h=4.12/scsi-fixes&id=48ae8484e9fc324b4968d33c585e54bc98e44d61

-- 
Johannes Thumshirn                                          Storage
jthumshirn@suse.de                                +49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850

Re: [PATCH blktests v2 0/3] Add SCSI generic test group

[Omar Sandoval]

On Fri, May 19, 2017 at 03:55:28PM +0200, Johannes Thumshirn wrote:
> Add a test group for the SCSI generic driver and one syzcaller
> reproducer for this group.
> 
> The reprodcuer is distributed as a C program, so the makefile is
> amended to build C files to be used in the test.
> 
> Changes to v1:
> * Stripped left over TODO comment
> * Modified reproducer to accept a device name
> * Fixed Makefile so it can build more than one target
> 
> Johannes Thumshirn (3):
>   Add ability to build test-cases
>   tests/sg: add SCSI generic test grouop
>   sg/001: add regression test for syzcaller generated GPF in sg_read
>     path

Hey, Johannes,

Sorry for the delay, I was chasing down a few Btrfs bugs for the last
couple of weeks. I applied these with some small changes. I stuck with
running syzkaller on the scsi-debug device like you had here, we can
revisit that for future tests if it makes sense to run them on a real
device. Thanks!

Re: [PATCH blktests v2 0/3] Add SCSI generic test group

[Johannes Thumshirn]

On 06/07/2017 08:44 AM, Omar Sandoval wrote:
> Sorry for the delay, I was chasing down a few Btrfs bugs for the last
> couple of weeks. I applied these with some small changes. I stuck with
> running syzkaller on the scsi-debug device like you had here, we can
> revisit that for future tests if it makes sense to run them on a real
> device. Thanks!

Yes I wanted to go to it tomorrow and send you a v3, I got stuck in NVMe
;-). Thanks for applying it anyways, I'll send a follow up.

	Johannes
-- 
Johannes Thumshirn                                          Storage
jthumshirn@suse.de                                +49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850