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

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

[Martin Doucha]

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.

Signed-off-by: Martin Doucha <mdoucha@suse.cz>
---

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.

 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]))
+	/* 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);
+		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")) {
+			tst_res(TINFO, "Skipping USB device %s", filename);
 			continue;
+		}
 
-		snprintf(devpath, PATH_MAX, "/dev/%s", ent->d_name);
+		snprintf(devpath, PATH_MAX, "/dev/%s", filename);
 		/* access() makes incorrect assumptions about block devices */
 		tmpfd = open(devpath, access_flags);
 
 		if (tmpfd >= 0) {
 			SAFE_CLOSE(tmpfd);
-			SAFE_CLOSEDIR(devdir);
+			SAFE_CLOSEDIR(sysdir);
 			return devpath;
 		}
+
+		tst_res(TINFO | TERRNO, "Cannot open device %s", devpath);
 	}
 
-	SAFE_CLOSEDIR(devdir);
+	SAFE_CLOSEDIR(sysdir);
 	return NULL;
 }
 
 static void setup(void)
 {
-	const char *devpath = find_generic_scsi_device(O_RDONLY);
+	const char *devpath = find_generic_scsi_device(O_RDONLY, 1);
 
 	if (!devpath)
 		tst_brk(TCONF, "Could not find any usable SCSI device");
-- 
2.51.0


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

[LTP] [PATCH 2/2] ioctl_sg01: Print buffer contents on failure

[Martin Doucha]

Signed-off-by: Martin Doucha <mdoucha@suse.cz>
---

This patch is a convenience feature. Feel free to reject it.

 testcases/kernel/syscalls/ioctl/ioctl_sg01.c | 24 +++++++++++++++++++-
 1 file changed, 23 insertions(+), 1 deletion(-)

diff --git a/testcases/kernel/syscalls/ioctl/ioctl_sg01.c b/testcases/kernel/syscalls/ioctl/ioctl_sg01.c
index 66ff980ce..dada174e0 100644
--- a/testcases/kernel/syscalls/ioctl/ioctl_sg01.c
+++ b/testcases/kernel/syscalls/ioctl/ioctl_sg01.c
@@ -36,7 +36,7 @@
 #define CMD_SIZE 6
 
 static int devfd = -1;
-static char buffer[BUF_SIZE];
+static char buffer[BUF_SIZE + 1];
 static unsigned char command[CMD_SIZE];
 static struct sg_io_hdr query;
 
@@ -100,6 +100,25 @@ static const char *find_generic_scsi_device(int access_flags, int skip_usb)
 	return NULL;
 }
 
+static void dump_hex(const char *str, size_t size)
+{
+	size_t i;
+
+	for (; size && !str[size - 1]; size--)
+		;
+
+	for (i = 0; i < size; i++) {
+		if (i && (i % 32) == 0)
+			printf("\n");
+		else if (i && (i % 4) == 0)
+			printf(" ");
+
+		printf("%02x", (unsigned int)str[i]);
+	}
+
+	printf("\n");
+}
+
 static void setup(void)
 {
 	const char *devpath = find_generic_scsi_device(O_RDONLY, 1);
@@ -135,6 +154,7 @@ static void run(void)
 
 	for (i = 0; i < 100; i++) {
 		TEST(ioctl(devfd, SG_IO, &query));
+		buffer[BUF_SIZE] = '\0';
 
 		if (TST_RET != 0 && TST_RET != -1)
 			tst_brk(TBROK|TTERRNO, "Invalid ioctl() return value");
@@ -143,6 +163,8 @@ static void run(void)
 		for (j = 0; j < BUF_SIZE; j++) {
 			if (buffer[j]) {
 				tst_res(TFAIL, "Kernel memory leaked");
+				tst_res(TINFO, "Buffer contents: %s", buffer);
+				dump_hex(buffer, BUF_SIZE);
 				return;
 			}
 		}
-- 
2.51.0


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

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

[Petr Vorel]

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);

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 2/2] ioctl_sg01: Print buffer contents on failure

[Petr Vorel]

Hi Martin,

> This patch is a convenience feature. Feel free to reject it.

IMHO it's useful to to print debug info.

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

Kind regards,
Petr

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

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

[Petr Vorel]

Hi Martin,

> 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).

FYI running test with this HDD connected over USB no leaked info was printed (no
false positive), but test runs very slow (10 iterations in ~5 min, it will
likely manage below the limit, but 50x slower run is just waste of time given
vast majority of testers runs tests sequentially).

Kind regards,
Petr

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 2/2] ioctl_sg01: Print buffer contents on failure

[Petr Vorel]

Hi Martin,

patchset merged, thank you for a great work.

Kind regards,
Petr

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 2/2] ioctl_sg01: Print buffer contents on failure

[Cyril Hrubis]

Hi!
> +static void dump_hex(const char *str, size_t size)
> +{
> +	size_t i;
> +
> +	for (; size && !str[size - 1]; size--)
> +		;
> +
> +	for (i = 0; i < size; i++) {
> +		if (i && (i % 32) == 0)
> +			printf("\n");
> +		else if (i && (i % 4) == 0)
> +			printf(" ");
> +
> +		printf("%02x", (unsigned int)str[i]);
> +	}
> +
> +	printf("\n");
> +}
> +
>  static void setup(void)
>  {
>  	const char *devpath = find_generic_scsi_device(O_RDONLY, 1);
> @@ -135,6 +154,7 @@ static void run(void)
>  
>  	for (i = 0; i < 100; i++) {
>  		TEST(ioctl(devfd, SG_IO, &query));
> +		buffer[BUF_SIZE] = '\0';
>  
>  		if (TST_RET != 0 && TST_RET != -1)
>  			tst_brk(TBROK|TTERRNO, "Invalid ioctl() return value");
> @@ -143,6 +163,8 @@ static void run(void)
>  		for (j = 0; j < BUF_SIZE; j++) {
>  			if (buffer[j]) {
>  				tst_res(TFAIL, "Kernel memory leaked");
> +				tst_res(TINFO, "Buffer contents: %s", buffer);

Any reason why we don't use tst_res_hexd() instead?

-- 
Cyril Hrubis
chrubis@suse.cz

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 2/2] ioctl_sg01: Print buffer contents on failure

[Martin Doucha]

On 10/29/25 10:54, Cyril Hrubis wrote:
> Hi!
> 
> Any reason why we don't use tst_res_hexd() instead?

Hi,
I didn't known about tst_res_hexd(). Please merge patch 1 and I'll send 
patch v2 for the hexdump afterwards.

-- 
Martin Doucha   mdoucha@suse.cz
SW Quality Engineer
SUSE LINUX, s.r.o.
CORSO IIa
Krizikova 148/34
186 00 Prague 8
Czech Republic

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 2/2] ioctl_sg01: Print buffer contents on failure

[Cyril Hrubis]

Hi!
> I didn't known about tst_res_hexd(). Please merge patch 1 and I'll send 
> patch v2 for the hexdump afterwards.

Too late for that, Petr already merged both, so please send a follow up
that replaces the hexdump function with tst_res_hexd()

-- 
Cyril Hrubis
chrubis@suse.cz

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp