2014-04-02 03:03:14

by Xiubo Li

[permalink] [raw]
Subject: [PATCH resend] regmap: mmio: Fix the bug of 'offset' value parsing.

'offset = *(u32 *)reg;'
This will be okey for 32/64-bits register device, but for 8/16-bits
register ones, the 'offset' value will overflow, for example:

The IMX2 Watchdog, whose registers and values are all 16-bits:

If the IO base virtual address is ctx->regs = 0x888c0000, and the now
doing the 0x00 register accessing:
Using 'offset = *(u32 *)reg' the offset value will possiblly be 0x77310000,
Using 'offset = *(u16 *)reg' the offset value will be 0x0000.

In the regmap_mmio_gather_write(), ctx->regs + 0x7731000 will be 0xffbd0000,
but actually it should be ctx->regs + 0x0000 = 0x888c0000.

Signed-off-by: Xiubo Li <[email protected]>
---
drivers/base/regmap/regmap-mmio.c | 29 ++++++++++++++++++++++++-----
1 file changed, 24 insertions(+), 5 deletions(-)

diff --git a/drivers/base/regmap/regmap-mmio.c b/drivers/base/regmap/regmap-mmio.c
index de45a1e..e1b42c5 100644
--- a/drivers/base/regmap/regmap-mmio.c
+++ b/drivers/base/regmap/regmap-mmio.c
@@ -67,12 +67,31 @@ static inline void regmap_mmio_count_check(size_t count)
BUG_ON(count % 2 != 0);
}

+static inline unsigned int
+regmap_mmio_get_offset(const void *reg, size_t reg_size)
+{
+ switch (reg_size) {
+ case 1:
+ return *(u8 *)reg;
+ case 2:
+ return *(u16 *)reg;
+ case 4:
+ return *(u32 *)reg;
+#ifdef CONFIG_64BIT
+ case 8:
+ return *(u64 *)reg;
+#endif
+ default:
+ BUG();
+ }
+}
+
static int regmap_mmio_gather_write(void *context,
const void *reg, size_t reg_size,
const void *val, size_t val_size)
{
struct regmap_mmio_context *ctx = context;
- u32 offset;
+ unsigned int offset;
int ret;

regmap_mmio_regsize_check(reg_size);
@@ -83,7 +102,7 @@ static int regmap_mmio_gather_write(void *context,
return ret;
}

- offset = *(u32 *)reg;
+ offset = regmap_mmio_get_offset(reg, reg_size);

while (val_size) {
switch (ctx->val_bytes) {
@@ -119,7 +138,7 @@ static int regmap_mmio_gather_write(void *context,
static int regmap_mmio_write(void *context, const void *data, size_t count)
{
struct regmap_mmio_context *ctx = context;
- u32 offset = ctx->reg_bytes + ctx->pad_bytes;
+ unsigned int offset = ctx->reg_bytes + ctx->pad_bytes;

regmap_mmio_count_check(count);

@@ -132,7 +151,7 @@ static int regmap_mmio_read(void *context,
void *val, size_t val_size)
{
struct regmap_mmio_context *ctx = context;
- u32 offset;
+ unsigned int offset;
int ret;

regmap_mmio_regsize_check(reg_size);
@@ -143,7 +162,7 @@ static int regmap_mmio_read(void *context,
return ret;
}

- offset = *(u32 *)reg;
+ offset = regmap_mmio_get_offset(reg, reg_size);

while (val_size) {
switch (ctx->val_bytes) {
--
1.8.4


2014-04-02 18:28:06

by Mark Brown

[permalink] [raw]
Subject: Re: [PATCH resend] regmap: mmio: Fix the bug of 'offset' value parsing.

On Wed, Apr 02, 2014 at 10:20:17AM +0800, Xiubo Li wrote:
> 'offset = *(u32 *)reg;'
> This will be okey for 32/64-bits register device, but for 8/16-bits
> register ones, the 'offset' value will overflow, for example:

Applied, thanks.


Attachments:
(No filename) (236.00 B)
signature.asc (836.00 B)
Digital signature
Download all attachments