scst_lib: Initialize WRITE SAME emulation CDBs and DIF tags

The internal WRITE(32) generated for DIF type 2 has three defects:
put_unaligned() infers a one-byte store from its destination pointer,
reserved/control bytes retain stack data, and the application-tag mask
is copied from bytes 26 and 27 even for a 16-byte input CDB. This can
corrupt the protection information or make the backend reject the write.

Clear the complete output buffer, use a sized big-endian application-tag
store and obtain the mask through scst_cmd_get_dif_app_tag_mask(). That
helper reads an explicit WRITE SAME(32) mask or derives the short-command
mask from the application's tag-check setting.

Extract the existing CDB builder behind the private KUnit interface. Add
regression cases for distinct tag bytes, full output images, short and
long commands, both short-command mask modes and buffer boundaries.
Nonzero poison patterns expose missing stores and reads beyond the short
CDB. Include the private header in generated in-tree kernel patches.

Fixes: 906f4dfefb ("Merge t10-pi branch back")

scripts/kernel/generate-kernel-patch

@@ -319,6 +319,7 @@ scst_05_targ="scst/src/scst_targ.c scst/src/scst_local_cmd.c \
 scst/src/scst_local_cmd.h \
 scst/src/scst_targ_test.h"
 scst_06_lib="scst/src/scst_lib.c \
+scst/src/scst_lib_priv.h \
 scst/src/scst_lib_test.h"
 scst_07_pres="scst/src/scst_pres.h scst/src/scst_pres.c"
 scst_08_sysfs="scst/src/scst_sysfs.c \

scst/src/kunit/lib/suites/scst_kunit_write_same.c

@@ -0,0 +1,240 @@
+// SPDX-License-Identifier: GPL-2.0-only
+#include "../../scst_kunit.h"
+
+#include "scst.h"
+#include "../../../scst_priv.h"
+#include "../../../scst_lib_priv.h"
+#include "../../../scst_lib_test.h"
+
+struct write_same_fixture {
+	struct scst_cmd cmd;
+	struct scst_device dev;
+	struct scst_write_same_priv wsp;
+	u8 cdb[32];
+};
+
+static int write_same_init(struct kunit *test)
+{
+	struct write_same_fixture *f = kunit_kzalloc(test, sizeof(*f), GFP_KERNEL);
+
+	if (!f)
+		return -ENOMEM;
+	test->priv = f;
+	f->cmd.dev = &f->dev;
+	f->cmd.cdb = f->cdb;
+	f->dev.dev_dif_type = 2;
+	f->wsp.ws_orig_cmd = &f->cmd;
+	f->wsp.dif_valid = 1;
+	f->wsp.ref_tag = 0x10203040;
+	return 0;
+}
+
+struct write_same_tag_case {
+	const char *name;
+	int cdb_len;
+	u16 tag;
+};
+
+static const struct write_same_tag_case write_same_tag_cases[] = {
+	{ "ws16_tag_1234", 16, 0x1234 },
+	{ "ws16_tag_abcd", 16, 0xabcd },
+	{ "ws16_tag_0001", 16, 0x0001 },
+	{ "ws16_tag_ff00", 16, 0xff00 },
+	{ "ws32_tag_1234", 32, 0x1234 },
+	{ "ws32_tag_abcd", 32, 0xabcd },
+	{ "ws32_tag_0001", 32, 0x0001 },
+	{ "ws32_tag_ff00", 32, 0xff00 },
+};
+
+static void write_same_tag_desc(const struct write_same_tag_case *p, char *desc)
+{
+	snprintf(desc, KUNIT_PARAM_DESC_SIZE, "%s", p->name);
+}
+
+KUNIT_ARRAY_PARAM(write_same_tag, write_same_tag_cases, write_same_tag_desc);
+
+static void write_same_app_tag(struct kunit *test)
+{
+	const struct write_same_tag_case *p = test->param_value;
+	struct write_same_fixture *f = test->priv;
+	u8 buffer[34];
+	u8 *cdb = &buffer[1];
+	int len;
+
+	f->cmd.cdb_len = p->cdb_len;
+	f->cdb[0] = p->cdb_len == 16 ? WRITE_SAME_16 : VARIABLE_LENGTH_CMD;
+	if (p->cdb_len == 32) {
+		f->cdb[7] = 0x18;
+		put_unaligned_be16(SUBCODE_WRITE_SAME_32, &f->cdb[8]);
+		f->cdb[26] = 0x5a;
+		f->cdb[27] = 0xc3;
+	}
+	f->wsp.app_tag = cpu_to_be16(p->tag);
+	/* Poison both tag bytes: zero-filled fixtures can hide a short store. */
+	memset(buffer, 0xa5, sizeof(buffer));
+	len = scst_ws_build_write_cdb(&f->wsp, cdb, 0x1122334455667788LL, 0x01020304);
+
+	KUNIT_ASSERT_EQ(test, len, 32);
+	KUNIT_EXPECT_EQ(test, cdb[0], (u8)VARIABLE_LENGTH_CMD);
+	KUNIT_EXPECT_EQ(test, get_unaligned_be16(&cdb[8]), (u16)SUBCODE_WRITE_32);
+	KUNIT_EXPECT_EQ(test, cdb[24], (u8)(p->tag >> 8));
+	KUNIT_EXPECT_EQ(test, cdb[25], (u8)p->tag);
+	/* The wider store must preserve the adjacent reference tag and mask. */
+	KUNIT_EXPECT_EQ(test, get_unaligned_be32(&cdb[20]), f->wsp.ref_tag);
+	if (p->cdb_len == 32) {
+		KUNIT_EXPECT_EQ(test, cdb[26], f->cdb[26]);
+		KUNIT_EXPECT_EQ(test, cdb[27], f->cdb[27]);
+	}
+	KUNIT_EXPECT_EQ(test, buffer[0], (u8)0xa5);
+	KUNIT_EXPECT_EQ(test, buffer[33], (u8)0xa5);
+}
+
+struct write_same_cdb_case {
+	const char *name;
+	int cdb_len;
+	int dif_type;
+	bool needs_dif;
+};
+
+static const struct write_same_cdb_case write_same_cdb_cases[] = {
+	{ "no_dif", 16, 0, false },
+	{ "type2_without_dif", 16, 2, false },
+	{ "dif_type1", 16, 1, true },
+	{ "dif_type3", 16, 3, true },
+	{ "ws16_dif_type2", 16, 2, true },
+	{ "ws32_dif_type2", 32, 2, true },
+};
+
+static void write_same_cdb_desc(const struct write_same_cdb_case *p, char *desc)
+{
+	snprintf(desc, KUNIT_PARAM_DESC_SIZE, "%s", p->name);
+}
+
+KUNIT_ARRAY_PARAM(write_same_cdb, write_same_cdb_cases, write_same_cdb_desc);
+
+static void write_same_cdb_image(struct kunit *test)
+{
+	static const u8 write16[32] = {
+		0x8a, 0x00, 0x11, 0x22, 0x33, 0x44, 0x55, 0x66,
+		0x77, 0x88, 0x01, 0x02, 0x03, 0x04, 0x00, 0x00,
+	};
+	static const u8 write32[32] = {
+		0x7f, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x18,
+		0x00, 0x0b, 0x20, 0x00, 0x11, 0x22, 0x33, 0x44,
+		0x55, 0x66, 0x77, 0x88, 0x10, 0x20, 0x30, 0x40,
+		0x12, 0x34, 0x00, 0x00, 0x01, 0x02, 0x03, 0x04,
+	};
+	static const u8 poisons[] = { 0xa5, 0x5a };
+	const struct write_same_cdb_case *p = test->param_value;
+	struct write_same_fixture *f = test->priv;
+	bool long_write = p->dif_type == 2 && p->needs_dif;
+	u8 buffer[34], expected[32];
+	u8 *cdb = &buffer[1];
+	int i;
+
+	f->cmd.cdb_len = p->cdb_len;
+	f->cdb[0] = p->cdb_len == 16 ? WRITE_SAME_16 : VARIABLE_LENGTH_CMD;
+	f->dev.dev_dif_type = p->dif_type;
+	f->wsp.dif_valid = p->needs_dif;
+	f->wsp.app_tag = cpu_to_be16(0x1234);
+	memcpy(expected, long_write ? write32 : write16, sizeof(expected));
+	if (!long_write && p->needs_dif)
+		expected[1] = 0x20;
+	if (p->cdb_len == 32) {
+		f->cdb[7] = 0x18;
+		put_unaligned_be16(SUBCODE_WRITE_SAME_32, &f->cdb[8]);
+		f->cdb[26] = 0x5a;
+		f->cdb[27] = 0xc3;
+		expected[26] = 0x5a;
+		expected[27] = 0xc3;
+	}
+
+	for (i = 0; i < ARRAY_SIZE(poisons); i++) {
+		int len;
+
+		memset(buffer, poisons[i], sizeof(buffer));
+		len = scst_ws_build_write_cdb(&f->wsp, cdb, 0x1122334455667788LL, 0x01020304);
+		KUNIT_ASSERT_EQ(test, len, long_write ? 32 : 16);
+		/* Check every byte, including control, group and reserved fields. */
+		KUNIT_EXPECT_MEMEQ(test, cdb, expected, sizeof(expected));
+		KUNIT_EXPECT_EQ(test, buffer[0], poisons[i]);
+		KUNIT_EXPECT_EQ(test, buffer[33], poisons[i]);
+	}
+}
+
+struct write_same_mask_case {
+	const char *name;
+	int cdb_len;
+	bool check_app_tag;
+	u16 mask;
+};
+
+static const struct write_same_mask_case write_same_mask_cases[] = {
+	{ "ws16_no_app_check", 16, false, 0x0000 },
+	{ "ws16_app_check", 16, true, 0xffff },
+	{ "ws32_zero_mask", 32, true, 0x0000 },
+	{ "ws32_full_mask", 32, false, 0xffff },
+	{ "ws32_partial_mask_no_check", 32, false, 0x5ac3 },
+	{ "ws32_partial_mask_check", 32, true, 0xa53c },
+};
+
+static void write_same_mask_desc(const struct write_same_mask_case *p, char *desc)
+{
+	snprintf(desc, KUNIT_PARAM_DESC_SIZE, "%s", p->name);
+}
+
+KUNIT_ARRAY_PARAM(write_same_mask, write_same_mask_cases, write_same_mask_desc);
+
+static void write_same_app_tag_mask(struct kunit *test)
+{
+	static const u8 poisons[] = { 0xa5, 0x5a };
+	const struct write_same_mask_case *p = test->param_value;
+	struct write_same_fixture *f = test->priv;
+	u8 cdb[32];
+	int i;
+
+	f->cmd.cdb_len = p->cdb_len;
+	f->cmd.cmd_dif_actions = p->check_app_tag ? SCST_DIF_CHECK_APP_TAG :
+		SCST_DIF_ACTION_NONE;
+	f->cdb[0] = p->cdb_len == 16 ? WRITE_SAME_16 : VARIABLE_LENGTH_CMD;
+	if (p->cdb_len == 32) {
+		f->cdb[7] = 0x18;
+		put_unaligned_be16(SUBCODE_WRITE_SAME_32, &f->cdb[8]);
+		put_unaligned_be16(p->mask, &f->cdb[26]);
+	}
+
+	for (i = 0; i < ARRAY_SIZE(poisons); i++) {
+		int len;
+
+		/* A short CDB has no mask: bytes beyond it must not affect output. */
+		if (p->cdb_len == 16)
+			memset(&f->cdb[16], poisons[i], 16);
+		memset(cdb, poisons[i], sizeof(cdb));
+		len = scst_ws_build_write_cdb(&f->wsp, cdb, 1, 1);
+		KUNIT_ASSERT_EQ(test, len, 32);
+		KUNIT_EXPECT_EQ(test, get_unaligned_be16(&cdb[26]), p->mask);
+	}
+
+	if (p->cdb_len == 16) {
+		int len;
+
+		/* Also exercise the actual 16-byte buffer used by scst_alloc_cmd(). */
+		memcpy(f->cmd.cdb_buf, f->cdb, sizeof(f->cmd.cdb_buf));
+		f->cmd.cdb = f->cmd.cdb_buf;
+		len = scst_ws_build_write_cdb(&f->wsp, cdb, 1, 1);
+		KUNIT_ASSERT_EQ(test, len, 32);
+		KUNIT_EXPECT_EQ(test, get_unaligned_be16(&cdb[26]), p->mask);
+	}
+}
+
+static struct kunit_case write_same_cases[] = {
+	KUNIT_CASE_PARAM(write_same_app_tag, write_same_tag_gen_params),
+	KUNIT_CASE_PARAM(write_same_cdb_image, write_same_cdb_gen_params),
+	KUNIT_CASE_PARAM(write_same_app_tag_mask, write_same_mask_gen_params),
+	{}
+};
+
+scst_kunit_suite(write_same_suite, "LIB", "-write-same",
+		 .init = write_same_init,
+		 .test_cases = write_same_cases,
+);

scst/src/kunit/objects.mk

@@ -11,6 +11,7 @@ scst-kunit-scst := \
 	kunit/event/suites/scst_kunit_event_delivery.o \
 	kunit/lib/suites/scst_kunit_capacity.o \
 	kunit/lib/suites/scst_kunit_lib.o \
+	kunit/lib/suites/scst_kunit_write_same.o \
 	kunit/lifecycle/suites/scst_kunit_lifecycle.o \
 	kunit/lifecycle/suites/scst_kunit_blocking.o \
 	kunit/order/suites/scst_kunit_order.o \

scst/src/scst_lib.c

@@ -55,6 +55,7 @@
 #include "scst_priv.h"
 #include "scst_mem.h"
 #include "scst_pres.h"
+#include "scst_lib_priv.h"
 #include "scst_lib_test.h"
 
 #ifndef SCST_KUNIT_ENABLED
@@ -6390,42 +6391,6 @@ ssize_t scst_writev(struct file *file, const struct kvec *vec,
 }
 EXPORT_SYMBOL(scst_writev);
 
-struct scst_ws_sg_tail {
-	struct scatterlist *sg;
-	int sg_cnt;
-};
-
-struct scst_write_same_priv {
-	/* Must be the first for scst_finish_internal_cmd()! */
-	scst_i_finish_fn_t ws_finish_fn;
-
-	struct scst_cmd *ws_orig_cmd;
-
-	struct mutex ws_mutex;
-
-	/* 0 len terminated */
-	struct scst_data_descriptor *ws_descriptors;
-
-	int ws_cur_descr;
-
-	int ws_left_to_send; /* in blocks */
-	int64_t ws_cur_lba; /* in blocks */
-
-	__be16 guard_tag;
-	__be16 app_tag;
-	uint32_t ref_tag;
-	unsigned int dif_valid:1;
-	unsigned int inc_ref_tag:1;
-
-	int ws_max_each;/* in blocks */
-	int ws_cur_in_flight; /* commands */
-
-	struct scst_ws_sg_tail *ws_sg_tails;
-
-	struct scatterlist *ws_sg_full;
-	int ws_sg_full_cnt;
-};
-
 #ifdef CONFIG_SCST_EXTRACHECKS
 static u64 sg_data_length(struct scatterlist *sgl, int nr)
 {
@@ -6440,12 +6405,61 @@ static u64 sg_data_length(struct scatterlist *sgl, int nr)
 }
 #endif
 
+/* Build the internal WRITE CDB in a caller-supplied 32-byte buffer. */
+SCST_KUNIT_VISIBLE
+int scst_ws_build_write_cdb(const struct scst_write_same_priv *wsp,
+			    u8 *write_cdb, s64 lba, int blocks)
+{
+	const struct scst_cmd *ws_cmd = wsp->ws_orig_cmd;
+	const struct scst_device *dev = ws_cmd->dev;
+	bool needs_dif = wsp->dif_valid;
+	int write_cdb_len;
+
+	memset(write_cdb, 0, 32);
+	if (!needs_dif || dev->dev_dif_type != 2) {
+		write_cdb[0] = WRITE_16;
+		write_cdb_len = 16;
+		write_cdb[1] = ws_cmd->cdb[1];
+		if (needs_dif) {
+			u8 wrprotect = ws_cmd->cdb[1] & 0xE0;
+
+			if (wrprotect == 0)
+				wrprotect = 0x20;
+			write_cdb[1] |= wrprotect;
+		}
+		put_unaligned_be64(lba, &write_cdb[2]);
+		put_unaligned_be32(blocks, &write_cdb[10]);
+	} else {
+		/* There might be WRITE SAME(16) with WRPROTECT 0 here */
+		u8 wrprotect;
+
+		if (ws_cmd->cdb_len != 32)
+			wrprotect = ws_cmd->cdb[1] & 0xE0;
+		else
+			wrprotect = ws_cmd->cdb[10] & 0xE0;
+		if (wrprotect == 0)
+			wrprotect = 0x20;
+		write_cdb[0] = VARIABLE_LENGTH_CMD;
+		put_unaligned_be16(SUBCODE_WRITE_32, &write_cdb[8]);
+		write_cdb[7] = 0x18;
+		write_cdb_len = 32;
+		write_cdb[10] = ws_cmd->cdb[10];
+		write_cdb[10] |= wrprotect;
+		put_unaligned_be64(lba, &write_cdb[12]);
+		put_unaligned_be32(blocks, &write_cdb[28]);
+		put_unaligned_be32(wsp->ref_tag, &write_cdb[20]);
+		put_unaligned_be16(be16_to_cpu(wsp->app_tag), &write_cdb[24]);
+		put_unaligned_be16(scst_cmd_get_dif_app_tag_mask(wsp->ws_orig_cmd), &write_cdb[26]);
+	}
+
+	return write_cdb_len;
+}
+
 /* ws_mutex suppose to be locked */
 static int scst_ws_push_single_write(struct scst_write_same_priv *wsp,
 	int64_t lba, int blocks)
 {
 	struct scst_cmd *ws_cmd = wsp->ws_orig_cmd;
-	struct scst_device *dev = ws_cmd->dev;
 	struct scatterlist *ws_sg;
 	int ws_sg_cnt;
 	int res;
@@ -6480,43 +6494,7 @@ static int scst_ws_push_single_write(struct scst_write_same_priv *wsp,
 		goto out;
 	}
 
-	if (!needs_dif || (dev->dev_dif_type != 2)) {
-		memset(write_cdb, 0, sizeof(write_cdb));
-		write_cdb[0] = WRITE_16;
-		write_cdb_len = 16;
-		write_cdb[1] = ws_cmd->cdb[1];
-		if (needs_dif) {
-			uint8_t wrprotect = ws_cmd->cdb[1] & 0xE0;
-
-			if (wrprotect == 0)
-				wrprotect = 0x20;
-			write_cdb[1] |= wrprotect;
-		}
-		put_unaligned_be64(lba, &write_cdb[2]);
-		put_unaligned_be32(blocks, &write_cdb[10]);
-	} else {
-		/* There might be WRITE SAME(16) with WRPROTECT 0 here */
-		uint8_t wrprotect;
-
-		if (ws_cmd->cdb_len != 32)
-			wrprotect = ws_cmd->cdb[1] & 0xE0;
-		else
-			wrprotect = ws_cmd->cdb[10] & 0xE0;
-		if (wrprotect == 0)
-			wrprotect = 0x20;
-		write_cdb[0] = VARIABLE_LENGTH_CMD;
-		put_unaligned_be16(SUBCODE_WRITE_32, &write_cdb[8]);
-		write_cdb[7] = 0x18;
-		write_cdb_len = 32;
-		write_cdb[10] = ws_cmd->cdb[10];
-		write_cdb[10] |= wrprotect;
-		put_unaligned_be64(lba, &write_cdb[12]);
-		put_unaligned_be32(blocks, &write_cdb[28]);
-		put_unaligned_be32(wsp->ref_tag, &write_cdb[20]);
-		put_unaligned(wsp->app_tag, &write_cdb[24]);
-		write_cdb[26] = ws_cmd->cdb[26];
-		write_cdb[27] = ws_cmd->cdb[27];
-	}
+	write_cdb_len = scst_ws_build_write_cdb(wsp, write_cdb, lba, blocks);
 
 	cmd = scst_create_prepare_internal_cmd(ws_cmd, write_cdb,
 		write_cdb_len, SCST_CMD_QUEUE_SIMPLE);

scst/src/scst_lib_priv.h

@@ -0,0 +1,42 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+#ifndef SCST_LIB_PRIV_H
+#define SCST_LIB_PRIV_H
+
+struct scst_ws_sg_tail {
+	struct scatterlist *sg;
+	int sg_cnt;
+};
+
+struct scst_write_same_priv {
+	/* Must be the first for scst_finish_internal_cmd()! */
+	scst_i_finish_fn_t ws_finish_fn;
+
+	struct scst_cmd *ws_orig_cmd;
+
+	/* Serializes generated writes and their completions. */
+	struct mutex ws_mutex;
+
+	/* 0 len terminated */
+	struct scst_data_descriptor *ws_descriptors;
+
+	int ws_cur_descr;
+
+	int ws_left_to_send; /* in blocks */
+	s64 ws_cur_lba; /* in blocks */
+
+	__be16 guard_tag;
+	__be16 app_tag;
+	u32 ref_tag;
+	unsigned int dif_valid:1;
+	unsigned int inc_ref_tag:1;
+
+	int ws_max_each; /* in blocks */
+	int ws_cur_in_flight; /* commands */
+
+	struct scst_ws_sg_tail *ws_sg_tails;
+
+	struct scatterlist *ws_sg_full;
+	int ws_sg_full_cnt;
+};
+
+#endif /* SCST_LIB_PRIV_H */

scst/src/scst_lib_test.h

@@ -6,6 +6,10 @@
 
 #ifdef SCST_KUNIT_ENABLED
 
+struct scst_write_same_priv;
+
+int scst_ws_build_write_cdb(const struct scst_write_same_priv *wsp,
+			    u8 *write_cdb, s64 lba, int blocks);
 void scst_del_free_acg_dev(struct scst_acg_dev *acg_dev, bool del_sysfs);
 void scst_tgt_dev_grace_period(void);
 int scst_lun_cm_add(struct scst_acg_dev *acg_dev, u64 lun, unsigned int *flags);