From: ChiYuan Huang <[email protected]>
This potential risk could happen at regmap_raw_read() or
regmap_raw_write() when accessing the over-bound register address.
For testing, I try to reproduce it with a testing attribute file.
Below's the issue trace log.
[41.314358] pc : i2c_smbus_xfer+0x58/0x120
[41.314371] lr : i2c_smbus_read_i2c_block_data+0x74/0xc0
[41.399677] Call trace:
[41.402153] i2c_smbus_xfer+0x58/0x120
[41.405956] i2c_smbus_read_i2c_block_data+0x74/0xc0
[41.410991] mt6370_regmap_read+0x40/0x60 [mt6370]
[41.415855] _regmap_raw_read+0xe4/0x278
[41.419834] regmap_raw_read+0xec/0x240
[41.423721] rg_bound_show+0xb0/0x120 [mt6370]
[41.428226] dev_attr_show+0x3c/0x80
[41.431851] sysfs_kf_seq_show+0xc4/0x150
[41.435916] kernfs_seq_show+0x48/0x60
[41.439718] seq_read_iter+0x11c/0x450
[41.443519] kernfs_fop_read_iter+0x124/0x1c0
[41.447937] vfs_read+0x1a8/0x288
[41.451296] ksys_read+0x74/0x100
[41.454654] __arm64_sys_read+0x24/0x30
[41.458541] invoke_syscall+0x54/0x118
[41.462344] el0_svc_common.constprop.4+0x94/0x128
[41.467202] do_el0_svc+0x3c/0xd0
[41.470562] el0_svc+0x20/0x60
[41.473658] el0t_64_sync_handler+0x94/0xb8
[41.477899] el0t_64_sync+0x15c/0x160
[41.481614] Code: 54000388 f9401262 aa1303e0 52800041 (f9400042)
[41.487793] ---[ end trace 0000000000000000 ]---
Fixes: b2adf788e603 ("mfd: mt6370: Add MediaTek MT6370 support")
Reported-by: Dan Carpenter <[email protected]>
Signed-off-by: ChiYuan Huang <[email protected]>
---
Dear reviewers:
In v2, I refined the patch title and commit message.
If there's still something improper, please kindly correct me.
---
drivers/mfd/mt6370.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/drivers/mfd/mt6370.c b/drivers/mfd/mt6370.c
index cf19cce..acbf960 100644
--- a/drivers/mfd/mt6370.c
+++ b/drivers/mfd/mt6370.c
@@ -190,6 +190,9 @@ static int mt6370_regmap_read(void *context, const void *reg_buf,
bank_idx = u8_buf[0];
bank_addr = u8_buf[1];
+ if (bank_idx >= MT6370_MAX_I2C)
+ return -EINVAL;
+
ret = i2c_smbus_read_i2c_block_data(info->i2c[bank_idx], bank_addr,
val_size, val_buf);
if (ret < 0)
@@ -211,6 +214,9 @@ static int mt6370_regmap_write(void *context, const void *data, size_t count)
bank_idx = u8_buf[0];
bank_addr = u8_buf[1];
+ if (bank_idx >= MT6370_MAX_I2C)
+ return -EINVAL;
+
return i2c_smbus_write_i2c_block_data(info->i2c[bank_idx], bank_addr,
len, data + MT6370_MAX_ADDRLEN);
}
--
2.7.4
On Fri, Oct 21, 2022 at 5:17 AM cy_huang <[email protected]> wrote:
>
> From: ChiYuan Huang <[email protected]>
>
> This potential risk could happen at regmap_raw_read() or
> regmap_raw_write() when accessing the over-bound register address.
>
> For testing, I try to reproduce it with a testing attribute file.
> Below's the issue trace log.
It looks like you randomly cut the trace.
It's not what I meant and documentation suggests.
> [41.314358] pc : i2c_smbus_xfer+0x58/0x120
> [41.314371] lr : i2c_smbus_read_i2c_block_data+0x74/0xc0
> [41.399677] Call trace:
> [41.402153] i2c_smbus_xfer+0x58/0x120
> [41.405956] i2c_smbus_read_i2c_block_data+0x74/0xc0
> [41.410991] mt6370_regmap_read+0x40/0x60 [mt6370]
> [41.415855] _regmap_raw_read+0xe4/0x278
> [41.419834] regmap_raw_read+0xec/0x240
> [41.423721] rg_bound_show+0xb0/0x120 [mt6370]
> [41.428226] dev_attr_show+0x3c/0x80
> [41.431851] sysfs_kf_seq_show+0xc4/0x150
> [41.435916] kernfs_seq_show+0x48/0x60
> [41.439718] seq_read_iter+0x11c/0x450
> [41.443519] kernfs_fop_read_iter+0x124/0x1c0
> [41.447937] vfs_read+0x1a8/0x288
> [41.451296] ksys_read+0x74/0x100
> [41.454654] __arm64_sys_read+0x24/0x30
> [41.458541] invoke_syscall+0x54/0x118
> [41.462344] el0_svc_common.constprop.4+0x94/0x128
> [41.467202] do_el0_svc+0x3c/0xd0
> [41.470562] el0_svc+0x20/0x60
> [41.473658] el0t_64_sync_handler+0x94/0xb8
> [41.477899] el0t_64_sync+0x15c/0x160
> [41.481614] Code: 54000388 f9401262 aa1303e0 52800041 (f9400042)
> [41.487793] ---[ end trace 0000000000000000 ]---
...
> If there's still something improper, please kindly correct me.
https://www.kernel.org/doc/html/latest/process/submitting-patches.html#backtraces-in-commit-messages
--
With Best Regards,
Andy Shevchenko
On Fri, Oct 21, 2022 at 12:02 PM ChiYuan Huang <[email protected]> wrote:
> Andy Shevchenko <[email protected]> 於 2022年10月21日 週五 下午4:34寫道：
> > On Fri, Oct 21, 2022 at 5:17 AM cy_huang <[email protected]> wrote:
> > It looks like you randomly cut the trace.
> > It's not what I meant and documentation suggests.
> >
> I checked the submitting-patch.rst.
> To satisfy the requirement for 70-75 chars per line, I only keep the
> important log.
>
> May I ask what you mean for the 'trim'?
> Is it 'Still keep 70-75 per line and cut the characters that's over
> the limit to the next line"?
> > https://www.kernel.org/doc/html/latest/process/submitting-patches.html#backtraces-in-commit-messages
Have you had a chance to read this section of the document?
--
With Best Regards,
Andy Shevchenko
Andy Shevchenko <[email protected]> 於 2022年10月21日 週五 下午4:34寫道：
>
> On Fri, Oct 21, 2022 at 5:17 AM cy_huang <[email protected]> wrote:
> >
> > From: ChiYuan Huang <[email protected]>
> >
> > This potential risk could happen at regmap_raw_read() or
> > regmap_raw_write() when accessing the over-bound register address.
> >
> > For testing, I try to reproduce it with a testing attribute file.
> > Below's the issue trace log.
>
> It looks like you randomly cut the trace.
> It's not what I meant and documentation suggests.
>
I checked the submitting-patch.rst.
To satisfy the requirement for 70-75 chars per line, I only keep the
important log.
May I ask what you mean for the 'trim'?
Is it 'Still keep 70-75 per line and cut the characters that's over
the limit to the next line"?
> > [41.314358] pc : i2c_smbus_xfer+0x58/0x120
> > [41.314371] lr : i2c_smbus_read_i2c_block_data+0x74/0xc0
> > [41.399677] Call trace:
> > [41.402153] i2c_smbus_xfer+0x58/0x120
> > [41.405956] i2c_smbus_read_i2c_block_data+0x74/0xc0
> > [41.410991] mt6370_regmap_read+0x40/0x60 [mt6370]
> > [41.415855] _regmap_raw_read+0xe4/0x278
> > [41.419834] regmap_raw_read+0xec/0x240
> > [41.423721] rg_bound_show+0xb0/0x120 [mt6370]
> > [41.428226] dev_attr_show+0x3c/0x80
> > [41.431851] sysfs_kf_seq_show+0xc4/0x150
> > [41.435916] kernfs_seq_show+0x48/0x60
> > [41.439718] seq_read_iter+0x11c/0x450
> > [41.443519] kernfs_fop_read_iter+0x124/0x1c0
> > [41.447937] vfs_read+0x1a8/0x288
> > [41.451296] ksys_read+0x74/0x100
> > [41.454654] __arm64_sys_read+0x24/0x30
> > [41.458541] invoke_syscall+0x54/0x118
> > [41.462344] el0_svc_common.constprop.4+0x94/0x128
> > [41.467202] do_el0_svc+0x3c/0xd0
> > [41.470562] el0_svc+0x20/0x60
> > [41.473658] el0t_64_sync_handler+0x94/0xb8
> > [41.477899] el0t_64_sync+0x15c/0x160
> > [41.481614] Code: 54000388 f9401262 aa1303e0 52800041 (f9400042)
> > [41.487793] ---[ end trace 0000000000000000 ]---
>
> ...
>
> > If there's still something improper, please kindly correct me.
>
> https://www.kernel.org/doc/html/latest/process/submitting-patches.html#backtraces-in-commit-messages
>
> --
> With Best Regards,
> Andy Shevchenko
Andy Shevchenko <[email protected]> 於 2022年10月21日 週五 下午5:14寫道：
>
> On Fri, Oct 21, 2022 at 12:02 PM ChiYuan Huang <[email protected]> wrote:
> > Andy Shevchenko <[email protected]> 於 2022年10月21日 週五 下午4:34寫道：
> > > On Fri, Oct 21, 2022 at 5:17 AM cy_huang <[email protected]> wrote:
>
> > > It looks like you randomly cut the trace.
> > > It's not what I meant and documentation suggests.
> > >
> > I checked the submitting-patch.rst.
> > To satisfy the requirement for 70-75 chars per line, I only keep the
> > important log.
> >
> > May I ask what you mean for the 'trim'?
> > Is it 'Still keep 70-75 per line and cut the characters that's over
> > the limit to the next line"?
>
> > > https://www.kernel.org/doc/html/latest/process/submitting-patches.html#backtraces-in-commit-messages
>
> Have you had a chance to read this section of the document?
>
OK, get it.
Is the below text enough to express this problem?
Ex. Testing as below (mt6370 register range from 0 to 0x1ff)
rg_bound_show()
regmap_raw_read(regmap, 0x200, &val, sizeof(val));
pstate: 60000005 (nZCv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
pc : i2c_smbus_xfer+0x58/0x120
lr : i2c_smbus_read_i2c_block_data+0x74/0xc0
Call trace:
i2c_smbus_xfer+0x58/0x120
i2c_smbus_read_i2c_block_data+0x74/0xc0
mt6370_regmap_read+0x40/0x60
_regmap_raw_read+0xe4/0x278
regmap_raw_read+0xec/0x240
rg_bound_show+0xb0/0x120
> --
> With Best Regards,
> Andy Shevchenko
On Fri, Oct 21, 2022 at 12:58 PM ChiYuan Huang <[email protected]> wrote:
> Andy Shevchenko <[email protected]> 於 2022年10月21日 週五 下午5:14寫道：
> > On Fri, Oct 21, 2022 at 12:02 PM ChiYuan Huang <[email protected]> wrote:
> > > Andy Shevchenko <[email protected]> 於 2022年10月21日 週五 下午4:34寫道：
> > > > On Fri, Oct 21, 2022 at 5:17 AM cy_huang <[email protected]> wrote:
...
> > > > https://www.kernel.org/doc/html/latest/process/submitting-patches.html#backtraces-in-commit-messages
> >
> > Have you had a chance to read this section of the document?
> >
> OK, get it.
> Is the below text enough to express this problem?
It is you who decides, because I don't know what exact problem this
will represent.
> Ex. Testing as below (mt6370 register range from 0 to 0x1ff)
> rg_bound_show()
> regmap_raw_read(regmap, 0x200, &val, sizeof(val));
> pstate: 60000005 (nZCv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
For example, why is this line here?
> pc : i2c_smbus_xfer+0x58/0x120
> lr : i2c_smbus_read_i2c_block_data+0x74/0xc0
> Call trace:
> i2c_smbus_xfer+0x58/0x120
> i2c_smbus_read_i2c_block_data+0x74/0xc0
> mt6370_regmap_read+0x40/0x60
> _regmap_raw_read+0xe4/0x278
> regmap_raw_read+0xec/0x240
> rg_bound_show+0xb0/0x120
The rule of thumb is that it's okay to shrink to ~4-5 lines (in most cases).
--
With Best Regards,
Andy Shevchenko
Andy Shevchenko <[email protected]> 於 2022年10月21日 週五 下午6:35寫道：
>
> On Fri, Oct 21, 2022 at 12:58 PM ChiYuan Huang <[email protected]> wrote:
> > Andy Shevchenko <[email protected]> 於 2022年10月21日 週五 下午5:14寫道：
> > > On Fri, Oct 21, 2022 at 12:02 PM ChiYuan Huang <[email protected]> wrote:
> > > > Andy Shevchenko <[email protected]> 於 2022年10月21日 週五 下午4:34寫道：
> > > > > On Fri, Oct 21, 2022 at 5:17 AM cy_huang <[email protected]> wrote:
>
> ...
>
> > > > > https://www.kernel.org/doc/html/latest/process/submitting-patches.html#backtraces-in-commit-messages
> > >
> > > Have you had a chance to read this section of the document?
> > >
> > OK, get it.
> > Is the below text enough to express this problem?
>
> It is you who decides, because I don't know what exact problem this
> will represent.
>
> > Ex. Testing as below (mt6370 register range from 0 to 0x1ff)
> > rg_bound_show()
> > regmap_raw_read(regmap, 0x200, &val, sizeof(val));
>
> > pstate: 60000005 (nZCv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
>
> For example, why is this line here?
>
> > pc : i2c_smbus_xfer+0x58/0x120
> > lr : i2c_smbus_read_i2c_block_data+0x74/0xc0
> > Call trace:
> > i2c_smbus_xfer+0x58/0x120
> > i2c_smbus_read_i2c_block_data+0x74/0xc0
> > mt6370_regmap_read+0x40/0x60
> > _regmap_raw_read+0xe4/0x278
> > regmap_raw_read+0xec/0x240
> > rg_bound_show+0xb0/0x120
>
> The rule of thumb is that it's okay to shrink to ~4-5 lines (in most cases).
>
At first, I start this thread due to the original thread seems stopped
Since it already restarted, please forget this one.
Just follow the restaed thread for the discussion.
https://lore.kernel.org/all/Y1aCiReTZDbPp%2FrS@kadam/
> --
> With Best Regards,
> Andy Shevchenko