* [PATCH v2] i2c: cpm: Fix i2c_ram structure
@ 2020-09-23 14:08 ` nico.vince
0 siblings, 0 replies; 8+ messages in thread
From: nico.vince @ 2020-09-23 14:08 UTC (permalink / raw)
To: jochen; +Cc: Nicolas VINCENT, linuxppc-dev, linux-i2c
From: Nicolas VINCENT <nicolas.vincent@vossloh.com>
the i2c_ram structure is missing the sdmatmp field mentionned in
datasheet for MPC8272 at paragraph 36.5. With this field missing, the
hardware would write past the allocated memory done through
cpm_muram_alloc for the i2c_ram structure and land in memory allocated
for the buffers descriptors corrupting the cbd_bufaddr field. Since this
field is only set during setup(), the first i2c transaction would work
and the following would send data read from an arbitrary memory
location.
Signed-off-by: Nicolas VINCENT <nicolas.vincent@vossloh.com>
---
drivers/i2c/busses/i2c-cpm.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/i2c/busses/i2c-cpm.c b/drivers/i2c/busses/i2c-cpm.c
index 1213e1932ccb..24d584a1c9a7 100644
--- a/drivers/i2c/busses/i2c-cpm.c
+++ b/drivers/i2c/busses/i2c-cpm.c
@@ -65,6 +65,9 @@ struct i2c_ram {
char res1[4]; /* Reserved */
ushort rpbase; /* Relocation pointer */
char res2[2]; /* Reserved */
+ /* The following elements are only for CPM2 */
+ char res3[4]; /* Reserved */
+ uint sdmatmp; /* Internal */
};
#define I2COM_START 0x80
--
2.17.1
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [PATCH v2] i2c: cpm: Fix i2c_ram structure
2020-09-23 14:08 ` nico.vince
(?)
@ 2020-09-23 14:12 ` Jochen Friedrich
-1 siblings, 0 replies; 8+ messages in thread
From: Jochen Friedrich @ 2020-09-23 14:12 UTC (permalink / raw)
To: nicolas.vincent; +Cc: linuxppc-dev, linux-i2c
Acked-by: Jochen Friedrich <jochen@scram.de>
Am 23.09.2020 um 16:08 schrieb nico.vince@gmail.com:
> From: Nicolas VINCENT <nicolas.vincent@vossloh.com>
>
> the i2c_ram structure is missing the sdmatmp field mentionned in
> datasheet for MPC8272 at paragraph 36.5. With this field missing, the
> hardware would write past the allocated memory done through
> cpm_muram_alloc for the i2c_ram structure and land in memory allocated
> for the buffers descriptors corrupting the cbd_bufaddr field. Since this
> field is only set during setup(), the first i2c transaction would work
> and the following would send data read from an arbitrary memory
> location.
>
> Signed-off-by: Nicolas VINCENT <nicolas.vincent@vossloh.com>
> ---
> drivers/i2c/busses/i2c-cpm.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/drivers/i2c/busses/i2c-cpm.c b/drivers/i2c/busses/i2c-cpm.c
> index 1213e1932ccb..24d584a1c9a7 100644
> --- a/drivers/i2c/busses/i2c-cpm.c
> +++ b/drivers/i2c/busses/i2c-cpm.c
> @@ -65,6 +65,9 @@ struct i2c_ram {
> char res1[4]; /* Reserved */
> ushort rpbase; /* Relocation pointer */
> char res2[2]; /* Reserved */
> + /* The following elements are only for CPM2 */
> + char res3[4]; /* Reserved */
> + uint sdmatmp; /* Internal */
> };
>
> #define I2COM_START 0x80
>
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH v2] i2c: cpm: Fix i2c_ram structure
2020-09-23 14:08 ` nico.vince
@ 2020-09-23 16:08 ` Wolfram Sang
-1 siblings, 0 replies; 8+ messages in thread
From: Wolfram Sang @ 2020-09-23 16:08 UTC (permalink / raw)
To: nicolas.vincent; +Cc: jochen, linuxppc-dev, linux-i2c
[-- Attachment #1: Type: text/plain, Size: 1406 bytes --]
On Wed, Sep 23, 2020 at 04:08:40PM +0200, nico.vince@gmail.com wrote:
> From: Nicolas VINCENT <nicolas.vincent@vossloh.com>
>
> the i2c_ram structure is missing the sdmatmp field mentionned in
> datasheet for MPC8272 at paragraph 36.5. With this field missing, the
> hardware would write past the allocated memory done through
> cpm_muram_alloc for the i2c_ram structure and land in memory allocated
> for the buffers descriptors corrupting the cbd_bufaddr field. Since this
> field is only set during setup(), the first i2c transaction would work
> and the following would send data read from an arbitrary memory
> location.
>
> Signed-off-by: Nicolas VINCENT <nicolas.vincent@vossloh.com>
Thanks!
Is someone able to identify a Fixes: tag I could add?
> ---
> drivers/i2c/busses/i2c-cpm.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/drivers/i2c/busses/i2c-cpm.c b/drivers/i2c/busses/i2c-cpm.c
> index 1213e1932ccb..24d584a1c9a7 100644
> --- a/drivers/i2c/busses/i2c-cpm.c
> +++ b/drivers/i2c/busses/i2c-cpm.c
> @@ -65,6 +65,9 @@ struct i2c_ram {
> char res1[4]; /* Reserved */
> ushort rpbase; /* Relocation pointer */
> char res2[2]; /* Reserved */
> + /* The following elements are only for CPM2 */
> + char res3[4]; /* Reserved */
> + uint sdmatmp; /* Internal */
> };
>
> #define I2COM_START 0x80
> --
> 2.17.1
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH v2] i2c: cpm: Fix i2c_ram structure
@ 2020-09-23 16:08 ` Wolfram Sang
0 siblings, 0 replies; 8+ messages in thread
From: Wolfram Sang @ 2020-09-23 16:08 UTC (permalink / raw)
To: nicolas.vincent; +Cc: linuxppc-dev, linux-i2c
[-- Attachment #1: Type: text/plain, Size: 1406 bytes --]
On Wed, Sep 23, 2020 at 04:08:40PM +0200, nico.vince@gmail.com wrote:
> From: Nicolas VINCENT <nicolas.vincent@vossloh.com>
>
> the i2c_ram structure is missing the sdmatmp field mentionned in
> datasheet for MPC8272 at paragraph 36.5. With this field missing, the
> hardware would write past the allocated memory done through
> cpm_muram_alloc for the i2c_ram structure and land in memory allocated
> for the buffers descriptors corrupting the cbd_bufaddr field. Since this
> field is only set during setup(), the first i2c transaction would work
> and the following would send data read from an arbitrary memory
> location.
>
> Signed-off-by: Nicolas VINCENT <nicolas.vincent@vossloh.com>
Thanks!
Is someone able to identify a Fixes: tag I could add?
> ---
> drivers/i2c/busses/i2c-cpm.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/drivers/i2c/busses/i2c-cpm.c b/drivers/i2c/busses/i2c-cpm.c
> index 1213e1932ccb..24d584a1c9a7 100644
> --- a/drivers/i2c/busses/i2c-cpm.c
> +++ b/drivers/i2c/busses/i2c-cpm.c
> @@ -65,6 +65,9 @@ struct i2c_ram {
> char res1[4]; /* Reserved */
> ushort rpbase; /* Relocation pointer */
> char res2[2]; /* Reserved */
> + /* The following elements are only for CPM2 */
> + char res3[4]; /* Reserved */
> + uint sdmatmp; /* Internal */
> };
>
> #define I2COM_START 0x80
> --
> 2.17.1
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH v2] i2c: cpm: Fix i2c_ram structure
2020-09-23 16:08 ` Wolfram Sang
(?)
@ 2020-09-23 18:27 ` Christophe Leroy
-1 siblings, 0 replies; 8+ messages in thread
From: Christophe Leroy @ 2020-09-23 18:27 UTC (permalink / raw)
To: Wolfram Sang, nicolas.vincent; +Cc: linuxppc-dev, linux-i2c
Le 23/09/2020 à 18:08, Wolfram Sang a écrit :
> On Wed, Sep 23, 2020 at 04:08:40PM +0200, nico.vince@gmail.com wrote:
>> From: Nicolas VINCENT <nicolas.vincent@vossloh.com>
>>
>> the i2c_ram structure is missing the sdmatmp field mentionned in
>> datasheet for MPC8272 at paragraph 36.5. With this field missing, the
>> hardware would write past the allocated memory done through
>> cpm_muram_alloc for the i2c_ram structure and land in memory allocated
>> for the buffers descriptors corrupting the cbd_bufaddr field. Since this
>> field is only set during setup(), the first i2c transaction would work
>> and the following would send data read from an arbitrary memory
>> location.
>>
>> Signed-off-by: Nicolas VINCENT <nicolas.vincent@vossloh.com>
>
> Thanks!
>
> Is someone able to identify a Fixes: tag I could add?
I'd suggest
Fixes: 61045dbe9d8d ("i2c: Add support for I2C bus on Freescale
CPM1/CPM2 controllers")
Christophe
>
>> ---
>> drivers/i2c/busses/i2c-cpm.c | 3 +++
>> 1 file changed, 3 insertions(+)
>>
>> diff --git a/drivers/i2c/busses/i2c-cpm.c b/drivers/i2c/busses/i2c-cpm.c
>> index 1213e1932ccb..24d584a1c9a7 100644
>> --- a/drivers/i2c/busses/i2c-cpm.c
>> +++ b/drivers/i2c/busses/i2c-cpm.c
>> @@ -65,6 +65,9 @@ struct i2c_ram {
>> char res1[4]; /* Reserved */
>> ushort rpbase; /* Relocation pointer */
>> char res2[2]; /* Reserved */
>> + /* The following elements are only for CPM2 */
>> + char res3[4]; /* Reserved */
>> + uint sdmatmp; /* Internal */
>> };
>>
>> #define I2COM_START 0x80
>> --
>> 2.17.1
>>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2] i2c: cpm: Fix i2c_ram structure
2020-09-23 14:08 ` nico.vince
@ 2020-09-27 13:15 ` Wolfram Sang
-1 siblings, 0 replies; 8+ messages in thread
From: Wolfram Sang @ 2020-09-27 13:15 UTC (permalink / raw)
To: nicolas.vincent; +Cc: jochen, linuxppc-dev, linux-i2c
[-- Attachment #1: Type: text/plain, Size: 771 bytes --]
On Wed, Sep 23, 2020 at 04:08:40PM +0200, nico.vince@gmail.com wrote:
> From: Nicolas VINCENT <nicolas.vincent@vossloh.com>
>
> the i2c_ram structure is missing the sdmatmp field mentionned in
> datasheet for MPC8272 at paragraph 36.5. With this field missing, the
> hardware would write past the allocated memory done through
> cpm_muram_alloc for the i2c_ram structure and land in memory allocated
> for the buffers descriptors corrupting the cbd_bufaddr field. Since this
> field is only set during setup(), the first i2c transaction would work
> and the following would send data read from an arbitrary memory
> location.
>
> Signed-off-by: Nicolas VINCENT <nicolas.vincent@vossloh.com>
Fixes tag aded and applied to for-current, thanks everyone!
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2] i2c: cpm: Fix i2c_ram structure
@ 2020-09-27 13:15 ` Wolfram Sang
0 siblings, 0 replies; 8+ messages in thread
From: Wolfram Sang @ 2020-09-27 13:15 UTC (permalink / raw)
To: nicolas.vincent; +Cc: linuxppc-dev, linux-i2c
[-- Attachment #1: Type: text/plain, Size: 771 bytes --]
On Wed, Sep 23, 2020 at 04:08:40PM +0200, nico.vince@gmail.com wrote:
> From: Nicolas VINCENT <nicolas.vincent@vossloh.com>
>
> the i2c_ram structure is missing the sdmatmp field mentionned in
> datasheet for MPC8272 at paragraph 36.5. With this field missing, the
> hardware would write past the allocated memory done through
> cpm_muram_alloc for the i2c_ram structure and land in memory allocated
> for the buffers descriptors corrupting the cbd_bufaddr field. Since this
> field is only set during setup(), the first i2c transaction would work
> and the following would send data read from an arbitrary memory
> location.
>
> Signed-off-by: Nicolas VINCENT <nicolas.vincent@vossloh.com>
Fixes tag aded and applied to for-current, thanks everyone!
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 8+ messages in thread