From: Dan Carpenter <dan.carpenter@oracle.com>
To: kbuild@lists.01.org, Mike Christie <michael.christie@oracle.com>,
lduncan@suse.com, cleech@redhat.com, martin.petersen@oracle.com,
linux-scsi@vger.kernel.org,
james.bottomley@hansenpartnership.com
Cc: lkp@intel.com, kbuild-all@lists.01.org, lutianxiong@huawei.com,
linfeilong@huawei.com, liuzhiqiang26@huawei.com,
haowenchao@huawei.com,
Mike Christie <michael.christie@oracle.com>
Subject: Re: [PATCH 2/9] libiscsi: drop taskqueuelock
Date: Wed, 3 Feb 2021 13:19:42 +0300 [thread overview]
Message-ID: <20210203101942.GU2696@kadam> (raw)
In-Reply-To: <20210203013356.11177-3-michael.christie@oracle.com>
[-- Attachment #1: Type: text/plain, Size: 10189 bytes --]
Hi Mike,
url: https://github.com/0day-ci/linux/commits/Mike-Christie/iscsi-fixes-and-cleanups/20210203-122757
base: https://git.kernel.org/pub/scm/linux/kernel/git/jejb/scsi.git for-next
config: i386-randconfig-m021-20210202 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-15) 9.3.0
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
smatch warnings:
drivers/scsi/libiscsi_tcp.c:586 iscsi_tcp_r2t_rsp() warn: variable dereferenced before check 'task->sc' (see line 547)
vim +586 drivers/scsi/libiscsi_tcp.c
f7dbf0662a0167 Mike Christie 2021-02-02 529 static int iscsi_tcp_r2t_rsp(struct iscsi_conn *conn, struct iscsi_hdr *hdr)
a081c13e39b5c1 Mike Christie 2008-12-02 530 {
a081c13e39b5c1 Mike Christie 2008-12-02 531 struct iscsi_session *session = conn->session;
f7dbf0662a0167 Mike Christie 2021-02-02 532 struct iscsi_tcp_task *tcp_task;
f7dbf0662a0167 Mike Christie 2021-02-02 533 struct iscsi_tcp_conn *tcp_conn;
f7dbf0662a0167 Mike Christie 2021-02-02 534 struct iscsi_r2t_rsp *rhdr;
a081c13e39b5c1 Mike Christie 2008-12-02 535 struct iscsi_r2t_info *r2t;
f7dbf0662a0167 Mike Christie 2021-02-02 536 struct iscsi_task *task;
5d0fddd0a72d30 Shlomo Pongratz 2014-02-07 537 u32 data_length;
5d0fddd0a72d30 Shlomo Pongratz 2014-02-07 538 u32 data_offset;
f7dbf0662a0167 Mike Christie 2021-02-02 539 int r2tsn;
a081c13e39b5c1 Mike Christie 2008-12-02 540 int rc;
a081c13e39b5c1 Mike Christie 2008-12-02 541
f7dbf0662a0167 Mike Christie 2021-02-02 542 spin_lock(&session->back_lock);
f7dbf0662a0167 Mike Christie 2021-02-02 543 task = iscsi_itt_to_ctask(conn, hdr->itt);
f7dbf0662a0167 Mike Christie 2021-02-02 544 if (!task) {
f7dbf0662a0167 Mike Christie 2021-02-02 545 spin_unlock(&session->back_lock);
f7dbf0662a0167 Mike Christie 2021-02-02 546 return ISCSI_ERR_BAD_ITT;
f7dbf0662a0167 Mike Christie 2021-02-02 @547 } else if (task->sc->sc_data_direction != DMA_TO_DEVICE) {
^^^^^^^^
New unchecked dereference.
f7dbf0662a0167 Mike Christie 2021-02-02 548 spin_unlock(&session->back_lock);
f7dbf0662a0167 Mike Christie 2021-02-02 549 return ISCSI_ERR_PROTO;
f7dbf0662a0167 Mike Christie 2021-02-02 550 }
f7dbf0662a0167 Mike Christie 2021-02-02 551 /*
f7dbf0662a0167 Mike Christie 2021-02-02 552 * A bad target might complete the cmd before we have handled R2Ts
f7dbf0662a0167 Mike Christie 2021-02-02 553 * so get a ref to the task that will be dropped in the xmit path.
f7dbf0662a0167 Mike Christie 2021-02-02 554 */
f7dbf0662a0167 Mike Christie 2021-02-02 555 if (task->state != ISCSI_TASK_RUNNING) {
f7dbf0662a0167 Mike Christie 2021-02-02 556 spin_unlock(&session->back_lock);
f7dbf0662a0167 Mike Christie 2021-02-02 557 /* Let the path that got the early rsp complete it */
f7dbf0662a0167 Mike Christie 2021-02-02 558 return 0;
f7dbf0662a0167 Mike Christie 2021-02-02 559 }
f7dbf0662a0167 Mike Christie 2021-02-02 560 task->last_xfer = jiffies;
f7dbf0662a0167 Mike Christie 2021-02-02 561 __iscsi_get_task(task);
f7dbf0662a0167 Mike Christie 2021-02-02 562
f7dbf0662a0167 Mike Christie 2021-02-02 563 tcp_conn = conn->dd_data;
f7dbf0662a0167 Mike Christie 2021-02-02 564 rhdr = (struct iscsi_r2t_rsp *)tcp_conn->in.hdr;
f7dbf0662a0167 Mike Christie 2021-02-02 565 /* fill-in new R2T associated with the task */
f7dbf0662a0167 Mike Christie 2021-02-02 566 iscsi_update_cmdsn(session, (struct iscsi_nopin *)rhdr);
f7dbf0662a0167 Mike Christie 2021-02-02 567 spin_unlock(&session->back_lock);
f7dbf0662a0167 Mike Christie 2021-02-02 568
a081c13e39b5c1 Mike Christie 2008-12-02 569 if (tcp_conn->in.datalen) {
a081c13e39b5c1 Mike Christie 2008-12-02 570 iscsi_conn_printk(KERN_ERR, conn,
a081c13e39b5c1 Mike Christie 2008-12-02 571 "invalid R2t with datalen %d\n",
a081c13e39b5c1 Mike Christie 2008-12-02 572 tcp_conn->in.datalen);
f7dbf0662a0167 Mike Christie 2021-02-02 573 rc = ISCSI_ERR_DATALEN;
f7dbf0662a0167 Mike Christie 2021-02-02 574 goto put_task;
a081c13e39b5c1 Mike Christie 2008-12-02 575 }
a081c13e39b5c1 Mike Christie 2008-12-02 576
f7dbf0662a0167 Mike Christie 2021-02-02 577 tcp_task = task->dd_data;
f7dbf0662a0167 Mike Christie 2021-02-02 578 r2tsn = be32_to_cpu(rhdr->r2tsn);
a081c13e39b5c1 Mike Christie 2008-12-02 579 if (tcp_task->exp_datasn != r2tsn){
0ab1c2529e6a70 Mike Christie 2009-03-05 580 ISCSI_DBG_TCP(conn, "task->exp_datasn(%d) != rhdr->r2tsn(%d)\n",
0ab1c2529e6a70 Mike Christie 2009-03-05 581 tcp_task->exp_datasn, r2tsn);
f7dbf0662a0167 Mike Christie 2021-02-02 582 rc = ISCSI_ERR_R2TSN;
f7dbf0662a0167 Mike Christie 2021-02-02 583 goto put_task;
a081c13e39b5c1 Mike Christie 2008-12-02 584 }
a081c13e39b5c1 Mike Christie 2008-12-02 585
a081c13e39b5c1 Mike Christie 2008-12-02 @586 if (!task->sc || session->state != ISCSI_STATE_LOGGED_IN) {
^^^^^^^^
Checked too late.
a081c13e39b5c1 Mike Christie 2008-12-02 587 iscsi_conn_printk(KERN_INFO, conn,
a081c13e39b5c1 Mike Christie 2008-12-02 588 "dropping R2T itt %d in recovery.\n",
a081c13e39b5c1 Mike Christie 2008-12-02 589 task->itt);
f7dbf0662a0167 Mike Christie 2021-02-02 590 rc = 0;
f7dbf0662a0167 Mike Christie 2021-02-02 591 goto put_task;
a081c13e39b5c1 Mike Christie 2008-12-02 592 }
a081c13e39b5c1 Mike Christie 2008-12-02 593
5d0fddd0a72d30 Shlomo Pongratz 2014-02-07 594 data_length = be32_to_cpu(rhdr->data_length);
5d0fddd0a72d30 Shlomo Pongratz 2014-02-07 595 if (data_length == 0) {
a081c13e39b5c1 Mike Christie 2008-12-02 596 iscsi_conn_printk(KERN_ERR, conn,
a081c13e39b5c1 Mike Christie 2008-12-02 597 "invalid R2T with zero data len\n");
f7dbf0662a0167 Mike Christie 2021-02-02 598 rc = ISCSI_ERR_DATALEN;
f7dbf0662a0167 Mike Christie 2021-02-02 599 goto put_task;
a081c13e39b5c1 Mike Christie 2008-12-02 600 }
a081c13e39b5c1 Mike Christie 2008-12-02 601
5d0fddd0a72d30 Shlomo Pongratz 2014-02-07 602 if (data_length > session->max_burst)
0ab1c2529e6a70 Mike Christie 2009-03-05 603 ISCSI_DBG_TCP(conn, "invalid R2T with data len %u and max "
0ab1c2529e6a70 Mike Christie 2009-03-05 604 "burst %u. Attempting to execute request.\n",
5d0fddd0a72d30 Shlomo Pongratz 2014-02-07 605 data_length, session->max_burst);
a081c13e39b5c1 Mike Christie 2008-12-02 606
5d0fddd0a72d30 Shlomo Pongratz 2014-02-07 607 data_offset = be32_to_cpu(rhdr->data_offset);
ae3d56d81507c3 Christoph Hellwig 2019-01-29 608 if (data_offset + data_length > task->sc->sdb.length) {
a081c13e39b5c1 Mike Christie 2008-12-02 609 iscsi_conn_printk(KERN_ERR, conn,
a081c13e39b5c1 Mike Christie 2008-12-02 610 "invalid R2T with data len %u at offset %u "
5d0fddd0a72d30 Shlomo Pongratz 2014-02-07 611 "and total length %d\n", data_length,
ae3d56d81507c3 Christoph Hellwig 2019-01-29 612 data_offset, task->sc->sdb.length);
f7dbf0662a0167 Mike Christie 2021-02-02 613 rc = ISCSI_ERR_DATALEN;
f7dbf0662a0167 Mike Christie 2021-02-02 614 goto put_task;
a081c13e39b5c1 Mike Christie 2008-12-02 615 }
a081c13e39b5c1 Mike Christie 2008-12-02 616
659743b02c4110 Shlomo Pongratz 2014-02-07 617 spin_lock(&tcp_task->pool2queue);
5d0fddd0a72d30 Shlomo Pongratz 2014-02-07 618 rc = kfifo_out(&tcp_task->r2tpool.queue, (void *)&r2t, sizeof(void *));
5d0fddd0a72d30 Shlomo Pongratz 2014-02-07 619 if (!rc) {
5d0fddd0a72d30 Shlomo Pongratz 2014-02-07 620 iscsi_conn_printk(KERN_ERR, conn, "Could not allocate R2T. "
5d0fddd0a72d30 Shlomo Pongratz 2014-02-07 621 "Target has sent more R2Ts than it "
5d0fddd0a72d30 Shlomo Pongratz 2014-02-07 622 "negotiated for or driver has leaked.\n");
659743b02c4110 Shlomo Pongratz 2014-02-07 623 spin_unlock(&tcp_task->pool2queue);
f7dbf0662a0167 Mike Christie 2021-02-02 624 rc = ISCSI_ERR_PROTO;
f7dbf0662a0167 Mike Christie 2021-02-02 625 goto put_task;
5d0fddd0a72d30 Shlomo Pongratz 2014-02-07 626 }
5d0fddd0a72d30 Shlomo Pongratz 2014-02-07 627
5d0fddd0a72d30 Shlomo Pongratz 2014-02-07 628 r2t->exp_statsn = rhdr->statsn;
5d0fddd0a72d30 Shlomo Pongratz 2014-02-07 629 r2t->data_length = data_length;
5d0fddd0a72d30 Shlomo Pongratz 2014-02-07 630 r2t->data_offset = data_offset;
5d0fddd0a72d30 Shlomo Pongratz 2014-02-07 631
a081c13e39b5c1 Mike Christie 2008-12-02 632 r2t->ttt = rhdr->ttt; /* no flip */
a081c13e39b5c1 Mike Christie 2008-12-02 633 r2t->datasn = 0;
a081c13e39b5c1 Mike Christie 2008-12-02 634 r2t->sent = 0;
a081c13e39b5c1 Mike Christie 2008-12-02 635
a081c13e39b5c1 Mike Christie 2008-12-02 636 tcp_task->exp_datasn = r2tsn + 1;
7acd72eb85f1c7 Stefani Seibold 2009-12-21 637 kfifo_in(&tcp_task->r2tqueue, (void*)&r2t, sizeof(void*));
a081c13e39b5c1 Mike Christie 2008-12-02 638 conn->r2t_pdus_cnt++;
659743b02c4110 Shlomo Pongratz 2014-02-07 639 spin_unlock(&tcp_task->pool2queue);
a081c13e39b5c1 Mike Christie 2008-12-02 640
a081c13e39b5c1 Mike Christie 2008-12-02 641 iscsi_requeue_task(task);
a081c13e39b5c1 Mike Christie 2008-12-02 642 return 0;
f7dbf0662a0167 Mike Christie 2021-02-02 643
f7dbf0662a0167 Mike Christie 2021-02-02 644 put_task:
f7dbf0662a0167 Mike Christie 2021-02-02 645 iscsi_put_task(task);
f7dbf0662a0167 Mike Christie 2021-02-02 646 return rc;
a081c13e39b5c1 Mike Christie 2008-12-02 647 }
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 38567 bytes --]
next prev parent reply other threads:[~2021-02-03 10:21 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-02-03 1:33 [PATCH 0/9 V5] iscsi fixes and cleanups Mike Christie
2021-02-03 1:33 ` [PATCH 1/9] libiscsi: fix iscsi_prep_scsi_cmd_pdu error handling Mike Christie
2021-02-03 1:33 ` [PATCH 2/9] libiscsi: drop taskqueuelock Mike Christie
2021-02-03 10:19 ` Dan Carpenter [this message]
2021-02-03 17:10 ` Mike Christie
2021-02-03 1:33 ` [PATCH 3/9] libiscsi: fix iscsi_task use after free Mike Christie
2021-02-03 1:33 ` [PATCH 4/9] libiscsi: fix iscsi host workq destruction Mike Christie
2021-02-03 1:33 ` [PATCH 5/9] libiscsi: add helper to calc max scsi cmds per session Mike Christie
2021-02-03 1:33 ` [PATCH 6/9] iscsi_tcp: fix shost can_queue initialization Mike Christie
2021-02-03 23:33 ` Lee Duncan
2021-02-03 1:33 ` [PATCH 7/9] libiscsi: reset max/exp cmdsn during recovery Mike Christie
2021-02-03 1:33 ` [PATCH 8/9] qla4xxx: use iscsi_is_session_online Mike Christie
2021-02-03 1:33 ` [PATCH 9/9] iscsi class: drop session lock in iscsi_session_chkready Mike Christie
-- strict thread matches above, loose matches on Subject: below --
2021-02-07 4:45 [PATCH 0/9 V6] iscsi fixes and cleanups Mike Christie
2021-02-07 4:46 ` [PATCH 2/9] libiscsi: drop taskqueuelock Mike Christie
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20210203101942.GU2696@kadam \
--to=dan.carpenter@oracle.com \
--cc=cleech@redhat.com \
--cc=haowenchao@huawei.com \
--cc=james.bottomley@hansenpartnership.com \
--cc=kbuild-all@lists.01.org \
--cc=kbuild@lists.01.org \
--cc=lduncan@suse.com \
--cc=linfeilong@huawei.com \
--cc=linux-scsi@vger.kernel.org \
--cc=liuzhiqiang26@huawei.com \
--cc=lkp@intel.com \
--cc=lutianxiong@huawei.com \
--cc=martin.petersen@oracle.com \
--cc=michael.christie@oracle.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox