[RFC PATCH 0/1] Verify if register is writeable before a write operation

[RFC PATCH 0/1] Verify if register is writeable before a write operation

[Han Nandor]

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

[RFC PATCH 1/1] regmap: verify if register is writeable before writing operations

[Han Nandor]

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 <nandor.han@vaisala.com>
---
 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

Re: [RFC PATCH 0/1] Verify if register is writeable before a write operation

[Mark Brown]

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

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. 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [RFC PATCH 0/1] Verify if register is writeable before a write operation

[Nandor Han]

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.