Re: [LTP] [PATCH 1/2] ioctl_sg01: Skip USB devices

Petr Vorel
2025-10-22T13:13:52Z

Hi Martin,

> Some USB devices write hardware info and flags to the ioctl(SG_IO)
> response buffer which results in test failure. But the info is constant
> and doesn't represent any security risk. Skip USB devices to prevent
> false positives.

> ---

> I've tested this patch on kernels v4.4 through v6.16. Non-USB generic SCSI
> block devices get correctly found and used, USB block device get skipped.

Thanks a lot for an extensive testing!

I was also verify on my machine with block device connected over USB
that it's skipped (and indeed test was blocked on master).

Tested-by: Petr Vorel <pvorel@suse.cz>
Reviewed-by: Petr Vorel <pvorel@suse.cz>

Few notes below.

> testcases/kernel/syscalls/ioctl/ioctl_sg01.c | 55 +++++++++++++++-----
> 1 file changed, 42 insertions(+), 13 deletions(-)

> diff --git a/testcases/kernel/syscalls/ioctl/ioctl_sg01.c b/testcases/kernel/syscalls/ioctl/ioctl_sg01.c
> index fba3816c3..66ff980ce 100644
> --- a/testcases/kernel/syscalls/ioctl/ioctl_sg01.c
> +++ b/testcases/kernel/syscalls/ioctl/ioctl_sg01.c
> @@ -29,6 +29,9 @@
> #include "tst_test.h"
> #include "tst_memutils.h"

> +#define SYSDIR "/sys/block"
> +#define BLOCKDIR "/sys/block/%s/device"
> +
> #define BUF_SIZE (128 * 4096)
> #define CMD_SIZE 6

> @@ -38,42 +41,68 @@ static unsigned char command[CMD_SIZE];
> static struct sg_io_hdr query;

> /* TODO: split this off to a separate SCSI library? */
> -static const char *find_generic_scsi_device(int access_flags)
> +static const char *find_generic_scsi_device(int access_flags, int skip_usb)
> {
> - DIR *devdir;
> + DIR *sysdir;
> struct dirent *ent;
> int tmpfd;
> - static char devpath[PATH_MAX];
> + ssize_t length;
> + char *filename;
> + static char devpath[PATH_MAX], syspath[PATH_MAX];

> - errno = 0;
> - devdir = opendir("/dev");
> + sysdir = opendir(SYSDIR);

> - if (!devdir)
> + if (!sysdir)
> return NULL;

> - while ((ent = SAFE_READDIR(devdir))) {
> - /* The bug is most likely reproducible only on /dev/sg* */
> - if (strncmp(ent->d_name, "sg", 2) || !isdigit(ent->d_name[2]))

The kernel fix was done in drivers/scsi/sg.c, it made sense to check it.
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=a45b599ad808

> + /* Scan block devices */
> + while ((ent = SAFE_READDIR(sysdir))) {
> + if (ent->d_name[0] == '.')
> + continue;
> +
> + snprintf(syspath, PATH_MAX, BLOCKDIR, ent->d_name);
> + syspath[PATH_MAX - 1] = '\0';
> +
> + /* Real device path matches the physical HW bus path */
> + if (!realpath(syspath, devpath))
> + continue;
> +
> + strncat(devpath, "/generic", PATH_MAX - strlen(devpath) - 1);

On one baremetal machine and on VM with added SCSI device this approach really
works (anything "/generic" was actually pointing to scsi_generic/sg*.

> + devpath[PATH_MAX - 1] = '\0';
> + length = readlink(devpath, syspath, PATH_MAX - 1);
> +
> + if (length < 0)
> + continue;
> +
> + syspath[length] = '\0';
> + filename = basename(syspath);
> +
> + /* USB devices often return HW info in SG_IO response buffer */
> + if (skip_usb && strstr(devpath, "/usb")) {

very nit: I would personally avoid skip_usb variable because it is always 1
(skip unconditionally). Or actually allow to set it via getopts.

Kind regards,
Petr

...
> static void setup(void)
> {
> - const char *devpath = find_generic_scsi_device(O_RDONLY);
> + const char *devpath = find_generic_scsi_device(O_RDONLY, 1);