• 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
scsi_transport_iscsi.c 150 KB