[PATCH 0/2] Improve a SCSI target configfs show function

[PATCH 0/2] Improve a SCSI target configfs show function

[Bart Van Assche]

Hi Martin,

A recent security fix made me take a closer look at target_lu_gp_members_show().
I found one bug and also noticed that this function can be simplified. Please
consider this patch series for the next merge window.

Thanks,

Bart.

Bart Van Assche (2):
  scsi: target: Do not write NUL characters into ASCII configfs output
  scsi: target: Simplify target_lu_gp_members_show()

 drivers/target/target_core_configfs.c | 23 +++++++----------------
 1 file changed, 7 insertions(+), 16 deletions(-)

[PATCH 1/2] scsi: target: Do not write NUL characters into ASCII configfs output

[Bart Van Assche]

NUL characters are not allowed in ASCII configfs output. Hence this patch.

Fixes: c66ac9db8d4a ("[SCSI] target: Add LIO target core v4.0.0-rc6")
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
 drivers/target/target_core_configfs.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/target/target_core_configfs.c b/drivers/target/target_core_configfs.c
index b19acd662726..1bd28482e7cb 100644
--- a/drivers/target/target_core_configfs.c
+++ b/drivers/target/target_core_configfs.c
@@ -2772,7 +2772,6 @@ static ssize_t target_lu_gp_members_show(struct config_item *item, char *page)
 		cur_len = snprintf(buf, LU_GROUP_NAME_BUF, "%s/%s\n",
 			config_item_name(&hba->hba_group.cg_item),
 			config_item_name(&dev->dev_group.cg_item));
-		cur_len++; /* Extra byte for NULL terminator */
 
 		if ((cur_len + len) > PAGE_SIZE || cur_len > LU_GROUP_NAME_BUF) {
 			pr_warn("Ran out of lu_gp_show_attr"

[PATCH 2/2] scsi: target: Simplify target_lu_gp_members_show()

[Bart Van Assche]

Remove the stack buffer 'buf'. This patch does not modify the output
produced by target_lu_gp_members_show().

An example of the output that is produced with this patch applied:

$ cat /sys/kernel/config/target/core/alua/lu_gps/default_lu_gp/members
fileio_0/vdev0
fileio_1/vdev1
iblock_0/vdev2
$ od -c /sys/kernel/config/target/core/alua/lu_gps/default_lu_gp/members
0000000   f   i   l   e   i   o   _   0   /   v   d   e   v   0  \n   f
0000020   i   l   e   i   o   _   1   /   v   d   e   v   1  \n   i   b
0000040   l   o   c   k   _   0   /   v   d   e   v   2  \n
0000055

Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
 drivers/target/target_core_configfs.c | 22 +++++++---------------
 1 file changed, 7 insertions(+), 15 deletions(-)

diff --git a/drivers/target/target_core_configfs.c b/drivers/target/target_core_configfs.c
index 1bd28482e7cb..3887825412e9 100644
--- a/drivers/target/target_core_configfs.c
+++ b/drivers/target/target_core_configfs.c
@@ -2758,32 +2758,24 @@ static ssize_t target_lu_gp_lu_gp_id_store(struct config_item *item,
 static ssize_t target_lu_gp_members_show(struct config_item *item, char *page)
 {
 	struct t10_alua_lu_gp *lu_gp = to_lu_gp(item);
-	struct se_device *dev;
-	struct se_hba *hba;
 	struct t10_alua_lu_gp_member *lu_gp_mem;
-	ssize_t len = 0, cur_len;
-	unsigned char buf[LU_GROUP_NAME_BUF] = { };
+	const char *const end = page + PAGE_SIZE;
+	char *cur = page;
 
 	spin_lock(&lu_gp->lu_gp_lock);
 	list_for_each_entry(lu_gp_mem, &lu_gp->lu_gp_mem_list, lu_gp_mem_list) {
-		dev = lu_gp_mem->lu_gp_mem_dev;
-		hba = dev->se_hba;
+		struct se_device *dev = lu_gp_mem->lu_gp_mem_dev;
+		struct se_hba *hba = dev->se_hba;
 
-		cur_len = snprintf(buf, LU_GROUP_NAME_BUF, "%s/%s\n",
+		cur += scnprintf(cur, end - cur, "%s/%s\n",
 			config_item_name(&hba->hba_group.cg_item),
 			config_item_name(&dev->dev_group.cg_item));
-
-		if ((cur_len + len) > PAGE_SIZE || cur_len > LU_GROUP_NAME_BUF) {
-			pr_warn("Ran out of lu_gp_show_attr"
-				"_members buffer\n");
+		if (WARN_ON_ONCE(cur >= end))
 			break;
-		}
-		memcpy(page+len, buf, cur_len);
-		len += cur_len;
 	}
 	spin_unlock(&lu_gp->lu_gp_lock);
 
-	return len;
+	return cur - page;
 }
 
 CONFIGFS_ATTR(target_lu_gp_, lu_gp_id);