Patch "scsi: iscsi: Add strlen() check in iscsi_if_set{_host}_param()" has been added to the 6.4-stable tree

[Sasha Levin <sashal@xxxxxxxxxx>]

This is a note to let you know that I've just added the patch titled

    scsi: iscsi: Add strlen() check in iscsi_if_set{_host}_param()

to the 6.4-stable tree which can be found at:
    http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary

The filename of the patch is:
     scsi-iscsi-add-strlen-check-in-iscsi_if_set-_host-_p.patch
and it can be found in the queue-6.4 subdirectory.

If you, or anyone else, feels it should not be added to the stable tree,
please let <stable@xxxxxxxxxxxxxxx> know about it.



commit 91e34da43e6a9676fbcdc941a5eb4ba93188953d
Author: Lin Ma <linma@xxxxxxxxxx>
Date:   Sun Jul 23 15:58:20 2023 +0800

    scsi: iscsi: Add strlen() check in iscsi_if_set{_host}_param()
    
    [ Upstream commit ce51c817008450ef4188471db31639d42d37a5e1 ]
    
    The functions iscsi_if_set_param() and iscsi_if_set_host_param() convert an
    nlattr payload to type char* and then call C string handling functions like
    sscanf and kstrdup:
    
      char *data = (char*)ev + sizeof(*ev);
      ...
      sscanf(data, "%d", &value);
    
    However, since the nlattr is provided by the user-space program and the
    nlmsg skb is allocated with GFP_KERNEL instead of GFP_ZERO flag (see
    netlink_alloc_large_skb() in netlink_sendmsg()), dirty data on the heap can
    lead to an OOB access for those string handling functions.
    
    By investigating how the bug is introduced, we find it is really
    interesting as the old version parsing code starting from commit
    fd7255f51a13 ("[SCSI] iscsi: add sysfs attrs for uspace sync up") treated
    the nlattr as integer bytes instead of string and had length check in
    iscsi_copy_param():
    
      if (ev->u.set_param.len != sizeof(uint32_t))
        BUG();
    
    But, since the commit a54a52caad4b ("[SCSI] iscsi: fixup set/get param
    functions"), the code treated the nlattr as C string while forgetting to
    add any strlen checks(), opening the possibility of an OOB access.
    
    Fix the potential OOB by adding the strlen() check before accessing the
    buf. If the data passes this check, all low-level set_param handlers can
    safely treat this buf as legal C string.
    
    Fixes: fd7255f51a13 ("[SCSI] iscsi: add sysfs attrs for uspace sync up")
    Fixes: 1d9bf13a9cf9 ("[SCSI] iscsi class: add iscsi host set param event")
    Signed-off-by: Lin Ma <linma@xxxxxxxxxx>
    Link: https://lore.kernel.org/r/20230723075820.3713119-1-linma@xxxxxxxxxx
    Reviewed-by: Chris Leech <cleech@xxxxxxxxxx>
    Signed-off-by: Martin K. Petersen <martin.petersen@xxxxxxxxxx>
    Signed-off-by: Sasha Levin <sashal@xxxxxxxxxx>

diff --git a/drivers/scsi/scsi_transport_iscsi.c b/drivers/scsi/scsi_transport_iscsi.c
index 2680de88f5bbc..49dbcd67579aa 100644
--- a/drivers/scsi/scsi_transport_iscsi.c
+++ b/drivers/scsi/scsi_transport_iscsi.c
@@ -3029,6 +3029,10 @@ iscsi_if_set_param(struct iscsi_transport *transport, struct iscsi_uevent *ev, u
 	if (!conn || !session)
 		return -EINVAL;
 
+	/* data will be regarded as NULL-ended string, do length check */
+	if (strlen(data) > ev->u.set_param.len)
+		return -EINVAL;
+
 	switch (ev->u.set_param.param) {
 	case ISCSI_PARAM_SESS_RECOVERY_TMO:
 		sscanf(data, "%d", &value);
@@ -3202,6 +3206,10 @@ iscsi_set_host_param(struct iscsi_transport *transport,
 		return -ENODEV;
 	}
 
+	/* see similar check in iscsi_if_set_param() */
+	if (strlen(data) > ev->u.set_host_param.len)
+		return -EINVAL;
+
 	err = transport->set_host_param(shost, ev->u.set_host_param.param,
 					data, ev->u.set_host_param.len);
 	scsi_host_put(shost);