Skip to content

Commit

Permalink
scsi: iscsi: Add length check for nlattr payload
Browse files Browse the repository at this point in the history 
[ Upstream commit 971dfcb ]

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: ac20c7b ("[SCSI] iscsi_transport: Added Ping support")
Fixes: 4351477 ("[SCSI] iscsi class: Add new NETLINK_ISCSI messages for cnic/bnx2i driver.")
Fixes: 1d9bf13 ("[SCSI] iscsi class: add iscsi host set param event")
Fixes: 01cb225 ("[SCSI] iscsi: add target discvery event to transport class")
Fixes: 264faaa ("[SCSI] iscsi: add transport end point callbacks")
Fixes: fd7255f ("[SCSI] iscsi: add sysfs attrs for uspace sync up")
Signed-off-by: Lin Ma <linma@zju.edu.cn>
Link: https://lore.kernel.org/r/20230725024529.428311-1-linma@zju.edu.cn
Reviewed-by: Chris Leech <cleech@redhat.com>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
  • Loading branch information
@f0rm2l1n @gregkh
f0rm2l1n authored and gregkh committed Sep 13, 2023
1 parent 2737d82 commit bb8d101
Showing 1 changed file with 43 additions and 29 deletions.
72 changes: 43 additions & 29 deletions drivers/scsi/scsi_transport_iscsi.c
View file
Expand Up @@ -3013,14 +3013,15 @@ iscsi_if_destroy_conn(struct iscsi_transport *transport, struct iscsi_uevent *ev
}

static int
iscsi_if_set_param(struct iscsi_transport *transport, struct iscsi_uevent *ev)
iscsi_if_set_param(struct iscsi_transport *transport, struct iscsi_uevent *ev, u32 rlen)
{
char *data = (char*)ev + sizeof(*ev);
struct iscsi_cls_conn *conn;
struct iscsi_cls_session *session;
int err = 0, value = 0, state;

if (ev->u.set_param.len > PAGE_SIZE)
if (ev->u.set_param.len > rlen ||
ev->u.set_param.len > PAGE_SIZE)
return -EINVAL;

session = iscsi_session_lookup(ev->u.set_param.sid);
Expand Down Expand Up @@ -3117,15 +3118,18 @@ static int iscsi_if_ep_disconnect(struct iscsi_transport *transport,

static int
iscsi_if_transport_ep(struct iscsi_transport *transport,
struct iscsi_uevent *ev, int msg_type)
struct iscsi_uevent *ev, int msg_type, u32 rlen)
{
struct iscsi_endpoint *ep;
int rc = 0;

switch (msg_type) {
case ISCSI_UEVENT_TRANSPORT_EP_CONNECT_THROUGH_HOST:
case ISCSI_UEVENT_TRANSPORT_EP_CONNECT:
rc = iscsi_if_ep_connect(transport, ev, msg_type);
if (rlen < sizeof(struct sockaddr))
rc = -EINVAL;
else
rc = iscsi_if_ep_connect(transport, ev, msg_type);
break;
case ISCSI_UEVENT_TRANSPORT_EP_POLL:
if (!transport->ep_poll)
Expand All @@ -3149,12 +3153,15 @@ iscsi_if_transport_ep(struct iscsi_transport *transport,

static int
iscsi_tgt_dscvr(struct iscsi_transport *transport,
struct iscsi_uevent *ev)
struct iscsi_uevent *ev, u32 rlen)
{
struct Scsi_Host *shost;
struct sockaddr *dst_addr;
int err;

if (rlen < sizeof(*dst_addr))
return -EINVAL;

if (!transport->tgt_dscvr)
return -EINVAL;

Expand All @@ -3175,7 +3182,7 @@ iscsi_tgt_dscvr(struct iscsi_transport *transport,

static int
iscsi_set_host_param(struct iscsi_transport *transport,
struct iscsi_uevent *ev)
struct iscsi_uevent *ev, u32 rlen)
{
char *data = (char*)ev + sizeof(*ev);
struct Scsi_Host *shost;
Expand All @@ -3184,7 +3191,8 @@ iscsi_set_host_param(struct iscsi_transport *transport,
if (!transport->set_host_param)
return -ENOSYS;

if (ev->u.set_host_param.len > PAGE_SIZE)
if (ev->u.set_host_param.len > rlen ||
ev->u.set_host_param.len > PAGE_SIZE)
return -EINVAL;

shost = scsi_host_lookup(ev->u.set_host_param.host_no);
Expand All @@ -3201,12 +3209,15 @@ iscsi_set_host_param(struct iscsi_transport *transport,
}

static int
iscsi_set_path(struct iscsi_transport *transport, struct iscsi_uevent *ev)
iscsi_set_path(struct iscsi_transport *transport, struct iscsi_uevent *ev, u32 rlen)
{
struct Scsi_Host *shost;
struct iscsi_path *params;
int err;

if (rlen < sizeof(*params))
return -EINVAL;

if (!transport->set_path)
return -ENOSYS;

Expand Down Expand Up @@ -3266,12 +3277,15 @@ iscsi_set_iface_params(struct iscsi_transport *transport,
}

static int
iscsi_send_ping(struct iscsi_transport *transport, struct iscsi_uevent *ev)
iscsi_send_ping(struct iscsi_transport *transport, struct iscsi_uevent *ev, u32 rlen)
{
struct Scsi_Host *shost;
struct sockaddr *dst_addr;
int err;

if (rlen < sizeof(*dst_addr))
return -EINVAL;

if (!transport->send_ping)
return -ENOSYS;

Expand Down Expand Up @@ -3769,13 +3783,12 @@ iscsi_get_host_stats(struct iscsi_transport *transport, struct nlmsghdr *nlh)
}

static int iscsi_if_transport_conn(struct iscsi_transport *transport,
struct nlmsghdr *nlh)
struct nlmsghdr *nlh, u32 pdu_len)
{
struct iscsi_uevent *ev = nlmsg_data(nlh);
struct iscsi_cls_session *session;
struct iscsi_cls_conn *conn = NULL;
struct iscsi_endpoint *ep;
uint32_t pdu_len;
int err = 0;

switch (nlh->nlmsg_type) {
Expand Down Expand Up @@ -3860,8 +3873,6 @@ static int iscsi_if_transport_conn(struct iscsi_transport *transport,

break;
case ISCSI_UEVENT_SEND_PDU:
pdu_len = nlh->nlmsg_len - sizeof(*nlh) - sizeof(*ev);

if ((ev->u.send_pdu.hdr_size > pdu_len) ||
(ev->u.send_pdu.data_size > (pdu_len - ev->u.send_pdu.hdr_size))) {
err = -EINVAL;
Expand Down Expand Up @@ -3891,6 +3902,7 @@ iscsi_if_recv_msg(struct sk_buff *skb, struct nlmsghdr *nlh, uint32_t *group)
struct iscsi_internal *priv;
struct iscsi_cls_session *session;
struct iscsi_endpoint *ep = NULL;
u32 rlen;

if (!netlink_capable(skb, CAP_SYS_ADMIN))
return -EPERM;
Expand All @@ -3910,6 +3922,13 @@ iscsi_if_recv_msg(struct sk_buff *skb, struct nlmsghdr *nlh, uint32_t *group)

portid = NETLINK_CB(skb).portid;

/*
* Even though the remaining payload may not be regarded as nlattr,
* (like address or something else), calculate the remaining length
* here to ease following length checks.
*/
rlen = nlmsg_attrlen(nlh, sizeof(*ev));

switch (nlh->nlmsg_type) {
case ISCSI_UEVENT_CREATE_SESSION:
err = iscsi_if_create_session(priv, ep, ev,
Expand Down Expand Up @@ -3966,15 +3985,15 @@ iscsi_if_recv_msg(struct sk_buff *skb, struct nlmsghdr *nlh, uint32_t *group)
err = -EINVAL;
break;
case ISCSI_UEVENT_SET_PARAM:
err = iscsi_if_set_param(transport, ev);
err = iscsi_if_set_param(transport, ev, rlen);
break;
case ISCSI_UEVENT_CREATE_CONN:
case ISCSI_UEVENT_DESTROY_CONN:
case ISCSI_UEVENT_STOP_CONN:
case ISCSI_UEVENT_START_CONN:
case ISCSI_UEVENT_BIND_CONN:
case ISCSI_UEVENT_SEND_PDU:
err = iscsi_if_transport_conn(transport, nlh);
err = iscsi_if_transport_conn(transport, nlh, rlen);
break;
case ISCSI_UEVENT_GET_STATS:
err = iscsi_if_get_stats(transport, nlh);
Expand All @@ -3983,23 +4002,22 @@ iscsi_if_recv_msg(struct sk_buff *skb, struct nlmsghdr *nlh, uint32_t *group)
case ISCSI_UEVENT_TRANSPORT_EP_POLL:
case ISCSI_UEVENT_TRANSPORT_EP_DISCONNECT:
case ISCSI_UEVENT_TRANSPORT_EP_CONNECT_THROUGH_HOST:
err = iscsi_if_transport_ep(transport, ev, nlh->nlmsg_type);
err = iscsi_if_transport_ep(transport, ev, nlh->nlmsg_type, rlen);
break;
case ISCSI_UEVENT_TGT_DSCVR:
err = iscsi_tgt_dscvr(transport, ev);
err = iscsi_tgt_dscvr(transport, ev, rlen);
break;
case ISCSI_UEVENT_SET_HOST_PARAM:
err = iscsi_set_host_param(transport, ev);
err = iscsi_set_host_param(transport, ev, rlen);
break;
case ISCSI_UEVENT_PATH_UPDATE:
err = iscsi_set_path(transport, ev);
err = iscsi_set_path(transport, ev, rlen);
break;
case ISCSI_UEVENT_SET_IFACE_PARAMS:
err = iscsi_set_iface_params(transport, ev,
nlmsg_attrlen(nlh, sizeof(*ev)));
err = iscsi_set_iface_params(transport, ev, rlen);
break;
case ISCSI_UEVENT_PING:
err = iscsi_send_ping(transport, ev);
err = iscsi_send_ping(transport, ev, rlen);
break;
case ISCSI_UEVENT_GET_CHAP:
err = iscsi_get_chap(transport, nlh);
Expand All @@ -4008,13 +4026,10 @@ iscsi_if_recv_msg(struct sk_buff *skb, struct nlmsghdr *nlh, uint32_t *group)
err = iscsi_delete_chap(transport, ev);
break;
case ISCSI_UEVENT_SET_FLASHNODE_PARAMS:
err = iscsi_set_flashnode_param(transport, ev,
nlmsg_attrlen(nlh,
sizeof(*ev)));
err = iscsi_set_flashnode_param(transport, ev, rlen);
break;
case ISCSI_UEVENT_NEW_FLASHNODE:
err = iscsi_new_flashnode(transport, ev,
nlmsg_attrlen(nlh, sizeof(*ev)));
err = iscsi_new_flashnode(transport, ev, rlen);
break;
case ISCSI_UEVENT_DEL_FLASHNODE:
err = iscsi_del_flashnode(transport, ev);
Expand All @@ -4029,8 +4044,7 @@ iscsi_if_recv_msg(struct sk_buff *skb, struct nlmsghdr *nlh, uint32_t *group)
err = iscsi_logout_flashnode_sid(transport, ev);
break;
case ISCSI_UEVENT_SET_CHAP:
err = iscsi_set_chap(transport, ev,
nlmsg_attrlen(nlh, sizeof(*ev)));
err = iscsi_set_chap(transport, ev, rlen);
break;
case ISCSI_UEVENT_GET_HOST_STATS:
err = iscsi_get_host_stats(transport, nlh);
Expand Down

0 comments on commit bb8d101

Please sign in to comment.