LinuxLists.cc - [broonie-ci:20220430_marex_regmap_add_bulk_read_write_callbacks_into_regmap_config 1/1] drivers/base/regmap/regmap.c:1842 _regmap_raw_write_impl() error: we previously assumed 'map->bus' could be null (see line 1770)

2022-05-09 06:43:44

Subject: [broonie-ci:20220430_marex_regmap_add_bulk_read_write_callbacks_into_regmap_config 1/1] drivers/base/regmap/regmap.c:1842 _regmap_raw_write_impl() error: we previously assumed 'map->bus' could be null (see line 1770)

tree:
/git.kernel.org/pub/scm/linux/kernel/git/broonie/ci.git">https://git.kernel.org/pub/scm/linux/kernel/git/broonie/ci.git 20220430_marex_regmap_add_bulk_read_write_callbacks_into_regmap_config
08470402e2b623dcd769681
08470402e2b623dcd769681 [1/1] regmap: Add bulk read/write callbacks into regmap_config
(https://download.01.org/0day-ci/archive/20220506/[email protected]/config)
(GCC) 11.3.0
kindly add following tag as appropriate
test robot <[email protected]>
Carpenter <[email protected]>
_regmap_raw_write_impl() error: we previously assumed 'map->bus' could be null (see line 1770)
gmap/regmap.c
2018-02-22 1677 static int _regmap_raw_write_impl(struct regmap *map, unsigned int reg,
2020-09-17 1678 const void *val, size_t val_len, bool noinc)
2011-05-11 1679 {
2012-10-04 1680 struct regmap_range_node *range;
2013-01-27 1681 unsigned long flags;
2013-01-27 1682 void *work_val = map->work_buf + map->format.reg_bytes +
2013-01-27 1683 map->format.pad_bytes;
2011-05-11 1684 void *buf;
2011-05-11 1685 int ret = -ENOTSUPP;
2011-05-11 1686 size_t len;
2011-07-24 1687 int i;
2011-07-24 1688
2020-01-18 1689 /* Check for unwritable or noinc registers in range
2020-01-18 1690 * before we start
2020-01-18 1691 */
2020-01-18 1692 if (!regmap_writeable_noinc(map, reg)) {
2020-01-18 1693 for (i = 0; i < val_len / map->format.val_bytes; i++) {
2020-01-18 1694 unsigned int element =
2020-01-18 1695 reg + regmap_get_offset(map, i);
2020-01-18 1696 if (!regmap_writeable(map, element) ||
2020-01-18 1697 regmap_writeable_noinc(map, element))
2011-07-24 1698 return -EINVAL;
2020-01-18 1699 }
2020-01-18 1700 }
2011-05-11 1701
2012-02-10 1702 if (!map->cache_bypass && map->format.parse_val) {
2012-02-10 1703 unsigned int ival;
2012-02-10 1704 int val_bytes = map->format.val_bytes;
2012-02-10 1705 for (i = 0; i < val_len / val_bytes; i++) {
2013-03-20 1706 ival = map->format.parse_val(val + (i * val_bytes));
2016-01-04 1707 ret = regcache_write(map,
2016-01-04 1708 reg + regmap_get_offset(map, i),
2012-04-09 1709 ival);
2012-02-10 1710 if (ret) {
2012-02-10 1711 dev_err(map->dev,
2012-10-26 1712 "Error in caching of register: %x ret: %d\n",
2021-07-01 1713 reg + regmap_get_offset(map, i), ret);
2012-02-10 1714 return ret;
2012-02-10 1715 }
2012-02-10 1716 }
2012-02-10 1717 if (map->cache_only) {
2012-02-10 1718 map->cache_dirty = true;
2012-02-10 1719 return 0;
2012-02-10 1720 }
2012-02-10 1721 }
2012-02-10 1722
2012-10-04 1723 range = _regmap_range_lookup(map, reg);
2012-10-04 1724 if (range) {
2012-10-04 1725 int val_num = val_len / map->format.val_bytes;
2012-10-04 1726 int win_offset = (reg - range->range_min) % range->window_len;
2012-10-04 1727 int win_residue = range->window_len - win_offset;
2012-10-04 1728
2012-10-04 1729 /* If the write goes beyond the end of the window split it */
2012-10-04 1730 while (val_num > win_residue) {
2012-10-25 1731 dev_dbg(map->dev, "Writing window %d/%zu\n",
2012-10-04 1732 win_residue, val_len / map->format.val_bytes);
2018-02-22 1733 ret = _regmap_raw_write_impl(map, reg, val,
2018-02-22 1734 win_residue *
2020-09-17 1735 map->format.val_bytes, noinc);
2012-10-04 1736 if (ret != 0)
2012-10-04 1737 return ret;
2012-10-04 1738
2012-10-04 1739 reg += win_residue;
2012-10-04 1740 val_num -= win_residue;
2012-10-04 1741 val += win_residue * map->format.val_bytes;
2012-10-04 1742 val_len -= win_residue * map->format.val_bytes;
2012-10-04 1743
2012-10-04 1744 win_offset = (reg - range->range_min) %
2012-10-04 1745 range->window_len;
2012-10-04 1746 win_residue = range->window_len - win_offset;
2012-10-04 1747 }
2012-10-04 1748
2020-09-17 1749 ret = _regmap_select_page(map, &reg, range, noinc ? 1 : val_num);
2012-10-04 1750 if (ret != 0)
2012-06-15 1751 return ret;
2012-10-04 1752 }
2012-06-15 1753
2022-03-13 1754 reg += map->reg_base;
2022-03-13 1755 reg >>= map->format.reg_downshift;
2012-03-16 1756 map->format.format_reg(map->work_buf, reg, map->reg_shift);
2016-09-15 1757 regmap_set_work_buf_flag_mask(map, map->format.reg_bytes,
2016-09-15 1758 map->write_flag_mask);
2011-09-05 1759
2013-10-08 1760 /*
2013-10-08 1761 * Essentially all I/O mechanisms will be faster with a single
2013-10-08 1762 * buffer to write. Since register syncs often generate raw
2013-10-08 1763 * writes of single registers optimise that case.
2013-10-08 1764 */
2013-10-08 1765 if (val != work_val && val_len == map->format.val_bytes) {
2013-10-08 1766 memcpy(work_val, val, map->format.val_bytes);
2013-10-08 1767 val = work_val;
2013-10-08 1768 }
2013-10-08 1769
2022-04-30 @1770 if (map->async && map->bus && map->bus->async_write) {
^^^^^^^^
to check map->bus?
2013-10-07 1771 struct regmap_async *async;
2013-01-27 1772
2015-03-09 1773 trace_regmap_async_write_start(map, reg, val_len);
2013-02-21 1774
2013-10-07 1775 spin_lock_irqsave(&map->async_lock, flags);
2013-10-07 1776 async = list_first_entry_or_null(&map->async_free,
2013-10-07 1777 struct regmap_async,
2013-10-07 1778 list);
2013-10-07 1779 if (async)
2013-10-07 1780 list_del(&async->list);
2013-10-07 1781 spin_unlock_irqrestore(&map->async_lock, flags);
2013-10-07 1782
2013-10-07 1783 if (!async) {
2013-10-07 1784 async = map->bus->async_alloc();
2013-10-07 1785 if (!async)
2013-10-07 1786 return -ENOMEM;
2013-10-07 1787
2013-01-27 1788 async->work_buf = kzalloc(map->format.buf_size,
2013-01-27 1789 GFP_KERNEL | GFP_DMA);
2013-01-27 1790 if (!async->work_buf) {
2013-01-27 1791 kfree(async);
2013-01-27 1792 return -ENOMEM;
2013-01-27 1793 }
2013-10-07 1794 }
2013-01-27 1795
2013-01-27 1796 async->map = map;
2013-01-27 1797
2013-01-27 1798 /* If the caller supplied the value we can use it safely. */
2013-01-27 1799 memcpy(async->work_buf, map->work_buf, map->format.pad_bytes +
2013-01-27 1800 map->format.reg_bytes + map->format.val_bytes);
2013-01-27 1801
2013-01-27 1802 spin_lock_irqsave(&map->async_lock, flags);
2013-01-27 1803 list_add_tail(&async->list, &map->async_list);
2013-01-27 1804 spin_unlock_irqrestore(&map->async_lock, flags);
2013-01-27 1805
2013-10-10 1806 if (val != work_val)
2013-10-10 1807 ret = map->bus->async_write(map->bus_context,
2013-10-10 1808 async->work_buf,
2013-01-27 1809 map->format.reg_bytes +
2013-01-27 1810 map->format.pad_bytes,
2013-01-27 1811 val, val_len, async);
2013-10-10 1812 else
2013-10-10 1813 ret = map->bus->async_write(map->bus_context,
2013-10-10 1814 async->work_buf,
2013-10-10 1815 map->format.reg_bytes +
2013-10-10 1816 map->format.pad_bytes +
2013-10-10 1817 val_len, NULL, 0, async);
2013-01-27 1818
2013-01-27 1819 if (ret != 0) {
2013-01-27 1820 dev_err(map->dev, "Failed to schedule write: %d\n",
2013-01-27 1821 ret);
2013-01-27 1822
2013-01-27 1823 spin_lock_irqsave(&map->async_lock, flags);
2013-10-07 1824 list_move(&async->list, &map->async_free);
2013-01-27 1825 spin_unlock_irqrestore(&map->async_lock, flags);
2013-01-27 1826 }
2013-03-27 1827
2013-03-27 1828 return ret;
2013-01-27 1829 }
2013-01-27 1830
2015-03-09 1831 trace_regmap_hw_write_start(map, reg, val_len / map->format.val_bytes);
2011-07-24 1832
2011-07-20 1833 /* If we're doing a single register write we can probably just
2011-07-20 1834 * send the work_buf directly, otherwise try to do a gather
2011-07-20 1835 * write.
2011-07-20 1836 */
2013-01-27 1837 if (val == work_val)
2022-04-30 1838 ret = map->write(map->bus_context, map->work_buf,
2012-01-18 1839 map->format.reg_bytes +
2012-01-18 1840 map->format.pad_bytes +
2012-01-18 1841 val_len);
2011-07-20 @1842 else if (map->bus->gather_write)
^^^^^^^^
2012-04-04 1843 ret = map->bus->gather_write(map->bus_context, map->work_buf,
2012-01-18 1844 map->format.reg_bytes +
2012-01-18 1845 map->format.pad_bytes,
2011-05-11 1846 val, val_len);
Kandagatla 2019-06-12 1847 else
Kandagatla 2019-06-12 1848 ret = -ENOTSUPP;
2011-05-11 1849
2011-07-20 1850 /* If that didn't work fall back on linearising by hand. */
2011-05-11 1851 if (ret == -ENOTSUPP) {
2012-01-18 1852 len = map->format.reg_bytes + map->format.pad_bytes + val_len;
2012-01-18 1853 buf = kzalloc(len, GFP_KERNEL);
2011-05-11 1854 if (!buf)
2011-05-11 1855 return -ENOMEM;
2011-05-11 1856
2011-05-11 1857 memcpy(buf, map->work_buf, map->format.reg_bytes);
2012-01-18 1858 memcpy(buf + map->format.reg_bytes + map->format.pad_bytes,
2012-01-18 1859 val, val_len);
2022-04-30 1860 ret = map->write(map->bus_context, buf, len);
2011-05-11 1861
2011-05-11 1862 kfree(buf);
2016-08-18 1863 } else if (ret != 0 && !map->cache_bypass && map->format.parse_val) {
2016-09-22 1864 /* regcache_drop_region() takes lock that we already have,
2016-09-22 1865 * thus call map->cache_ops->drop() directly
2016-09-22 1866 */
2016-09-22 1867 if (map->cache_ops && map->cache_ops->drop)
2016-09-22 1868 map->cache_ops->drop(map, reg, reg + 1);
2011-05-11 1869 }
2011-05-11 1870
2015-03-09 1871 trace_regmap_hw_write_done(map, reg, val_len / map->format.val_bytes);
2011-07-24 1872
2011-05-11 1873 return ret;
2011-05-11 1874 }
lkp">https://01.org/lkp

2022-05-09 09:56:17

by Marek Vasut

[permalink] [raw]

Subject: Re: [broonie-ci:20220430_marex_regmap_add_bulk_read_write_callbacks_into_regmap_config 1/1] drivers/base/regmap/regmap.c:1842 _regmap_raw_write_impl() error: we previously assumed 'map->bus' could be null (see line 1770)

On 5/6/22 09:12, Dan Carpenter wrote:
> tree: https://git.kernel.org/pub/scm/linux/kernel/git/broonie/ci.git 20220430_marex_regmap_add_bulk_read_write_callbacks_into_regmap_config
> head: d77e745613680c54708470402e2b623dcd769681
> commit: d77e745613680c54708470402e2b623dcd769681 [1/1] regmap: Add bulk read/write callbacks into regmap_config
> config: m68k-randconfig-m031-20220505 (https://download.01.org/0day-ci/archive/20220506/[email protected]/config)
> compiler: m68k-linux-gcc (GCC) 11.3.0
>
> If you fix the issue, kindly add following tag as appropriate
> Reported-by: kernel test robot <[email protected]>
> Reported-by: Dan Carpenter <[email protected]>
>
> New smatch warnings:
> drivers/base/regmap/regmap.c:1842 _regmap_raw_write_impl() error: we previously assumed 'map->bus' could be null (see line 1770)
>
> vim +1842 drivers/base/regmap/regmap.c

[...]

> 0074f3f2b1e43d Colin Foster 2022-03-13 1754 reg += map->reg_base;
> 86fc59ef818beb Colin Foster 2022-03-13 1755 reg >>= map->format.reg_downshift;
> d939fb9a78b474 Marc Reilly 2012-03-16 1756 map->format.format_reg(map->work_buf, reg, map->reg_shift);
> f50e38c9966076 Tony Lindgren 2016-09-15 1757 regmap_set_work_buf_flag_mask(map, map->format.reg_bytes,
> f50e38c9966076 Tony Lindgren 2016-09-15 1758 map->write_flag_mask);
> 6f306441e97f8f Lars-Peter Clausen 2011-09-05 1759
> 651e013e3ce6c0 Mark Brown 2013-10-08 1760 /*
> 651e013e3ce6c0 Mark Brown 2013-10-08 1761 * Essentially all I/O mechanisms will be faster with a single
> 651e013e3ce6c0 Mark Brown 2013-10-08 1762 * buffer to write. Since register syncs often generate raw
> 651e013e3ce6c0 Mark Brown 2013-10-08 1763 * writes of single registers optimise that case.
> 651e013e3ce6c0 Mark Brown 2013-10-08 1764 */
> 651e013e3ce6c0 Mark Brown 2013-10-08 1765 if (val != work_val && val_len == map->format.val_bytes) {
> 651e013e3ce6c0 Mark Brown 2013-10-08 1766 memcpy(work_val, val, map->format.val_bytes);
> 651e013e3ce6c0 Mark Brown 2013-10-08 1767 val = work_val;
> 651e013e3ce6c0 Mark Brown 2013-10-08 1768 }
> 651e013e3ce6c0 Mark Brown 2013-10-08 1769
> d77e745613680c Marek Vasut 2022-04-30 @1770 if (map->async && map->bus && map->bus->async_write) {
> ^^^^^^^^
> Do we really need to check map->bus?

Here I think so, because bus can be NULL.

[...]

> c6b570d97c0e77 Philipp Zabel 2015-03-09 1831 trace_regmap_hw_write_start(map, reg, val_len / map->format.val_bytes);
> fb2736bbaee0e7 Mark Brown 2011-07-24 1832
> 2547e201b3693f Mark Brown 2011-07-20 1833 /* If we're doing a single register write we can probably just
> 2547e201b3693f Mark Brown 2011-07-20 1834 * send the work_buf directly, otherwise try to do a gather
> 2547e201b3693f Mark Brown 2011-07-20 1835 * write.
> 2547e201b3693f Mark Brown 2011-07-20 1836 */
> 0d509f2b112b21 Mark Brown 2013-01-27 1837 if (val == work_val)
> d77e745613680c Marek Vasut 2022-04-30 1838 ret = map->write(map->bus_context, map->work_buf,
> 82159ba8e6ef8c Mark Brown 2012-01-18 1839 map->format.reg_bytes +
> 82159ba8e6ef8c Mark Brown 2012-01-18 1840 map->format.pad_bytes +
> 82159ba8e6ef8c Mark Brown 2012-01-18 1841 val_len);
> 2547e201b3693f Mark Brown 2011-07-20 @1842 else if (map->bus->gather_write)
> ^^^^^^^^
> It's not checked here

This is a bug indeed.