Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Medium] selftests/proc: fail and leak scsi_debug host without CONFIG_SCSI_PROC_FS --
commit d0cceed0cde591b286c7977ad630183fc653ded2 Author: Christian Brauner <[email protected]> selftests/proc: check that a faulting writer doesn't hold up removal This commit adds a procfs selftest that uses userfaultfd and scsi_debug to verify that a writer faulting during copy-in does not block removal of a /proc/scsi/scsi_debug/<host> entry and fails with -EIO once the fault is resolved. > diff --git a/tools/testing/selftests/proc/proc-remove-slow-writer.c > b/tools/testing/selftests/proc/proc-remove-slow-writer.c > new file mode 100644 > index 0000000000000..f69ca75ba87cb > --- /dev/null > +++ b/tools/testing/selftests/proc/proc-remove-slow-writer.c > @@ -0,0 +1,203 @@ [ ... ] > +/* the numbers of the hosts in /proc/scsi/scsi_debug as a bitmap */ > +static int list_hosts(unsigned long *hosts) > +{ > + struct dirent *d; > + DIR *dir; > + > + memset(hosts, 0, MAX_HOSTS / 8); > + dir = opendir(HOST_DIR); > + if (!dir) > + /* the directory comes with the first host and goes with the > last */ > + return errno == ENOENT ? 0 : -errno; [ ... ] > +FIXTURE_SETUP(host) > +{ > + unsigned long before[MAX_HOSTS / HOST_BITS]; > + unsigned long after[MAX_HOSTS / HOST_BITS]; > + > + self->host = -1; > + if (access(ADD_HOST, W_OK) && system("modprobe -q scsi_debug > add_host=0")) > + SKIP(return, "no scsi_debug"); > + if (access(ADD_HOST, W_OK)) > + SKIP(return, ADD_HOST ": %s", strerror(errno)); > + > + /* host numbers are reused, so look for the one that's new */ > + ASSERT_EQ(list_hosts(before), 0); > + ASSERT_EQ(add_host(1), 0); > + ASSERT_EQ(list_hosts(after), 0); > + self->host = new_host(before, after); > + ASSERT_GE(self->host, 0); > + snprintf(self->path, sizeof(self->path), HOST_DIR "/%d", self->host); > +} > + > +FIXTURE_TEARDOWN(host) > +{ > + if (self->host >= 0 && !access(self->path, F_OK)) > + add_host(-1); > +} [Severity: Medium] What happens if the test runs on a kernel where scsi_debug is available (CONFIG_SCSI_DEBUG is enabled), but CONFIG_SCSI_PROC_FS is disabled? The sysfs attribute /sys/bus/pseudo/drivers/scsi_debug/add_host is still present and writable without CONFIG_SCSI_PROC_FS, while /proc/scsi is never created. In FIXTURE_SETUP(host), list_hosts(before) sees ENOENT on /proc/scsi/scsi_debug and returns 0, add_host(1) creates a new scsi_debug host in the kernel, and list_hosts(after) still sees ENOENT and returns 0 with an empty bitmap. As a result, new_host(before, after) returns -1 and ASSERT_GE(self->host, 0) fails the test instead of skipping for the missing prerequisite. Because FIXTURE_SETUP(host) aborts with self->host still set to -1, the newly created scsi_debug host is never removed by add_host(-1) and remains leaked. Should FIXTURE_SETUP(host) check that /proc/scsi exists and skip before calling add_host(1)? -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=11
