Description
-----------
This is an RFC because I don't know if this is a bug or a normal use
case. It seems that the function `_regmap_raw_write_impl` from the regmap
framework verifies that a register is writable only using
the callback function, ignoring the other two (max allowed register,
register ranges)
Note: As a left right look I did check also `_regmap_raw_read` function,
and it seems that is missing this checks completely.
Is this a problem as well?
Device/Subsystems Impacted
-------------------------
This will impact drivers that end up using raw writes (firmware
download, ...)
Testing
-------
Test configuration:
- Kernel Version: 4.14.60 (just for clarification, the patch is rebased on master)
- The testing was done using a driver that has NVMEM support and is
using `regmap_bulk_write` method to write data to registers.
- The valid register range is 0x00->0xFF.
1. Configure a nvcell (4 bytes long) in DT which is outside the valid register range.
2. Write data to NVMEM using the nvcell
3. Verify the result:
3.1 Without the patch the data write in registers is successful with
no error displayed.
3.2 With the patch the data write is denied and an error is displayed.
Nandor Han (1):
regmap: verify if register is writeable before writing operations
drivers/base/regmap/regmap.c | 9 ++++-----
1 file changed, 4 insertions(+), 5 deletions(-)
--
2.17.2
On Tue, Apr 02, 2019 at 08:01:21AM +0000, Han Nandor wrote:
> Description
> -----------
> This is an RFC because I don't know if this is a bug or a normal use
> case. It seems that the function `_regmap_raw_write_impl` from the regmap
> framework verifies that a register is writable only using
> the callback function, ignoring the other two (max allowed register,
> register ranges)
Please don't send cover letters for single patches, if there is anything
that needs saying put it in the changelog of the patch or after the ---
if it's administrative stuff. This reduces mail volume and ensures that
any important information is recorded in the changelog rather than being
lost.
regmap provides a couple of ways to validate the register range used.
a) maxim allowed register, b) writable/readable register tables,
c) callback function that can be provided by the driver to validate
a register. regmap framework should verify if registers
are writeable before every write operation. However this doesn't
seems to happen in every situation.
The method `_regmap_raw_write_impl` is only using the `writeable_reg`
callback to verify if register is writeable, ignoring the other two.
This can lead to undefined behaviour since this allows to write to
registers that could be declared un-writeable by using any other
option.
Change `_regmap_raw_write_impl` to use the `regmap_writeable` method
to verify if registers are writable before the write operation.
Signed-off-by: Nandor Han <[email protected]>
---
drivers/base/regmap/regmap.c | 9 ++++-----
1 file changed, 4 insertions(+), 5 deletions(-)
diff --git a/drivers/base/regmap/regmap.c b/drivers/base/regmap/regmap.c
index 4f822e087def..42d8404bc8cc 100644
--- a/drivers/base/regmap/regmap.c
+++ b/drivers/base/regmap/regmap.c
@@ -1493,11 +1493,10 @@ static int _regmap_raw_write_impl(struct regmap *map, unsigned int reg,
WARN_ON(!map->bus);
/* Check for unwritable registers before we start */
- if (map->writeable_reg)
- for (i = 0; i < val_len / map->format.val_bytes; i++)
- if (!map->writeable_reg(map->dev,
- reg + regmap_get_offset(map, i)))
- return -EINVAL;
+ for (i = 0; i < val_len / map->format.val_bytes; i++)
+ if (!regmap_writeable(map,
+ reg + regmap_get_offset(map, i)))
+ return -EINVAL;
if (!map->cache_bypass && map->format.parse_val) {
unsigned int ival;
--
2.17.2
On 4/2/19 12:06 PM, Mark Brown wrote:
> On Tue, Apr 02, 2019 at 08:01:21AM +0000, Han Nandor wrote:
>> Description
>> -----------
>> This is an RFC because I don't know if this is a bug or a normal use
>> case. It seems that the function `_regmap_raw_write_impl` from the regmap
>> framework verifies that a register is writable only using
>> the callback function, ignoring the other two (max allowed register,
>> register ranges)
>
> Please don't send cover letters for single patches, if there is anything
> that needs saying put it in the changelog of the patch or after the ---
> if it's administrative stuff. This reduces mail volume and ensures that
> any important information is recorded in the changelog rather than being
> lost.
>
Copy that. If is OK, we can still discuss about this using this patch
and I can send a single patch once the things are clear.