[bug report] [SCSI] iscsi_transport: Add support to display CHAP list and delete CHAP entry

[Dan Carpenter]

[ This code is obviously really old, but the warning may still be worth looking
at. -dan ]

Commit 6260a5d22122 ("[SCSI] iscsi_transport: Add support to display
CHAP list and delete CHAP entry") from Feb 27, 2012 (linux-next),
leads to the following Smatch static checker warning:

drivers/scsi/scsi_transport_iscsi.c:3341 iscsi_get_chap()
warn: potential user controlled sizeof overflow '56 + chap_buf_size' '56 + 0-u32max'

drivers/scsi/scsi_transport_iscsi.c
3319 static int
3320 iscsi_get_chap(struct iscsi_transport *transport, struct nlmsghdr *nlh)
3321 {
3322 struct iscsi_uevent *ev = nlmsg_data(nlh);

Smatch marks nlmsg_data() as untrusted.

3323 struct Scsi_Host *shost = NULL;
3324 struct iscsi_chap_rec *chap_rec;
3325 struct iscsi_internal *priv;
3326 struct sk_buff *skbchap;
3327 struct nlmsghdr *nlhchap;
3328 struct iscsi_uevent *evchap;
3329 uint32_t chap_buf_size;
3330 int len, err = 0;
3331 char *buf;
3332
3333 if (!transport->get_chap)
3334 return -EINVAL;
3335
3336 priv = iscsi_if_transport_lookup(transport);
3337 if (!priv)
3338 return -EINVAL;
3339
3340 chap_buf_size = (ev->u.get_chap.num_entries * sizeof(*chap_rec));

This warning is for 32 bits but it affects 64bit as well because chap_buf_size
is a u32. Potentially this "ev->u.get_chap.num_entries * sizeof(*chap_rec)"
multiply could integer wrap.

--> 3341 len = nlmsg_total_size(sizeof(*ev) + chap_buf_size);

Then nlmsg_total_size() has some addition and the + sizeof(*ev) as well and len
is stored as an int.

3342
3343 shost = scsi_host_lookup(ev->u.get_chap.host_no);
3344 if (!shost) {
3345 printk(KERN_ERR "%s: failed. Could not find host no %u\n",
3346 __func__, ev->u.get_chap.host_no);
3347 return -ENODEV;
3348 }
3349
3350 do {
3351 int actual_size;
3352
3353 skbchap = alloc_skb(len, GFP_KERNEL);
3354 if (!skbchap) {
3355 printk(KERN_ERR "can not deliver chap: OOM\n");
3356 err = -ENOMEM;
3357 goto exit_get_chap;
3358 }
3359
3360 nlhchap = __nlmsg_put(skbchap, 0, 0, 0,
3361 (len - sizeof(*nlhchap)), 0);
3362 evchap = nlmsg_data(nlhchap);
3363 memset(evchap, 0, sizeof(*evchap));
3364 evchap->transport_handle = iscsi_handle(transport);
3365 evchap->type = nlh->nlmsg_type;
3366 evchap->u.get_chap.host_no = ev->u.get_chap.host_no;
3367 evchap->u.get_chap.chap_tbl_idx = ev->u.get_chap.chap_tbl_idx;
3368 evchap->u.get_chap.num_entries = ev->u.get_chap.num_entries;
3369 buf = (char *)evchap + sizeof(*evchap);
3370 memset(buf, 0, chap_buf_size);
3371
3372 err = transport->get_chap(shost, ev->u.get_chap.chap_tbl_idx,
3373 &evchap->u.get_chap.num_entries, buf);
3374
3375 actual_size = nlmsg_total_size(sizeof(*ev) + chap_buf_size);
3376 skb_trim(skbchap, NLMSG_ALIGN(actual_size));
3377 nlhchap->nlmsg_len = actual_size;
3378
3379 err = iscsi_multicast_skb(skbchap, ISCSI_NL_GRP_ISCSID,
3380 GFP_KERNEL);
3381 } while (err < 0 && err != -ECONNREFUSED);
3382
3383 exit_get_chap:
3384 scsi_host_put(shost);
3385 return err;
3386 }

regards,
dan carpenter