Skip to content

L
linux
Switch branch/tag
  1. 08 Aug, 2023 12 commits
  3. 31 Jul, 2023 23 commits
  5. 26 Jul, 2023 5 commits
    • YueHaibing's avatar
      scsi: iscsi: Remove unused extern declaration iscsi_lookup_iface() · a615e93d
      YueHaibing authored 
      This is not used anymore and can be removed.
Signed-off-by: default avatarYueHaibing <yuehaibing@huawei.com>
Link: https://lore.kernel.org/r/20230725141531.10424-1-yuehaibing@huawei.comReviewed-by: default avatarChris Leech <cleech@redhat.com>
Signed-off-by: default avatarMartin K. Petersen <martin.petersen@oracle.com>
      a615e93d
    • Lin Ma's avatar
      scsi: qla4xxx: Add length check when parsing nlattrs · 47cd3770
      Lin Ma authored 
      There are three places that qla4xxx parses nlattrs:

 - qla4xxx_set_chap_entry()

 - qla4xxx_iface_set_param()

 - qla4xxx_sysfs_ddb_set_param()

and each of them directly converts the nlattr to specific pointer of
structure without length checking. This could be dangerous as those
attributes are not validated and a malformed nlattr (e.g., length 0) could
result in an OOB read that leaks heap dirty data.

Add the nla_len check before accessing the nlattr data and return EINVAL if
the length check fails.

Fixes: 26ffd7b4 ("[SCSI] qla4xxx: Add support to set CHAP entries")
Fixes: 1e9e2be3 ("[SCSI] qla4xxx: Add flash node mgmt support")
Fixes: 00c31889 ("[SCSI] qla4xxx: fix data alignment and use nl helpers")
Signed-off-by: default avatarLin Ma <linma@zju.edu.cn>
Link: https://lore.kernel.org/r/20230723080053.3714534-1-linma@zju.edu.cnReviewed-by: default avatarChris Leech <cleech@redhat.com>
Signed-off-by: default avatarMartin K. Petersen <martin.petersen@oracle.com>
      47cd3770
    • Lin Ma's avatar
      scsi: be2iscsi: Add length check when parsing nlattrs · ee0268f2
      Lin Ma authored 
      beiscsi_iface_set_param() parses nlattr with nla_for_each_attr and assumes
every attributes can be viewed as struct iscsi_iface_param_info.

This is not true because there is no any nla_policy to validate the
attributes passed from the upper function iscsi_set_iface_params().

Add the nla_len check before accessing the nlattr data and return EINVAL if
the length check fails.

Fixes: 0e43895e ("[SCSI] be2iscsi: adding functionality to change network settings using iscsiadm")
Signed-off-by: default avatarLin Ma <linma@zju.edu.cn>
Link: https://lore.kernel.org/r/20230723075938.3713864-1-linma@zju.edu.cnReviewed-by: default avatarChris Leech <cleech@redhat.com>
Signed-off-by: default avatarMartin K. Petersen <martin.petersen@oracle.com>
      ee0268f2
    • Lin Ma's avatar
      scsi: iscsi: Add strlen() check in iscsi_if_set{_host}_param() · ce51c817
      Lin Ma authored 
      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
fd7255f5 ("[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 a54a52ca ("[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: fd7255f5 ("[SCSI] iscsi: add sysfs attrs for uspace sync up")
Fixes: 1d9bf13a ("[SCSI] iscsi class: add iscsi host set param event")
Signed-off-by: default avatarLin Ma <linma@zju.edu.cn>
Link: https://lore.kernel.org/r/20230723075820.3713119-1-linma@zju.edu.cnReviewed-by: default avatarChris Leech <cleech@redhat.com>
Signed-off-by: default avatarMartin K. Petersen <martin.petersen@oracle.com>
      ce51c817
    • Lin Ma's avatar
      scsi: iscsi: Add length check for nlattr payload · 971dfcb7
      Lin Ma authored 
      The current NETLINK_ISCSI netlink parsing loop checks every nlmsg to make
sure the length is bigger than sizeof(struct iscsi_uevent) and then calls
iscsi_if_recv_msg().

  nlh = nlmsg_hdr(skb);
  if (nlh->nlmsg_len < sizeof(*nlh) + sizeof(*ev) ||
    skb->len < nlh->nlmsg_len) {
    break;
  }
  ...
  err = iscsi_if_recv_msg(skb, nlh, &group);

Hence, in iscsi_if_recv_msg() the nlmsg_data can be safely converted to
iscsi_uevent as the length is already checked.

However, in other cases the length of nlattr payload is not checked before
the payload is converted to other data structures. One example is
iscsi_set_path() which converts the payload to type iscsi_path without any
checks:

  params = (struct iscsi_path *)((char *)ev + sizeof(*ev));

Whereas iscsi_if_transport_conn() correctly checks the pdu_len:

  pdu_len = nlh->nlmsg_len - sizeof(*nlh) - sizeof(*ev);
  if ((ev->u.send_pdu.hdr_size > pdu_len) ..
    err = -EINVAL;

To sum up, some code paths called in iscsi_if_recv_msg() do not check the
length of the data (see below picture) and directly convert the data to
another data structure. This could result in an out-of-bound reads and heap
dirty data leakage.

             _________  nlmsg_len(nlh) _______________
            /                                         \
+----------+--------------+---------------------------+
| nlmsghdr | iscsi_uevent |          data              |
+----------+--------------+---------------------------+
                          \                          /
                         iscsi_uevent->u.set_param.len

Fix the issue by adding the length check before accessing it. To clean up
the code, an additional parameter named rlen is added. The rlen is
calculated at the beginning of iscsi_if_recv_msg() which avoids duplicated
calculation.

Fixes: ac20c7bf ("[SCSI] iscsi_transport: Added Ping support")
Fixes: 43514774 ("[SCSI] iscsi class: Add new NETLINK_ISCSI messages for cnic/bnx2i driver.")
Fixes: 1d9bf13a ("[SCSI] iscsi class: add iscsi host set param event")
Fixes: 01cb225d ("[SCSI] iscsi: add target discvery event to transport class")
Fixes: 264faaaa ("[SCSI] iscsi: add transport end point callbacks")
Fixes: fd7255f5 ("[SCSI] iscsi: add sysfs attrs for uspace sync up")
Signed-off-by: default avatarLin Ma <linma@zju.edu.cn>
Link: https://lore.kernel.org/r/20230725024529.428311-1-linma@zju.edu.cnReviewed-by: default avatarChris Leech <cleech@redhat.com>
Signed-off-by: default avatarMartin K. Petersen <martin.petersen@oracle.com>
      971dfcb7
GitLab Nexedi Edition | About GitLab | About Nexedi | 沪ICP备2021021310号-2 | 沪ICP备2021021310号-7