[PATCH v2][next] scsi: sd: Avoid -Wflex-array-member-not-at-end warning

[PATCH v2][next] scsi: sd: Avoid -Wflex-array-member-not-at-end warning

[Gustavo A. R. Silva]

Apparently, the flexible-array member in `struct scsi_stream_status_header`
is not actually needed. Remove it along with the related KUnit test.

So, with these changes, fix the following warning:

drivers/scsi/sd.c:3195:50: warning: structure containing a flexible array member is not at the end of another structure [-Wflex-array-member-not-at-end]

Co-developed-by: Christoph Hellwig <hch@infradead.org>
Signed-off-by: Christoph Hellwig <hch@infradead.org>
Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org>
---
Changes in v2:
 - Revome flexible-array member. (Christoph)

v1:
 - Link: https://lore.kernel.org/linux-hardening/aAwos0mLxneG9R_t@kspp/

 drivers/scsi/scsi_proto_test.c | 8 --------
 drivers/scsi/sd.c              | 2 +-
 include/scsi/scsi_proto.h      | 1 -
 3 files changed, 1 insertion(+), 10 deletions(-)

diff --git a/drivers/scsi/scsi_proto_test.c b/drivers/scsi/scsi_proto_test.c
index c093389edabb..2fc5c6523184 100644
--- a/drivers/scsi/scsi_proto_test.c
+++ b/drivers/scsi/scsi_proto_test.c
@@ -30,14 +30,6 @@ static void test_scsi_proto(struct kunit *test)
 	KUNIT_EXPECT_EQ(test, get_unaligned_be16(&ss.s.stream_identifier),
 			0x1234);
 	KUNIT_EXPECT_EQ(test, ss.s.rel_lifetime + 0, 0x3f);
-
-	static const union {
-		struct scsi_stream_status_header h;
-		u8 arr[sizeof(struct scsi_stream_status_header)];
-	} sh = { .arr = { 1, 2, 3, 4, 0, 0, 5, 6 } };
-	KUNIT_EXPECT_EQ(test, get_unaligned_be32(&sh.h.len), 0x1020304);
-	KUNIT_EXPECT_EQ(test, get_unaligned_be16(&sh.h.number_of_open_streams),
-			0x506);
 }
 
 static struct kunit_case scsi_proto_test_cases[] = {
diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index 950d8c9fb884..3f6e87705b62 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -3215,7 +3215,7 @@ static bool sd_is_perm_stream(struct scsi_disk *sdkp, unsigned int stream_id)
 		return false;
 	if (get_unaligned_be32(&buf.h.len) < sizeof(struct scsi_stream_status))
 		return false;
-	return buf.h.stream_status[0].perm;
+	return buf.s.perm;
 }
 
 static void sd_read_io_hints(struct scsi_disk *sdkp, unsigned char *buffer)
diff --git a/include/scsi/scsi_proto.h b/include/scsi/scsi_proto.h
index aeca37816506..bc8f2b2226be 100644
--- a/include/scsi/scsi_proto.h
+++ b/include/scsi/scsi_proto.h
@@ -349,7 +349,6 @@ struct scsi_stream_status_header {
 	__be32 len;	/* length in bytes of stream_status[] array. */
 	u16 reserved;
 	__be16 number_of_open_streams;
-	DECLARE_FLEX_ARRAY(struct scsi_stream_status, stream_status);
 };
 
 static_assert(sizeof(struct scsi_stream_status_header) == 8);
-- 
2.43.0

Re: [PATCH v2][next] scsi: sd: Avoid -Wflex-array-member-not-at-end warning

[Christoph Hellwig]

I actually send a patch for this yesterday, also keeping the proto
test that actually still works despite my initial overreaching removal.
(I still wish we'd kill the stupid struct and this test and just used
the simpler and cleaner bitshifting and masking)

Re: [PATCH v2][next] scsi: sd: Avoid -Wflex-array-member-not-at-end warning

[Bart Van Assche]

On 5/1/25 11:50 PM, Christoph Hellwig wrote:
> (I still wish we'd kill the stupid struct and this test and just used
> the simpler and cleaner bitshifting and masking)

Bit-shifting and masking results in faster code. For slow path code I
prefer bitfields because this results in much more compact source code.

Bart.

Re: [PATCH v2][next] scsi: sd: Avoid -Wflex-array-member-not-at-end warning

[Christoph Hellwig]

On Sat, May 03, 2025 at 12:08:23PM -0700, Bart Van Assche wrote:
> On 5/1/25 11:50 PM, Christoph Hellwig wrote:
> > (I still wish we'd kill the stupid struct and this test and just used
> > the simpler and cleaner bitshifting and masking)
> 
> Bit-shifting and masking results in faster code. For slow path code I
> prefer bitfields because this results in much more compact source code.

Despite claims to the contrary bit tests and masking are often a lot
simpler, and you don't need the duplicate struct layouts and tests
verifying that you get them right as it is almost impossible to write
code that only works on LE or BE.  E.g. the patch below would simplify
the stream status quite a bit.  It also fixes the assumption that the
bitfield endianess is the same as the byte endianess, which is not
true in general (although I think it is for all Linux supported
architectures).

diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
index f0eec4708ddd..41ed22f1ad0b 100644
--- a/drivers/scsi/scsi_debug.c
+++ b/drivers/scsi/scsi_debug.c
@@ -5528,7 +5528,8 @@ static int resp_get_stream_status(struct scsi_cmnd *scp,
 	     offset += 8, stream_id++) {
 		struct scsi_stream_status *stream_status = (void *)arr + offset;
 
-		stream_status->perm = stream_id < PERMANENT_STREAM_COUNT;
+		if (stream_id < PERMANENT_STREAM_COUNT)
+			stream_status->flags |= SCSI_STREAM_STATUS_PERM;
 		put_unaligned_be16(stream_id,
 				   &stream_status->stream_identifier);
 		stream_status->rel_lifetime = stream_id + 1;
diff --git a/drivers/scsi/scsi_proto_test.c b/drivers/scsi/scsi_proto_test.c
index c093389edabb..d61302e7715a 100644
--- a/drivers/scsi/scsi_proto_test.c
+++ b/drivers/scsi/scsi_proto_test.c
@@ -22,15 +22,6 @@ static void test_scsi_proto(struct kunit *test)
 	KUNIT_EXPECT_EQ(test, d.desc.params[0] + 0, 0xe4);
 	KUNIT_EXPECT_EQ(test, d.desc.params[1] + 0, 0xe3);
 
-	static const union {
-		struct scsi_stream_status s;
-		u8 arr[sizeof(struct scsi_stream_status)];
-	} ss = { .arr = { 0x80, 0, 0x12, 0x34, 0x3f } };
-	KUNIT_EXPECT_EQ(test, ss.s.perm + 0, 1);
-	KUNIT_EXPECT_EQ(test, get_unaligned_be16(&ss.s.stream_identifier),
-			0x1234);
-	KUNIT_EXPECT_EQ(test, ss.s.rel_lifetime + 0, 0x3f);
-
 	static const union {
 		struct scsi_stream_status_header h;
 		u8 arr[sizeof(struct scsi_stream_status_header)];
diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index 3f6e87705b62..deb8af152f13 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -3215,7 +3215,7 @@ static bool sd_is_perm_stream(struct scsi_disk *sdkp, unsigned int stream_id)
 		return false;
 	if (get_unaligned_be32(&buf.h.len) < sizeof(struct scsi_stream_status))
 		return false;
-	return buf.s.perm;
+	return buf.s.flags & SCSI_STREAM_STATUS_PERM;
 }
 
 static void sd_read_io_hints(struct scsi_disk *sdkp, unsigned char *buffer)
diff --git a/include/scsi/scsi_proto.h b/include/scsi/scsi_proto.h
index f64385cde5b9..88c1d950be54 100644
--- a/include/scsi/scsi_proto.h
+++ b/include/scsi/scsi_proto.h
@@ -319,26 +319,10 @@ static_assert(sizeof(struct scsi_io_group_descriptor) == 16);
 
 /* SCSI stream status descriptor */
 struct scsi_stream_status {
-#if defined(__BIG_ENDIAN)
-	u8 perm: 1;
-	u8 reserved1: 7;
-#elif defined(__LITTLE_ENDIAN)
-	u8 reserved1: 7;
-	u8 perm: 1;
-#else
-#error
-#endif
-	u8 reserved2;
+	u8 flags;
+#define SCSI_STREAM_STATUS_PERM		(1u << 7)
 	__be16 stream_identifier;
-#if defined(__BIG_ENDIAN)
-	u8 reserved3: 2;
-	u8 rel_lifetime: 6;
-#elif defined(__LITTLE_ENDIAN)
-	u8 rel_lifetime: 6;
-	u8 reserved3: 2;
-#else
-#error
-#endif
+	u8 rel_lifetime;
 	u8 reserved4[3];
 };