Hello Guoqing Jiang,
Commit 9ddae3bab6d7 ("rnbd-clt: open code send_msg_open in
rnbd_clt_map_device") from Jul 6, 2022 (linux-next), leads to the
following Smatch static checker warning:
drivers/block/rnbd/rnbd-clt.c:1641 rnbd_clt_map_device()
warn: double destroy '&dev->lock' (orig line 1589)
drivers/block/rnbd/rnbd-clt.c
1527 struct rnbd_clt_dev *rnbd_clt_map_device(const char *sessname,
1528 struct rtrs_addr *paths,
1529 size_t path_cnt, u16 port_nr,
1530 const char *pathname,
1531 enum rnbd_access_mode access_mode,
1532 u32 nr_poll_queues)
1533 {
1534 struct rnbd_clt_session *sess;
1535 struct rnbd_clt_dev *dev;
1536 int ret, errno;
1537 struct rnbd_msg_open_rsp *rsp;
1538 struct rnbd_msg_open msg;
1539 struct rnbd_iu *iu;
1540 struct kvec vec = {
1541 .iov_base = &msg,
1542 .iov_len = sizeof(msg)
1543 };
1544
1545 if (exists_devpath(pathname, sessname))
1546 return ERR_PTR(-EEXIST);
1547
1548 sess = find_and_get_or_create_sess(sessname, paths, path_cnt, port_nr, nr_poll_queues);
1549 if (IS_ERR(sess))
1550 return ERR_CAST(sess);
1551
1552 dev = init_dev(sess, access_mode, pathname, nr_poll_queues);
1553 if (IS_ERR(dev)) {
1554 pr_err("map_device: failed to map device '%s' from session %s, can't initialize device, err: %pe\n",
1555 pathname, sess->sessname, dev);
1556 ret = PTR_ERR(dev);
1557 goto put_sess;
1558 }
1559 if (insert_dev_if_not_exists_devpath(dev)) {
1560 ret = -EEXIST;
1561 goto put_dev;
Should these error paths really call rnbd_clt_put_dev()?
1562 }
1563
1564 rsp = kzalloc(sizeof(*rsp), GFP_KERNEL);
1565 if (!rsp) {
1566 ret = -ENOMEM;
1567 goto del_dev;
1568 }
1569
1570 iu = rnbd_get_iu(sess, RTRS_ADMIN_CON, RTRS_PERMIT_WAIT);
1571 if (!iu) {
1572 ret = -ENOMEM;
1573 kfree(rsp);
1574 goto del_dev;
1575 }
1576 iu->buf = rsp;
1577 iu->dev = dev;
1578 sg_init_one(iu->sgt.sgl, rsp, sizeof(*rsp));
1579
1580 msg.hdr.type = cpu_to_le16(RNBD_MSG_OPEN);
1581 msg.access_mode = dev->access_mode;
1582 strscpy(msg.dev_name, dev->pathname, sizeof(msg.dev_name));
1583
1584 WARN_ON(!rnbd_clt_get_dev(dev));
The patch copied the rnbd_clt_get_dev() send_msg_open() to here. Why do we
need to take a second reference? The gotos above imply that we are already
holding a reference.
1585 ret = send_usr_msg(sess->rtrs, READ, iu,
1586 &vec, sizeof(*rsp), iu->sgt.sgl, 1,
1587 msg_open_conf, &errno, RTRS_PERMIT_WAIT);
1588 if (ret) {
1589 rnbd_clt_put_dev(dev);
1590 rnbd_put_iu(sess, iu);
And these two puts. Originally this failure path used to do a goto del_dev but
commit commit 52334f4a573d ("rnbd-clt: don't free rsp in msg_open_conf for map
scenario") changed it to goto put_iu so rnbd_put_iu() is called twice.
1591 } else {
1592 ret = errno;
1593 }
1594 if (ret) {
1595 rnbd_clt_err(dev,
1596 "map_device: failed, can't open remote device, err: %d\n",
1597 ret);
1598 goto put_iu;
^^^^^^^^^^^^
This goto here.
1599 }
1600 mutex_lock(&dev->lock);
1601 pr_debug("Opened remote device: session=%s, path='%s'\n",
1602 sess->sessname, pathname);
1603 ret = rnbd_client_setup_device(dev, rsp);
1604 if (ret) {
1605 rnbd_clt_err(dev,
1606 "map_device: Failed to configure device, err: %d\n",
1607 ret);
1608 mutex_unlock(&dev->lock);
1609 goto send_close;
1610 }
1611
1612 rnbd_clt_info(dev,
1613 "map_device: Device mapped as %s (nsectors: %llu, logical_block_size: %d, physical_block_size: %d, max_write_zeroes_sectors: %d, max_discard_sectors: %d, discard_granularity: %d, discard_alignment: %d, secure_discard: %d, max_segments: %d, max_hw_sectors: %d, wc: %d, fua: %d)\n",
1614 dev->gd->disk_name, le64_to_cpu(rsp->nsectors),
1615 le16_to_cpu(rsp->logical_block_size),
1616 le16_to_cpu(rsp->physical_block_size),
1617 le32_to_cpu(rsp->max_write_zeroes_sectors),
1618 le32_to_cpu(rsp->max_discard_sectors),
1619 le32_to_cpu(rsp->discard_granularity),
1620 le32_to_cpu(rsp->discard_alignment),
1621 le16_to_cpu(rsp->secure_discard),
1622 sess->max_segments, sess->max_io_size / SECTOR_SIZE,
1623 !!(rsp->cache_policy & RNBD_WRITEBACK),
1624 !!(rsp->cache_policy & RNBD_FUA));
1625
1626 mutex_unlock(&dev->lock);
1627 kfree(rsp);
1628 rnbd_put_iu(sess, iu);
1629 rnbd_clt_put_sess(sess);
1630
1631 return dev;
1632
1633 send_close:
1634 send_msg_close(dev, dev->device_id, RTRS_PERMIT_WAIT);
1635 put_iu:
1636 kfree(rsp);
1637 rnbd_put_iu(sess, iu);
^^^^^^^^^^^^^^^^^^^^^
1638 del_dev:
1639 delete_dev(dev);
1640 put_dev:
--> 1641 rnbd_clt_put_dev(dev);
^^^^^^^^^^^^^^^^^^^^^
1642 put_sess:
1643 rnbd_clt_put_sess(sess);
1644
1645 return ERR_PTR(ret);
1646 }
regards,
dan carpenter
