[PATCH] scsi: target: configfs: bound snprintf return in tg_pt_gp_members_show

[PATCH] scsi: target: configfs: bound snprintf return in tg_pt_gp_members_show

[Greg Kroah-Hartman]

target_tg_pt_gp_members_show() formats LUN paths with snprintf() into a
256-byte stack buffer, then will memcpy cur_len bytes from that buffer.
snprintf() returns the length the output would have had, which can
exceed the buffer size when the fabric WWN is long because iSCSI IQN
names can be up to 223 bytes.  The check at the memcpy site only guards
the destination page write, not the source read, so memcpy() will read
past the stack buffer and copy adjacent stack contents to the sysfs
reader, which when CONFIG_FORTIFY_SOURCE is enabled, fortify_panic()
will be triggered.

Commit 27e06650a5ea ("scsi: target: target_core_configfs: Add length
check to avoid buffer overflow") added the same bound to the
target_lu_gp_members_show() but the tg_pt_gp variant was missed so
resolve that here.

Cc: "Martin K. Petersen" <martin.petersen@oracle.com>
Fixes: c66ac9db8d4a ("[SCSI] target: Add LIO target core v4.0.0-rc6")
Assisted-by: gregkh_clanker_t1000
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 drivers/target/target_core_configfs.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/target/target_core_configfs.c b/drivers/target/target_core_configfs.c
index a1c91d4515bc..84124b222a99 100644
--- a/drivers/target/target_core_configfs.c
+++ b/drivers/target/target_core_configfs.c
@@ -3227,7 +3227,7 @@ static ssize_t target_tg_pt_gp_members_show(struct config_item *item,
 			config_item_name(&lun->lun_group.cg_item));
 		cur_len++; /* Extra byte for NULL terminator */
 
-		if ((cur_len + len) > PAGE_SIZE) {
+		if (cur_len > TG_PT_GROUP_NAME_BUF || (cur_len + len) > PAGE_SIZE) {
 			pr_warn("Ran out of lu_gp_show_attr"
 				"_members buffer\n");
 			break;
-- 
2.53.0

Re: [PATCH] scsi: target: configfs: bound snprintf return in tg_pt_gp_members_show

[Martin K. Petersen]

Greg,

> target_tg_pt_gp_members_show() formats LUN paths with snprintf() into
> a 256-byte stack buffer, then will memcpy cur_len bytes from that
> buffer. snprintf() returns the length the output would have had, which
> can exceed the buffer size when the fabric WWN is long because iSCSI
> IQN names can be up to 223 bytes. The check at the memcpy site only
> guards the destination page write, not the source read, so memcpy()
> will read past the stack buffer and copy adjacent stack contents to
> the sysfs reader, which when CONFIG_FORTIFY_SOURCE is enabled,
> fortify_panic() will be triggered.

Applied to 7.1/scsi-staging, thanks!

-- 
Martin K. Petersen