iscsi-scstd: Bound source reads in the private string-copy helper

Checkpatch reports STRLCPY at the daemon copy sites:
  Prefer strscpy over strlcpy

The private strlcpy() implementation calls strlen(src) even when the
caller only needs a short prefix. Manual inspection of that warning
shows an unbounded source scan, including for a zero-capacity
destination.

Replace it with iscsi_strscpy(), which examines at most size source
bytes, returns -E2BIG on truncation and terminates every nonempty
destination. Use a private name instead of defining a replacement libc
strlcpy symbol. All thirty callers discard the previous
full-source-length result; their bounded, NUL-terminated output remains
the same for valid string inputs.

Add isolated Ginkgo coverage of the production userspace helper through
the existing C-probe mechanism. Guard both buffers with inaccessible
pages and cover zero capacity, empty strings, fitting strings, exact
fits, one-byte destinations and nonterminated sources at the copy limit.
No kernel code is compiled into this userspace probe.

iscsi-scst/usr/config.c

@@ -147,7 +147,7 @@ int iscsi_attr_create(int attr_size, struct __qelem *attrs_list,
 		goto add;
 
 	if (sysfs_name_tmpl != NULL) {
-		strlcpy(attr->sysfs_name, sysfs_name_tmpl, sizeof(attr->sysfs_name));
+		iscsi_strscpy(attr->sysfs_name, sysfs_name_tmpl, sizeof(attr->sysfs_name));
 		if (iscsi_attr_lookup_by_sysfs_name(attrs_list, attr->sysfs_name) == NULL)
 			goto add;
 	}
@@ -390,7 +390,7 @@ int config_account_query(u32 tid, int dir, const char *name, char *pass)
 	if (!(user = account_lookup_by_name(tid, dir, name)))
 		return -ENOENT;
 
-	strlcpy(pass, ISCSI_USER_PASS(user), ISCSI_NAME_LEN);
+	iscsi_strscpy(pass, ISCSI_USER_PASS(user), ISCSI_NAME_LEN);
 
 	return 0;
 }
@@ -418,7 +418,7 @@ int config_account_list(u32 tid, int dir, u32 *cnt, u32 *overflow,
 
 	list_for_each_entry(user, list, ulist) {
 		if (buf_sz >= ISCSI_NAME_LEN) {
-			strlcpy(buf, ISCSI_USER_NAME(user), ISCSI_NAME_LEN);
+			iscsi_strscpy(buf, ISCSI_USER_NAME(user), ISCSI_NAME_LEN);
 			buf_sz -= ISCSI_NAME_LEN;
 			buf += ISCSI_NAME_LEN;
 			*cnt += 1;

iscsi-scst/usr/ctldev.c

@@ -74,7 +74,7 @@ int kernel_target_create(struct target *target, u32 *tid, u32 cookie)
 	struct iscsi_kern_attr *kern_attrs;
 
 	memset(&info, 0, sizeof(info));
-	strlcpy(info.name, target->name, sizeof(info.name));
+	iscsi_strscpy(info.name, target->name, sizeof(info.name));
 	info.tid = (tid != NULL) ? *tid : 0;
 	info.cookie = cookie;
 
@@ -113,52 +113,52 @@ int kernel_target_create(struct target *target, u32 *tid, u32 cookie)
 	i = 0;
 
 	kern_attrs[i].mode = 0644;
-	strlcpy(kern_attrs[i].name, ISCSI_PER_PORTAL_ACL_ATTR_NAME,
-		sizeof(ISCSI_PER_PORTAL_ACL_ATTR_NAME));
+	iscsi_strscpy(kern_attrs[i].name, ISCSI_PER_PORTAL_ACL_ATTR_NAME,
+		      sizeof(ISCSI_PER_PORTAL_ACL_ATTR_NAME));
 	i++;
 
 	kern_attrs[i].mode = 0644;
-	strlcpy(kern_attrs[i].name, ISCSI_TARGET_REDIRECTION_ATTR_NAME,
-		sizeof(ISCSI_TARGET_REDIRECTION_ATTR_NAME));
+	iscsi_strscpy(kern_attrs[i].name, ISCSI_TARGET_REDIRECTION_ATTR_NAME,
+		      sizeof(ISCSI_TARGET_REDIRECTION_ATTR_NAME));
 	i++;
 
 	kern_attrs[i].mode = 0644;
-	strlcpy(kern_attrs[i].name, ISCSI_TARGET_ALIAS_ATTR_NAME,
-		sizeof(ISCSI_TARGET_ALIAS_ATTR_NAME));
+	iscsi_strscpy(kern_attrs[i].name, ISCSI_TARGET_ALIAS_ATTR_NAME,
+		      sizeof(ISCSI_TARGET_ALIAS_ATTR_NAME));
 	i++;
 
 	for (j = 0; j < session_key_last; j++) {
 		if (!session_keys[j].show_in_sysfs)
 			continue;
 		kern_attrs[i].mode = 0644;
-		strlcpy(kern_attrs[i].name, session_keys[j].name,
-			sizeof(kern_attrs[i].name));
+		iscsi_strscpy(kern_attrs[i].name, session_keys[j].name,
+			      sizeof(kern_attrs[i].name));
 		i++;
 	}
 	for (j = 0; j < target_key_last; j++) {
 		if (!target_keys[j].show_in_sysfs)
 			continue;
 		kern_attrs[i].mode = 0644;
-		strlcpy(kern_attrs[i].name, target_keys[j].name,
-			sizeof(kern_attrs[i].name));
+		iscsi_strscpy(kern_attrs[i].name, target_keys[j].name,
+			      sizeof(kern_attrs[i].name));
 		i++;
 	}
 	list_for_each_entry(user, &target->target_in_accounts, ulist) {
 		kern_attrs[i].mode = user->sysfs_mode;
-		strlcpy(kern_attrs[i].name, user->sysfs_name,
-			sizeof(kern_attrs[i].name));
+		iscsi_strscpy(kern_attrs[i].name, user->sysfs_name,
+			      sizeof(kern_attrs[i].name));
 		i++;
 	}
 	list_for_each_entry(user, &target->target_out_accounts, ulist) {
 		kern_attrs[i].mode = user->sysfs_mode;
-		strlcpy(kern_attrs[i].name, user->sysfs_name,
-			sizeof(kern_attrs[i].name));
+		iscsi_strscpy(kern_attrs[i].name, user->sysfs_name,
+			      sizeof(kern_attrs[i].name));
 		i++;
 	}
 	list_for_each_entry(portal, &target->allowed_portals, ulist) {
 		kern_attrs[i].mode = portal->sysfs_mode;
-		strlcpy(kern_attrs[i].name, portal->sysfs_name,
-			sizeof(kern_attrs[i].name));
+		iscsi_strscpy(kern_attrs[i].name, portal->sysfs_name,
+			      sizeof(kern_attrs[i].name));
 		i++;
 	}
 
@@ -213,7 +213,7 @@ int kernel_attr_add(struct target *target, const char *name, u32 mode,
 	info.cookie = cookie;
 
 	info.attr.mode = mode;
-	strlcpy(info.attr.name, name, sizeof(info.attr.name));
+	iscsi_strscpy(info.attr.name, name, sizeof(info.attr.name));
 
 	res = ioctl(ctrl_fd, ISCSI_ATTR_ADD, &info);
 	if (res < 0)
@@ -232,7 +232,7 @@ int kernel_attr_del(struct target *target, const char *name, u32 cookie)
 		info.tid = target->tid;
 	info.cookie = cookie;
 
-	strlcpy(info.attr.name, name, sizeof(info.attr.name));
+	iscsi_strscpy(info.attr.name, name, sizeof(info.attr.name));
 
 	res = ioctl(ctrl_fd, ISCSI_ATTR_DEL, &info);
 	if (res < 0)
@@ -259,7 +259,8 @@ int kernel_initiator_allowed(u32 tid, const char *full_initiator_name)
 
 	memset(&info, 0, sizeof(info));
 	info.tid = tid;
-	strlcpy(info.full_initiator_name, full_initiator_name, sizeof(info.full_initiator_name));
+	iscsi_strscpy(info.full_initiator_name, full_initiator_name,
+		      sizeof(info.full_initiator_name));
 
 	if ((err = ioctl(ctrl_fd, ISCSI_INITIATOR_ALLOWED, &info)) < 0) {
 		err = -errno;
@@ -375,7 +376,7 @@ int kernel_session_create(struct connection *conn)
 	info.tid = conn->tid;
 	info.sid = conn->sess->sid.id64;
 	info.exp_cmd_sn = conn->exp_cmd_sn;
-	strlcpy(info.initiator_name, conn->sess->initiator, sizeof(info.initiator_name));
+	iscsi_strscpy(info.initiator_name, conn->sess->initiator, sizeof(info.initiator_name));
 
 
 	iscsi_make_full_initiator_name(target->per_portal_acl,

iscsi-scst/usr/event.c

@@ -131,7 +131,7 @@ static int send_mgmt_cmd_res(u32 tid, u32 cookie, u32 req_cmd, int result,
 	cinfo.result = result;
 
 	if (res_str != NULL)
-		strlcpy(cinfo.value, res_str, sizeof(cinfo.value));
+		iscsi_strscpy(cinfo.value, res_str, sizeof(cinfo.value));
 
 	log_debug(1, "Sending result %d (cookie %d)", result, cookie);
 
@@ -751,7 +751,7 @@ static int handle_target_redirect(struct target *target, char *p)
 			t = addr+1;
 		else
 			t = addr;
-		strlcpy(tmp, t, strchrnul(t, ']')-t+1);
+		iscsi_strscpy(tmp, t, strchrnul(t, ']') - t + 1);
 		if (inet_pton(AF_INET6, tmp, &ia) != 1) {
 			log_error("Invalid addr %s", addr);
 			res = -EINVAL;
@@ -1096,7 +1096,7 @@ static int handle_e_set_attr_value(int fd, const struct iscsi_kern_event *event)
 			goto out_free_server;
 	} else if (strcasecmp(ISCSI_ISNS_ENTITY_ATTR_NAME, pp) == 0) {
 		pp = config_sep_string(&p);
-		strlcpy(isns_entity_target_name, pp, sizeof(isns_entity_target_name));
+		iscsi_strscpy(isns_entity_target_name, pp, sizeof(isns_entity_target_name));
 	} else if (strcasecmp(ISCSI_LINK_LOCAL_ATTR_NAME, pp) == 0) {
 		if (target != NULL) {
 			log_error("Not NULL target %s for global attribute %s",

iscsi-scst/usr/iscsi_adm.c

@@ -261,7 +261,7 @@ static void show_iscsi_params(int type, struct iscsi_param *param)
 
 	for (i = 0; i < nr; i++) {
 		memset(buf, 0, sizeof(buf));
-		strlcpy(buf, keys[i].name, sizeof(buf));
+		iscsi_strscpy(buf, keys[i].name, sizeof(buf));
 		len = strlen(buf);
 		p = buf + len;
 		*p++ = '=';
@@ -348,7 +348,7 @@ static int trgt_handle(int op, u32 set, u32 tid, char *params)
 			goto out;
 		}
 		req.rcmnd = C_TRGT_NEW;
-		strlcpy(req.u.trgt.name, p, sizeof(req.u.trgt.name));
+		iscsi_strscpy(req.u.trgt.name, p, sizeof(req.u.trgt.name));
 		break;
 	}
 	case OP_DELETE:
@@ -464,7 +464,7 @@ static int user_handle_show_user(struct iscsi_adm_req *req, char *user)
 	int err;
 
 	req->rcmnd = C_ACCT_SHOW;
-	strlcpy(req->u.acnt.u.user.name, user, sizeof(req->u.acnt.u.user.name));
+	iscsi_strscpy(req->u.acnt.u.user.name, user, sizeof(req->u.acnt.u.user.name));
 
 	err = iscsid_request(req, NULL, 0);
 	if (!err)
@@ -547,8 +547,8 @@ static int user_handle_new(struct iscsi_adm_req *req, char *user, char *pass)
 
 	req->rcmnd = C_ACCT_NEW;
 
-	strlcpy(req->u.acnt.u.user.name, user, sizeof(req->u.acnt.u.user.name));
-	strlcpy(req->u.acnt.u.user.pass, pass, sizeof(req->u.acnt.u.user.pass));
+	iscsi_strscpy(req->u.acnt.u.user.name, user, sizeof(req->u.acnt.u.user.name));
+	iscsi_strscpy(req->u.acnt.u.user.pass, pass, sizeof(req->u.acnt.u.user.pass));
 
 	return iscsid_request(req, NULL, 0);
 }
@@ -565,7 +565,7 @@ static int user_handle_del(struct iscsi_adm_req *req, char *user, char *pass)
 
 	req->rcmnd = C_ACCT_DEL;
 
-	strlcpy(req->u.acnt.u.user.name, user, sizeof(req->u.acnt.u.user.name));
+	iscsi_strscpy(req->u.acnt.u.user.name, user, sizeof(req->u.acnt.u.user.name));
 
 	return iscsid_request(req, NULL, 0);
 }

iscsi-scst/usr/param.c

@@ -22,18 +22,22 @@
 
 #include "iscsid.h"
 
-/* Taken from Linux kernel sources */
-size_t strlcpy(char *dest, const char *src, size_t size)
+/* Copy a string without reading beyond size bytes; report truncation. */
+ssize_t iscsi_strscpy(char *dest, const char *src, size_t size)
 {
-	size_t ret = strlen(src);
+	size_t len;
 
-	if (size) {
-		size_t len = (ret >= size) ? size - 1 : ret;
+	if (!size)
+		return -E2BIG;
 
-		memcpy(dest, src, len);
-		dest[len] = '\0';
+	len = strnlen(src, size);
+	if (len == size) {
+		memcpy(dest, src, size - 1);
+		dest[size - 1] = '\0';
+		return -E2BIG;
 	}
-	return ret;
+	memcpy(dest, src, len + 1);
+	return len;
 }
 
 int params_index_by_name(const char *name, const struct iscsi_key *keys)
@@ -102,10 +106,10 @@ static int bool_val_to_str(unsigned int val, char *str, int len)
 
 	switch (val) {
 	case 0:
-		strlcpy(str, "No", len);
+		iscsi_strscpy(str, "No", len);
 		break;
 	case 1:
-		strlcpy(str, "Yes", len);
+		iscsi_strscpy(str, "Yes", len);
 		break;
 	default:
 		err = -EINVAL;
@@ -247,9 +251,9 @@ static int digest_set_val(struct iscsi_param *param, int idx, unsigned int *val)
 static int marker_val_to_str(unsigned int val, char *str, int len)
 {
 	if (val == 0)
-		strlcpy(str, "Irrelevant", len);
+		iscsi_strscpy(str, "Irrelevant", len);
 	else
-		strlcpy(str, "Reject", len);
+		iscsi_strscpy(str, "Reject", len);
 
 	return 0;
 }

iscsi-scst/usr/param.h

@@ -57,7 +57,7 @@ extern struct iscsi_key session_keys[];
 extern struct iscsi_key target_keys[];
 extern struct iscsi_key user_keys[];
 
-extern size_t strlcpy(char *dest, const char *src, size_t size);
+ssize_t iscsi_strscpy(char *dest, const char *src, size_t size);
 
 extern void params_set_defaults(unsigned int *, const struct iscsi_key *);
 extern int params_index_by_name(const char *, const struct iscsi_key *);

iscsi-scst/usr/target.c

@@ -422,7 +422,7 @@ int target_create(const char *name, struct target **out_target)
 	}
 
 	memset(target, 0, sizeof(*target));
-	strlcpy(target->name, name, sizeof(target->name));
+	iscsi_strscpy(target->name, name, sizeof(target->name));
 
 	params_set_defaults(target->target_params, target_keys);
 	params_set_defaults(target->session_params, session_keys);

tests/ginkgo/README.md

@@ -137,6 +137,10 @@ malformed payloads ending at a guard page, including preservation of existing
 ACLs. It also runs through `ginkgo-check`; use `go test ./internal/isns` to
 run it separately. It requires `cc` and opens no sockets.
 
+The isolated `internal/param` suite checks the daemon string-copy helper
+with guard pages, covering empty, fitting and truncated strings and zero
+capacity. It runs through `ginkgo-check` with `cc`, without a running daemon.
+
 ## SCSI and I/O
 
 Install Python 3, util-linux and sg3_utils on the command host: the target for

tests/ginkgo/internal/param/param_test.go

@@ -0,0 +1,78 @@
+// SPDX-License-Identifier: GPL-2.0-only
+package param
+
+import (
+	_ "embed"
+	"os"
+	"path/filepath"
+	"runtime"
+	"testing"
+	"time"
+
+	. "github.com/onsi/ginkgo/v2"
+	. "github.com/onsi/gomega"
+
+	"scst-component-tests/internal/admin"
+	"scst-component-tests/internal/cprobe"
+)
+
+//go:embed testdata/string_probe.c
+var stringProbe string
+
+func TestStringCopy(t *testing.T) {
+	RegisterFailHandler(Fail)
+	RunSpecs(t, "SCST daemon string copies")
+}
+
+var _ = Describe("Bounded daemon string copies", Label("unit", "userspace", "param"), func() {
+	var probe string
+	var client *admin.Client
+
+	BeforeEach(func(ctx SpecContext) {
+		client = &admin.Client{Log: GinkgoWriter}
+		dir, err := os.MkdirTemp("", "scst-param-probe-")
+		Expect(err).NotTo(HaveOccurred())
+		DeferCleanup(func() {
+			if client.Blocked() != nil {
+				AddReportEntry("preserved string-copy probe", dir)
+				return
+			}
+			Expect(os.RemoveAll(dir)).To(Succeed())
+		})
+		source := os.Getenv("SCST_TEST_SOURCE_DIR")
+		if source == "" {
+			_, file, _, ok := runtime.Caller(0)
+			Expect(ok).To(BeTrue())
+			source = filepath.Clean(filepath.Join(filepath.Dir(file), "../../../.."))
+		}
+		Expect(cprobe.PrepareISCSIHeaders(source, dir)).To(Succeed())
+		input := filepath.Join(dir, "string_probe.c")
+		Expect(os.WriteFile(input, []byte(stringProbe), 0600)).To(Succeed())
+		probe = filepath.Join(dir, "string-probe")
+		// Dead-section removal keeps this unit probe independent of the
+		// daemon's unrelated authentication and transport dependencies.
+		r, err := client.RunProgram(ctx, 30*time.Second,
+			"cc", "-D_GNU_SOURCE", "-O2", "-ffunction-sections", "-fdata-sections",
+			"-I"+dir,
+			"-I"+filepath.Join(source, "iscsi-scst/usr"),
+			"-I"+filepath.Join(source, "iscsi-scst/include"),
+			"-I"+filepath.Join(source, "scst/include"),
+			input, "-Wl,--gc-sections", "-o", probe)
+		Expect(err).NotTo(HaveOccurred(), r.String())
+		Expect(r.ExitCode).To(BeZero(), r.String())
+	})
+
+	DescribeTable("bounds source reads and destination writes", func(ctx SpecContext, scenario string) {
+		r, err := client.RunProgram(ctx, 10*time.Second, probe, scenario)
+		Expect(err).NotTo(HaveOccurred(), r.String())
+		Expect(r.ExitCode).To(BeZero(), r.String())
+		Expect(r.Stdout).To(Equal("ok\n"), r.String())
+	},
+		Entry("zero capacity touches neither pointer", "zero"),
+		Entry("an empty source writes only its terminator", "empty"),
+		Entry("a fitting string leaves the tail untouched", "fits"),
+		Entry("an exact fit retains its terminator", "exact-fit"),
+		Entry("a one-byte destination is terminated", "one-byte"),
+		Entry("a full source stops before the guard page", "truncated"),
+	)
+})

tests/ginkgo/internal/param/testdata/string_probe.c

@@ -0,0 +1,90 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+#include <stdbool.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <sys/mman.h>
+#include <unistd.h>
+
+/* Compile the production helper without starting the daemon. */
+#include "param.c"
+
+static void check(bool condition, int line, const char *expression)
+{
+	if (!condition) {
+		fprintf(stderr, "line %d: %s\n", line, expression);
+		exit(EXIT_FAILURE);
+	}
+}
+
+#define CHECK(condition) check(!!(condition), __LINE__, #condition)
+
+int main(int argc, char **argv)
+{
+	char *source_mapping, *destination_mapping, *source, *destination;
+	const char *expected = NULL;
+	ssize_t result;
+	size_t capacity, copied, i;
+	long page = sysconf(_SC_PAGESIZE);
+	bool truncated;
+
+	CHECK(argc == 2 && page > 16);
+	truncated = strcmp(argv[1], "truncated") == 0 ||
+		strcmp(argv[1], "one-byte") == 0;
+	if (strcmp(argv[1], "zero") == 0) {
+		capacity = 0;
+	} else if (strcmp(argv[1], "one-byte") == 0) {
+		capacity = 1;
+	} else if (strcmp(argv[1], "empty") == 0) {
+		capacity = 8;
+		expected = "";
+	} else if (strcmp(argv[1], "fits") == 0) {
+		capacity = 8;
+		expected = "abc";
+	} else if (strcmp(argv[1], "exact-fit") == 0) {
+		capacity = 4;
+		expected = "abc";
+	} else {
+		CHECK(truncated);
+		capacity = 8;
+	}
+
+	source_mapping = mmap(NULL, 2 * page, PROT_READ | PROT_WRITE,
+			      MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
+	destination_mapping = mmap(NULL, 2 * page, PROT_READ | PROT_WRITE,
+				   MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
+	CHECK(source_mapping != MAP_FAILED && destination_mapping != MAP_FAILED);
+	CHECK(mprotect(source_mapping + page, page, PROT_NONE) == 0);
+	CHECK(mprotect(destination_mapping + page, page, PROT_NONE) == 0);
+	source = source_mapping + page - capacity;
+	destination = destination_mapping + page - capacity;
+	destination[-1] = 0x5a;
+	if (capacity) {
+		memset(source, 'x', capacity);
+		memset(destination, 0x5a, capacity);
+		if (expected)
+			memcpy(source, expected, strlen(expected) + 1);
+	}
+
+	/* A source without a NUL ends exactly at the inaccessible page. */
+	result = iscsi_strscpy(destination, source, capacity);
+	CHECK(destination[-1] == 0x5a);
+	if (!capacity) {
+		CHECK(result == -E2BIG);
+	} else if (truncated) {
+		CHECK(result == -E2BIG);
+		for (i = 0; i < capacity - 1; i++)
+			CHECK(destination[i] == 'x');
+		CHECK(destination[capacity - 1] == '\0');
+	} else {
+		copied = strlen(expected) + 1;
+		CHECK(result == (ssize_t)copied - 1);
+		CHECK(memcmp(destination, expected, copied) == 0);
+		for (i = copied; i < capacity; i++)
+			CHECK(destination[i] == 0x5a);
+	}
+	CHECK(munmap(source_mapping, 2 * page) == 0);
+	CHECK(munmap(destination_mapping, 2 * page) == 0);
+	puts("ok");
+	return 0;
+}