From: Crystal Kolipe <kolipe.c@exoticsilicon.com>
Subject: [patch] fix /sbin/scsi printing of raw modepage data
To: tech@openbsd.org
Date: Wed, 21 Jan 2026 18:38:41 +0000

This diff has been in our local patchset since May last year, and in fact I
posted it to -tech around that time, but I just noticed that the relevant code
is still broken in -current...

It fixes some broken logic in the mode page printing code of /sbin/scsi,
specifically when printing raw unformatted hex bytes.

For those not familiar with SCSI modepages, it's a bit non-obvious, but here
goes:

From mode_edit() in scsi.c:

	mode_sense(fd, data, sizeof(data), pagectl, page);

	/* Skip over the block descriptors.
	 */
	mh = (struct mode_header *)data;
	mph = (struct mode_page_header *)(((char *)mh) + sizeof(*mh) + mh->bdl);
	mode_pars = (char *)mph + sizeof(*mph);

	if (!fmt) {
		for (i = 0; i < mh->mdl; i++) {
			printf("%02x%c",mode_pars[i],
			(((i + 1) % 8) == 0) ? '\n' : ' ');
		}

This loop is intended to print SCSI mode page data as hex.

The data returned by mode_sense() in 'data' has the following format:

4 byte header
variable size block descriptors
2 byte mode page header
mode page data

If page 0x3f is requested, multiple pages are concatenated with their 2 byte
headers, so we get:

4 byte header
variable size block descriptors
2 byte mode page header
mode page data
2 byte mode page header
mode page data
...

The for loop loops for the number of bytes specified in the mode sense header.

This value includes the length of not only the mode page data that follows,
but also the last three bytes of that four byte mode sense header itself as
well as the block descriptors.  In other words, the value is the total data
length in 'data' minus one.

The existing printing logic starts printing byte values after the
2-byte mode _page_ header, (struct mode_page_header), that follows the
4-byte mode sense header, (struct mode_header).

So it seems that the intention of the author was to dump _only_ the actual
mode page data bytes, (variable names in earlier version of the code support
this theory as well).  In this case, we just have an excess five bytes
trailing the useful output.

However, this intention to skip the 2-byte mode page header breaks down when
requesting mode page 0x3f, which represents _all_ supported mode pages.

In this case, the data returned is the concatenated octet stream of:

2 bytes mode page header, variable-size mode page data, (repeat) ...

So the hex dump output in this case is also 'off by five', but the mode page
header for the first mode page is missing, (because it was skipped), whereas
those for the second and subsequent mode pages are present in the output.

This bug seems to was introduced to the code in 1995, before the original
revision in the OpenBSD cvs.

Additionally, the existing code skips over any returned block descriptors,
but doesn't subtract that number of bytes from the loop test condition, so it
ends up reading extra meaningless bytes.

Attached fix has two parts:

1. Print the correct number of bytes for all mode page requests.

2. Disable skipping the two-byte mode page header if requesting page 0x3f.

Whilst it seems unlikely that the output is being using in scripts, part 2
maintains the existing byte offsets for displays of single mode pages, and
makes the display consistent when requesting all pages.

--- sbin/scsi/scsi.c
+++ sbin/scsi/scsi.c
@@ -695,6 +695,8 @@
 mode_edit(int fd, int page, int edit, int argc, char *argv[])
 {
 	int i;
+	int flag_all;
+	int offset;
 	u_char data[255];
 	u_char *mode_pars;
 	struct mode_header
@@ -804,10 +806,26 @@
 	 */
 	mh = (struct mode_header *)data;
 	mph = (struct mode_page_header *)(((char *)mh) + sizeof(*mh) + mh->bdl);
-	mode_pars = (char *)mph + sizeof(*mph);
 
+	/*
+	 * If the data length was just block descriptors and the three header
+	 * bytes, we're done.
+	 */
+
+	if (mh->bdl + 3 >= mh->mdl)
+		return ;
+
+	/*
+	 * Skip mode page header unless we are requesting page 0x3f, (all pages)
+	 */
+
+	flag_all = ((page & 0x3f) == 0x3f);
+	offset = (flag_all ? 3 : 5);
+
+	mode_pars = (char *)mph + ( flag_all ? 0 : sizeof(*mph) );
+
 	if (!fmt) {
-		for (i = 0; i < mh->mdl; i++) {
+		for (i = 0; i < (mh->mdl - mh->bdl - offset); i++) {
 			printf("%02x%c",mode_pars[i],
 			(((i + 1) % 8) == 0) ? '\n' : ' ');
 		}