LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: kernel test robot <rong.a.chen@intel.com>
To: Juergen Gross <jgross@suse.com>,
	xen-devel@lists.xenproject.org, linux-block@vger.kernel.org,
	linux-kernel@vger.kernel.org
Cc: kbuild-all@lists.01.org, "Juergen Gross" <jgross@suse.com>,
	"Boris Ostrovsky" <boris.ostrovsky@oracle.com>,
	"Stefano Stabellini" <sstabellini@kernel.org>,
	"Konrad Rzeszutek Wilk" <konrad.wilk@oracle.com>,
	"Roger Pau Monné" <roger.pau@citrix.com>,
	"Jens Axboe" <axboe@kernel.dk>
Subject: Re: [PATCH v2 3/3] xen/blkfront: don't trust the backend response data blindly
Date: Fri, 9 Jul 2021 19:09:06 +0800	[thread overview]
Message-ID: <20210709110906.GN2022171@shao2-debian> (raw)
In-Reply-To: <20210708124345.10173-4-jgross@suse.com>

[-- Attachment #1: Type: text/plain, Size: 14228 bytes --]

Hi Juergen,

I love your patch! Perhaps something to improve:

[auto build test WARNING on xen-tip/linux-next]
[also build test WARNING on next-20210708]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Juergen-Gross/xen-harden-blkfront-against-malicious-backends/20210708-204423
base:   https://git.kernel.org/pub/scm/linux/kernel/git/xen/tip.git linux-next
config: x86_64-rhel-8.3-kselftests (attached as .config)
compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
reproduce:
        # apt-get install sparse
        # sparse version: v0.6.3-341-g8af24329-dirty
        # https://github.com/0day-ci/linux/commit/26379fb9eaab91fc62eefa414619d27072941f59
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Juergen-Gross/xen-harden-blkfront-against-malicious-backends/20210708-204423
        git checkout 26379fb9eaab91fc62eefa414619d27072941f59
        # save the attached .config to linux build tree
        make W=1 C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' O=build_dir ARCH=x86_64 SHELL=/bin/bash

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>


sparse warnings: (new ones prefixed by >>)
>> drivers/block/xen-blkfront.c:1568:20: sparse: sparse: context imbalance in 'blkif_interrupt' - different lock contexts for basic block

vim +/blkif_interrupt +1568 drivers/block/xen-blkfront.c

9f27ee59503865 Jeremy Fitzhardinge   2007-07-17  1567  
9f27ee59503865 Jeremy Fitzhardinge   2007-07-17 @1568  static irqreturn_t blkif_interrupt(int irq, void *dev_id)
9f27ee59503865 Jeremy Fitzhardinge   2007-07-17  1569  {
9f27ee59503865 Jeremy Fitzhardinge   2007-07-17  1570  	struct request *req;
4c0a9a02397621 Juergen Gross         2021-07-08  1571  	struct blkif_response bret;
9f27ee59503865 Jeremy Fitzhardinge   2007-07-17  1572  	RING_IDX i, rp;
9f27ee59503865 Jeremy Fitzhardinge   2007-07-17  1573  	unsigned long flags;
81f35161577236 Bob Liu               2015-11-14  1574  	struct blkfront_ring_info *rinfo = (struct blkfront_ring_info *)dev_id;
81f35161577236 Bob Liu               2015-11-14  1575  	struct blkfront_info *info = rinfo->dev_info;
9f27ee59503865 Jeremy Fitzhardinge   2007-07-17  1576  
11659569f7202d Bob Liu               2015-11-14  1577  	if (unlikely(info->connected != BLKIF_STATE_CONNECTED))
9f27ee59503865 Jeremy Fitzhardinge   2007-07-17  1578  		return IRQ_HANDLED;
9f27ee59503865 Jeremy Fitzhardinge   2007-07-17  1579  
11659569f7202d Bob Liu               2015-11-14  1580  	spin_lock_irqsave(&rinfo->ring_lock, flags);
9f27ee59503865 Jeremy Fitzhardinge   2007-07-17  1581   again:
26379fb9eaab91 Juergen Gross         2021-07-08  1582  	rp = READ_ONCE(rinfo->ring.sring->rsp_prod);
26379fb9eaab91 Juergen Gross         2021-07-08  1583  	virt_rmb(); /* Ensure we see queued responses up to 'rp'. */
26379fb9eaab91 Juergen Gross         2021-07-08  1584  	if (RING_RESPONSE_PROD_OVERFLOW(&rinfo->ring, rp)) {
26379fb9eaab91 Juergen Gross         2021-07-08  1585  		pr_alert("%s: illegal number of responses %u\n",
26379fb9eaab91 Juergen Gross         2021-07-08  1586  			 info->gd->disk_name, rp - rinfo->ring.rsp_cons);
26379fb9eaab91 Juergen Gross         2021-07-08  1587  		goto err;
26379fb9eaab91 Juergen Gross         2021-07-08  1588  	}
9f27ee59503865 Jeremy Fitzhardinge   2007-07-17  1589  
81f35161577236 Bob Liu               2015-11-14  1590  	for (i = rinfo->ring.rsp_cons; i != rp; i++) {
9f27ee59503865 Jeremy Fitzhardinge   2007-07-17  1591  		unsigned long id;
26379fb9eaab91 Juergen Gross         2021-07-08  1592  		unsigned int op;
9f27ee59503865 Jeremy Fitzhardinge   2007-07-17  1593  
4c0a9a02397621 Juergen Gross         2021-07-08  1594  		RING_COPY_RESPONSE(&rinfo->ring, i, &bret);
4c0a9a02397621 Juergen Gross         2021-07-08  1595  		id = bret.id;
4c0a9a02397621 Juergen Gross         2021-07-08  1596  
6878c32e5cc0e4 Konrad Rzeszutek Wilk 2012-05-25  1597  		/*
6878c32e5cc0e4 Konrad Rzeszutek Wilk 2012-05-25  1598  		 * The backend has messed up and given us an id that we would
6878c32e5cc0e4 Konrad Rzeszutek Wilk 2012-05-25  1599  		 * never have given to it (we stamp it up to BLK_RING_SIZE -
6878c32e5cc0e4 Konrad Rzeszutek Wilk 2012-05-25  1600  		 * look in get_id_from_freelist.
6878c32e5cc0e4 Konrad Rzeszutek Wilk 2012-05-25  1601  		 */
86839c56dee28c Bob Liu               2015-06-03  1602  		if (id >= BLK_RING_SIZE(info)) {
26379fb9eaab91 Juergen Gross         2021-07-08  1603  			pr_alert("%s: response has incorrect id (%ld)\n",
26379fb9eaab91 Juergen Gross         2021-07-08  1604  				 info->gd->disk_name, id);
26379fb9eaab91 Juergen Gross         2021-07-08  1605  			goto err;
26379fb9eaab91 Juergen Gross         2021-07-08  1606  		}
26379fb9eaab91 Juergen Gross         2021-07-08  1607  		if (rinfo->shadow[id].status != REQ_WAITING) {
26379fb9eaab91 Juergen Gross         2021-07-08  1608  			pr_alert("%s: response references no pending request\n",
26379fb9eaab91 Juergen Gross         2021-07-08  1609  				 info->gd->disk_name);
26379fb9eaab91 Juergen Gross         2021-07-08  1610  			goto err;
6878c32e5cc0e4 Konrad Rzeszutek Wilk 2012-05-25  1611  		}
26379fb9eaab91 Juergen Gross         2021-07-08  1612  
26379fb9eaab91 Juergen Gross         2021-07-08  1613  		rinfo->shadow[id].status = REQ_PROCESSING;
81f35161577236 Bob Liu               2015-11-14  1614  		req  = rinfo->shadow[id].request;
9f27ee59503865 Jeremy Fitzhardinge   2007-07-17  1615  
26379fb9eaab91 Juergen Gross         2021-07-08  1616  		op = rinfo->shadow[id].req.operation;
26379fb9eaab91 Juergen Gross         2021-07-08  1617  		if (op == BLKIF_OP_INDIRECT)
26379fb9eaab91 Juergen Gross         2021-07-08  1618  			op = rinfo->shadow[id].req.u.indirect.indirect_op;
26379fb9eaab91 Juergen Gross         2021-07-08  1619  		if (bret.operation != op) {
26379fb9eaab91 Juergen Gross         2021-07-08  1620  			pr_alert("%s: response has wrong operation (%u instead of %u)\n",
26379fb9eaab91 Juergen Gross         2021-07-08  1621  				 info->gd->disk_name, bret.operation, op);
26379fb9eaab91 Juergen Gross         2021-07-08  1622  			goto err;
26379fb9eaab91 Juergen Gross         2021-07-08  1623  		}
26379fb9eaab91 Juergen Gross         2021-07-08  1624  
4c0a9a02397621 Juergen Gross         2021-07-08  1625  		if (bret.operation != BLKIF_OP_DISCARD) {
6cc5683390472c Julien Grall          2015-08-13  1626  			/*
6cc5683390472c Julien Grall          2015-08-13  1627  			 * We may need to wait for an extra response if the
6cc5683390472c Julien Grall          2015-08-13  1628  			 * I/O request is split in 2
6cc5683390472c Julien Grall          2015-08-13  1629  			 */
4c0a9a02397621 Juergen Gross         2021-07-08  1630  			if (!blkif_completion(&id, rinfo, &bret))
6cc5683390472c Julien Grall          2015-08-13  1631  				continue;
6cc5683390472c Julien Grall          2015-08-13  1632  		}
9f27ee59503865 Jeremy Fitzhardinge   2007-07-17  1633  
81f35161577236 Bob Liu               2015-11-14  1634  		if (add_id_to_freelist(rinfo, id)) {
6878c32e5cc0e4 Konrad Rzeszutek Wilk 2012-05-25  1635  			WARN(1, "%s: response to %s (id %ld) couldn't be recycled!\n",
4c0a9a02397621 Juergen Gross         2021-07-08  1636  			     info->gd->disk_name, op_name(bret.operation), id);
6878c32e5cc0e4 Konrad Rzeszutek Wilk 2012-05-25  1637  			continue;
6878c32e5cc0e4 Konrad Rzeszutek Wilk 2012-05-25  1638  		}
9f27ee59503865 Jeremy Fitzhardinge   2007-07-17  1639  
4c0a9a02397621 Juergen Gross         2021-07-08  1640  		if (bret.status == BLKIF_RSP_OKAY)
2a842acab109f4 Christoph Hellwig     2017-06-03  1641  			blkif_req(req)->error = BLK_STS_OK;
2a842acab109f4 Christoph Hellwig     2017-06-03  1642  		else
2a842acab109f4 Christoph Hellwig     2017-06-03  1643  			blkif_req(req)->error = BLK_STS_IOERR;
2a842acab109f4 Christoph Hellwig     2017-06-03  1644  
4c0a9a02397621 Juergen Gross         2021-07-08  1645  		switch (bret.operation) {
ed30bf317c5ceb Li Dongyang           2011-09-01  1646  		case BLKIF_OP_DISCARD:
4c0a9a02397621 Juergen Gross         2021-07-08  1647  			if (unlikely(bret.status == BLKIF_RSP_EOPNOTSUPP)) {
ed30bf317c5ceb Li Dongyang           2011-09-01  1648  				struct request_queue *rq = info->rq;
26379fb9eaab91 Juergen Gross         2021-07-08  1649  
26379fb9eaab91 Juergen Gross         2021-07-08  1650  				pr_warn_ratelimited("blkfront: %s: %s op failed\n",
4c0a9a02397621 Juergen Gross         2021-07-08  1651  					   info->gd->disk_name, op_name(bret.operation));
2a842acab109f4 Christoph Hellwig     2017-06-03  1652  				blkif_req(req)->error = BLK_STS_NOTSUPP;
ed30bf317c5ceb Li Dongyang           2011-09-01  1653  				info->feature_discard = 0;
5ea42986694a96 Konrad Rzeszutek Wilk 2011-10-12  1654  				info->feature_secdiscard = 0;
8b904b5b6b58b9 Bart Van Assche       2018-03-07  1655  				blk_queue_flag_clear(QUEUE_FLAG_DISCARD, rq);
8b904b5b6b58b9 Bart Van Assche       2018-03-07  1656  				blk_queue_flag_clear(QUEUE_FLAG_SECERASE, rq);
ed30bf317c5ceb Li Dongyang           2011-09-01  1657  			}
ed30bf317c5ceb Li Dongyang           2011-09-01  1658  			break;
edf6ef59ec7ee8 Konrad Rzeszutek Wilk 2011-05-03  1659  		case BLKIF_OP_FLUSH_DISKCACHE:
9f27ee59503865 Jeremy Fitzhardinge   2007-07-17  1660  		case BLKIF_OP_WRITE_BARRIER:
4c0a9a02397621 Juergen Gross         2021-07-08  1661  			if (unlikely(bret.status == BLKIF_RSP_EOPNOTSUPP)) {
26379fb9eaab91 Juergen Gross         2021-07-08  1662  				pr_warn_ratelimited("blkfront: %s: %s op failed\n",
4c0a9a02397621 Juergen Gross         2021-07-08  1663  				       info->gd->disk_name, op_name(bret.operation));
31c4ccc3ecb494 Bart Van Assche       2017-07-21  1664  				blkif_req(req)->error = BLK_STS_NOTSUPP;
dcb8baeceaa1c6 Jeremy Fitzhardinge   2010-11-02  1665  			}
4c0a9a02397621 Juergen Gross         2021-07-08  1666  			if (unlikely(bret.status == BLKIF_RSP_ERROR &&
81f35161577236 Bob Liu               2015-11-14  1667  				     rinfo->shadow[id].req.u.rw.nr_segments == 0)) {
26379fb9eaab91 Juergen Gross         2021-07-08  1668  				pr_warn_ratelimited("blkfront: %s: empty %s op failed\n",
4c0a9a02397621 Juergen Gross         2021-07-08  1669  				       info->gd->disk_name, op_name(bret.operation));
2a842acab109f4 Christoph Hellwig     2017-06-03  1670  				blkif_req(req)->error = BLK_STS_NOTSUPP;
dcb8baeceaa1c6 Jeremy Fitzhardinge   2010-11-02  1671  			}
2609587c1eeb4f Christoph Hellwig     2017-04-20  1672  			if (unlikely(blkif_req(req)->error)) {
2a842acab109f4 Christoph Hellwig     2017-06-03  1673  				if (blkif_req(req)->error == BLK_STS_NOTSUPP)
2a842acab109f4 Christoph Hellwig     2017-06-03  1674  					blkif_req(req)->error = BLK_STS_OK;
a418090aa88b9b Mike Christie         2016-06-05  1675  				info->feature_fua = 0;
4913efe456c987 Tejun Heo             2010-09-03  1676  				info->feature_flush = 0;
4913efe456c987 Tejun Heo             2010-09-03  1677  				xlvbd_flush(info);
9f27ee59503865 Jeremy Fitzhardinge   2007-07-17  1678  			}
df561f6688fef7 Gustavo A. R. Silva   2020-08-23  1679  			fallthrough;
9f27ee59503865 Jeremy Fitzhardinge   2007-07-17  1680  		case BLKIF_OP_READ:
9f27ee59503865 Jeremy Fitzhardinge   2007-07-17  1681  		case BLKIF_OP_WRITE:
4c0a9a02397621 Juergen Gross         2021-07-08  1682  			if (unlikely(bret.status != BLKIF_RSP_OKAY))
26379fb9eaab91 Juergen Gross         2021-07-08  1683  				dev_dbg_ratelimited(&info->xbdev->dev,
26379fb9eaab91 Juergen Gross         2021-07-08  1684  					"Bad return from blkdev data request: %x\n", bret.status);
9f27ee59503865 Jeremy Fitzhardinge   2007-07-17  1685  
9f27ee59503865 Jeremy Fitzhardinge   2007-07-17  1686  			break;
9f27ee59503865 Jeremy Fitzhardinge   2007-07-17  1687  		default:
9f27ee59503865 Jeremy Fitzhardinge   2007-07-17  1688  			BUG();
9f27ee59503865 Jeremy Fitzhardinge   2007-07-17  1689  		}
2609587c1eeb4f Christoph Hellwig     2017-04-20  1690  
15f73f5b3e5958 Christoph Hellwig     2020-06-11  1691  		if (likely(!blk_should_fake_timeout(req->q)))
08e0029aa2a4ac Christoph Hellwig     2017-04-20  1692  			blk_mq_complete_request(req);
9f27ee59503865 Jeremy Fitzhardinge   2007-07-17  1693  	}
9f27ee59503865 Jeremy Fitzhardinge   2007-07-17  1694  
81f35161577236 Bob Liu               2015-11-14  1695  	rinfo->ring.rsp_cons = i;
9f27ee59503865 Jeremy Fitzhardinge   2007-07-17  1696  
81f35161577236 Bob Liu               2015-11-14  1697  	if (i != rinfo->ring.req_prod_pvt) {
9f27ee59503865 Jeremy Fitzhardinge   2007-07-17  1698  		int more_to_do;
81f35161577236 Bob Liu               2015-11-14  1699  		RING_FINAL_CHECK_FOR_RESPONSES(&rinfo->ring, more_to_do);
9f27ee59503865 Jeremy Fitzhardinge   2007-07-17  1700  		if (more_to_do)
9f27ee59503865 Jeremy Fitzhardinge   2007-07-17  1701  			goto again;
9f27ee59503865 Jeremy Fitzhardinge   2007-07-17  1702  	} else
81f35161577236 Bob Liu               2015-11-14  1703  		rinfo->ring.sring->rsp_event = i + 1;
9f27ee59503865 Jeremy Fitzhardinge   2007-07-17  1704  
11659569f7202d Bob Liu               2015-11-14  1705  	kick_pending_request_queues_locked(rinfo);
9f27ee59503865 Jeremy Fitzhardinge   2007-07-17  1706  
11659569f7202d Bob Liu               2015-11-14  1707  	spin_unlock_irqrestore(&rinfo->ring_lock, flags);
9f27ee59503865 Jeremy Fitzhardinge   2007-07-17  1708  
9f27ee59503865 Jeremy Fitzhardinge   2007-07-17  1709  	return IRQ_HANDLED;
26379fb9eaab91 Juergen Gross         2021-07-08  1710  
26379fb9eaab91 Juergen Gross         2021-07-08  1711   err:
26379fb9eaab91 Juergen Gross         2021-07-08  1712  	info->connected = BLKIF_STATE_ERROR;
26379fb9eaab91 Juergen Gross         2021-07-08  1713  	pr_alert("%s disabled for further use\n", info->gd->disk_name);
26379fb9eaab91 Juergen Gross         2021-07-08  1714  	return IRQ_HANDLED;
9f27ee59503865 Jeremy Fitzhardinge   2007-07-17  1715  }
9f27ee59503865 Jeremy Fitzhardinge   2007-07-17  1716  

---
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: 41910 bytes --]

[-- Attachment #3: Type: text/plain, Size: 149 bytes --]

_______________________________________________
kbuild mailing list -- kbuild@lists.01.org
To unsubscribe send an email to kbuild-leave@lists.01.org

  parent reply	other threads:[~2021-07-09 11:09 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-07-08 12:43 [PATCH v2 0/3] xen: harden blkfront against malicious backends Juergen Gross
2021-07-08 12:43 ` [PATCH v2 1/3] xen/blkfront: read response from backend only once Juergen Gross
2021-07-09  8:33   ` Roger Pau Monné
2021-07-08 12:43 ` [PATCH v2 2/3] xen/blkfront: don't take local copy of a request from the ring page Juergen Gross
2021-07-09  8:55   ` Roger Pau Monné
2021-07-09 13:54     ` Juergen Gross
2021-07-08 12:43 ` [PATCH v2 3/3] xen/blkfront: don't trust the backend response data blindly Juergen Gross
2021-07-08 13:11   ` Jan Beulich
2021-07-08 13:14     ` Juergen Gross
2021-07-09  9:42   ` Roger Pau Monné
2021-07-09 13:58     ` Juergen Gross
2021-07-09 11:09   ` kernel test robot [this message]
2021-07-30 10:08   ` Juergen Gross
2021-07-30 10:31     ` Juergen Gross
2021-07-08 14:22 ` [PATCH v2 0/3] xen: harden blkfront against malicious backends Konrad Rzeszutek Wilk
2021-07-08 14:39   ` Juergen Gross
2021-07-10  1:18     ` Marek Marczykowski-Górecki

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=20210709110906.GN2022171@shao2-debian \
    --to=rong.a.chen@intel.com \
    --cc=axboe@kernel.dk \
    --cc=boris.ostrovsky@oracle.com \
    --cc=jgross@suse.com \
    --cc=kbuild-all@lists.01.org \
    --cc=konrad.wilk@oracle.com \
    --cc=linux-block@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=roger.pau@citrix.com \
    --cc=sstabellini@kernel.org \
    --cc=xen-devel@lists.xenproject.org \
    --subject='Re: [PATCH v2 3/3] xen/blkfront: don'\''t trust the backend response data blindly' \
    /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

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).