Linux-RTC Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH] firmware: imx: Align imx SC msg structs to 4
@ 2020-02-11 21:24 Leonard Crestez
  2020-02-17  6:21 ` Shawn Guo
                   ` (2 more replies)
  0 siblings, 3 replies; 9+ messages in thread
From: Leonard Crestez @ 2020-02-11 21:24 UTC (permalink / raw)
  To: Shawn Guo, Dong Aisheng
  Cc: Fabio Estevam, Michael Turquette, Stephen Boyd, Stefan Agner,
	Linus Walleij, Alessandro Zummo, Alexandre Belloni, Anson Huang,
	Abel Vesa, Franck LENORMAND, kernel, linux-imx, linux-arm-kernel,
	linux-clk, linux-gpio, linux-rtc

The imx SC api strongly assumes that messages are composed out of
4-bytes words but some of our message structs have sizeof "6" and "7".

This produces many oopses with CONFIG_KASAN=y:

	BUG: KASAN: stack-out-of-bounds in imx_mu_send_data+0x108/0x1f0

It shouldn't cause an issues in normal use because these structs are
always allocated on the stack.

Cc: stable@vger.kernel.org
Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com>
Reported-by: Iuliana Prodan <iuliana.prodan@nxp.com>
---
 drivers/clk/imx/clk-scu.c               | 8 ++++----
 drivers/firmware/imx/misc.c             | 8 ++++----
 drivers/firmware/imx/scu-pd.c           | 2 +-
 drivers/pinctrl/freescale/pinctrl-scu.c | 4 ++--
 drivers/rtc/rtc-imx-sc.c                | 2 +-
 drivers/soc/imx/soc-imx-scu.c           | 2 +-
 6 files changed, 13 insertions(+), 13 deletions(-)

diff --git a/drivers/clk/imx/clk-scu.c b/drivers/clk/imx/clk-scu.c
index fbef740704d0..b8b2072742a5 100644
--- a/drivers/clk/imx/clk-scu.c
+++ b/drivers/clk/imx/clk-scu.c
@@ -41,16 +41,16 @@ struct clk_scu {
 struct imx_sc_msg_req_set_clock_rate {
 	struct imx_sc_rpc_msg hdr;
 	__le32 rate;
 	__le16 resource;
 	u8 clk;
-} __packed;
+} __packed __aligned(4);
 
 struct req_get_clock_rate {
 	__le16 resource;
 	u8 clk;
-} __packed;
+} __packed __aligned(4);
 
 struct resp_get_clock_rate {
 	__le32 rate;
 };
 
@@ -82,11 +82,11 @@ struct imx_sc_msg_get_clock_parent {
 	struct imx_sc_rpc_msg hdr;
 	union {
 		struct req_get_clock_parent {
 			__le16 resource;
 			u8 clk;
-		} __packed req;
+		} __packed __aligned(4) req;
 		struct resp_get_clock_parent {
 			u8 parent;
 		} resp;
 	} data;
 };
@@ -119,11 +119,11 @@ struct imx_sc_msg_req_clock_enable {
 	struct imx_sc_rpc_msg hdr;
 	__le16 resource;
 	u8 clk;
 	u8 enable;
 	u8 autog;
-} __packed;
+} __packed __aligned(4);
 
 static inline struct clk_scu *to_clk_scu(struct clk_hw *hw)
 {
 	return container_of(hw, struct clk_scu, hw);
 }
diff --git a/drivers/firmware/imx/misc.c b/drivers/firmware/imx/misc.c
index 4b56a587dacd..d073cb3ce699 100644
--- a/drivers/firmware/imx/misc.c
+++ b/drivers/firmware/imx/misc.c
@@ -14,30 +14,30 @@
 struct imx_sc_msg_req_misc_set_ctrl {
 	struct imx_sc_rpc_msg hdr;
 	u32 ctrl;
 	u32 val;
 	u16 resource;
-} __packed;
+} __packed __aligned(4);
 
 struct imx_sc_msg_req_cpu_start {
 	struct imx_sc_rpc_msg hdr;
 	u32 address_hi;
 	u32 address_lo;
 	u16 resource;
 	u8 enable;
-} __packed;
+} __packed __aligned(4);
 
 struct imx_sc_msg_req_misc_get_ctrl {
 	struct imx_sc_rpc_msg hdr;
 	u32 ctrl;
 	u16 resource;
-} __packed;
+} __packed __aligned(4);
 
 struct imx_sc_msg_resp_misc_get_ctrl {
 	struct imx_sc_rpc_msg hdr;
 	u32 val;
-} __packed;
+} __packed __aligned(4);
 
 /*
  * This function sets a miscellaneous control value.
  *
  * @param[in]     ipc         IPC handle
diff --git a/drivers/firmware/imx/scu-pd.c b/drivers/firmware/imx/scu-pd.c
index b556612207e5..af3ae0087de4 100644
--- a/drivers/firmware/imx/scu-pd.c
+++ b/drivers/firmware/imx/scu-pd.c
@@ -59,11 +59,11 @@
 /* SCU Power Mode Protocol definition */
 struct imx_sc_msg_req_set_resource_power_mode {
 	struct imx_sc_rpc_msg hdr;
 	u16 resource;
 	u8 mode;
-} __packed;
+} __packed __aligned(4);
 
 #define IMX_SCU_PD_NAME_SIZE 20
 struct imx_sc_pm_domain {
 	struct generic_pm_domain pd;
 	char name[IMX_SCU_PD_NAME_SIZE];
diff --git a/drivers/pinctrl/freescale/pinctrl-scu.c b/drivers/pinctrl/freescale/pinctrl-scu.c
index 73bf1d9f9cc6..23cf04bdfc55 100644
--- a/drivers/pinctrl/freescale/pinctrl-scu.c
+++ b/drivers/pinctrl/freescale/pinctrl-scu.c
@@ -21,16 +21,16 @@ enum pad_func_e {
 
 struct imx_sc_msg_req_pad_set {
 	struct imx_sc_rpc_msg hdr;
 	u32 val;
 	u16 pad;
-} __packed;
+} __packed __aligned(4);
 
 struct imx_sc_msg_req_pad_get {
 	struct imx_sc_rpc_msg hdr;
 	u16 pad;
-} __packed;
+} __packed __aligned(4);
 
 struct imx_sc_msg_resp_pad_get {
 	struct imx_sc_rpc_msg hdr;
 	u32 val;
 } __packed;
diff --git a/drivers/rtc/rtc-imx-sc.c b/drivers/rtc/rtc-imx-sc.c
index cf2c12107f2b..a5f59e6f862e 100644
--- a/drivers/rtc/rtc-imx-sc.c
+++ b/drivers/rtc/rtc-imx-sc.c
@@ -35,11 +35,11 @@ struct imx_sc_msg_timer_rtc_set_alarm {
 	u8 mon;
 	u8 day;
 	u8 hour;
 	u8 min;
 	u8 sec;
-} __packed;
+} __packed __aligned(4);
 
 static int imx_sc_rtc_read_time(struct device *dev, struct rtc_time *tm)
 {
 	struct imx_sc_msg_timer_get_rtc_time msg;
 	struct imx_sc_rpc_msg *hdr = &msg.hdr;
diff --git a/drivers/soc/imx/soc-imx-scu.c b/drivers/soc/imx/soc-imx-scu.c
index fb70b8a3f7c5..20d37eaeb5f2 100644
--- a/drivers/soc/imx/soc-imx-scu.c
+++ b/drivers/soc/imx/soc-imx-scu.c
@@ -23,11 +23,11 @@ struct imx_sc_msg_misc_get_soc_id {
 		} __packed req;
 		struct {
 			u32 id;
 		} resp;
 	} data;
-} __packed;
+} __packed __aligned(4);
 
 struct imx_sc_msg_misc_get_soc_uid {
 	struct imx_sc_rpc_msg hdr;
 	u32 uid_low;
 	u32 uid_high;
-- 
2.17.1


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] firmware: imx: Align imx SC msg structs to 4
  2020-02-11 21:24 [PATCH] firmware: imx: Align imx SC msg structs to 4 Leonard Crestez
@ 2020-02-17  6:21 ` Shawn Guo
  2020-02-17 20:37   ` Leonard Crestez
  2020-02-18  9:52 ` Alexandre Belloni
  2020-02-19 23:57 ` Stephen Boyd
  2 siblings, 1 reply; 9+ messages in thread
From: Shawn Guo @ 2020-02-17  6:21 UTC (permalink / raw)
  To: Leonard Crestez
  Cc: Dong Aisheng, Fabio Estevam, Michael Turquette, Stephen Boyd,
	Stefan Agner, Linus Walleij, Alessandro Zummo, Alexandre Belloni,
	Anson Huang, Abel Vesa, Franck LENORMAND, kernel, linux-imx,
	linux-arm-kernel, open list:COMMON CLK FRAMEWORK,
	open list:PIN CONTROLLER - FREESCALE,
	open list:REAL TIME CLOCK (RTC) SUBSYSTEM

On Tue, Feb 11, 2020 at 11:24:33PM +0200, Leonard Crestez wrote:
> The imx SC api strongly assumes that messages are composed out of
> 4-bytes words but some of our message structs have sizeof "6" and "7".
> 
> This produces many oopses with CONFIG_KASAN=y:
> 
> 	BUG: KASAN: stack-out-of-bounds in imx_mu_send_data+0x108/0x1f0
> 
> It shouldn't cause an issues in normal use because these structs are
> always allocated on the stack.
> 
> Cc: stable@vger.kernel.org

Should we have a fixes tag and send it for -rc?

Shawn

> Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com>
> Reported-by: Iuliana Prodan <iuliana.prodan@nxp.com>
> ---
>  drivers/clk/imx/clk-scu.c               | 8 ++++----
>  drivers/firmware/imx/misc.c             | 8 ++++----
>  drivers/firmware/imx/scu-pd.c           | 2 +-
>  drivers/pinctrl/freescale/pinctrl-scu.c | 4 ++--
>  drivers/rtc/rtc-imx-sc.c                | 2 +-
>  drivers/soc/imx/soc-imx-scu.c           | 2 +-
>  6 files changed, 13 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/clk/imx/clk-scu.c b/drivers/clk/imx/clk-scu.c
> index fbef740704d0..b8b2072742a5 100644
> --- a/drivers/clk/imx/clk-scu.c
> +++ b/drivers/clk/imx/clk-scu.c
> @@ -41,16 +41,16 @@ struct clk_scu {
>  struct imx_sc_msg_req_set_clock_rate {
>  	struct imx_sc_rpc_msg hdr;
>  	__le32 rate;
>  	__le16 resource;
>  	u8 clk;
> -} __packed;
> +} __packed __aligned(4);
>  
>  struct req_get_clock_rate {
>  	__le16 resource;
>  	u8 clk;
> -} __packed;
> +} __packed __aligned(4);
>  
>  struct resp_get_clock_rate {
>  	__le32 rate;
>  };
>  
> @@ -82,11 +82,11 @@ struct imx_sc_msg_get_clock_parent {
>  	struct imx_sc_rpc_msg hdr;
>  	union {
>  		struct req_get_clock_parent {
>  			__le16 resource;
>  			u8 clk;
> -		} __packed req;
> +		} __packed __aligned(4) req;
>  		struct resp_get_clock_parent {
>  			u8 parent;
>  		} resp;
>  	} data;
>  };
> @@ -119,11 +119,11 @@ struct imx_sc_msg_req_clock_enable {
>  	struct imx_sc_rpc_msg hdr;
>  	__le16 resource;
>  	u8 clk;
>  	u8 enable;
>  	u8 autog;
> -} __packed;
> +} __packed __aligned(4);
>  
>  static inline struct clk_scu *to_clk_scu(struct clk_hw *hw)
>  {
>  	return container_of(hw, struct clk_scu, hw);
>  }
> diff --git a/drivers/firmware/imx/misc.c b/drivers/firmware/imx/misc.c
> index 4b56a587dacd..d073cb3ce699 100644
> --- a/drivers/firmware/imx/misc.c
> +++ b/drivers/firmware/imx/misc.c
> @@ -14,30 +14,30 @@
>  struct imx_sc_msg_req_misc_set_ctrl {
>  	struct imx_sc_rpc_msg hdr;
>  	u32 ctrl;
>  	u32 val;
>  	u16 resource;
> -} __packed;
> +} __packed __aligned(4);
>  
>  struct imx_sc_msg_req_cpu_start {
>  	struct imx_sc_rpc_msg hdr;
>  	u32 address_hi;
>  	u32 address_lo;
>  	u16 resource;
>  	u8 enable;
> -} __packed;
> +} __packed __aligned(4);
>  
>  struct imx_sc_msg_req_misc_get_ctrl {
>  	struct imx_sc_rpc_msg hdr;
>  	u32 ctrl;
>  	u16 resource;
> -} __packed;
> +} __packed __aligned(4);
>  
>  struct imx_sc_msg_resp_misc_get_ctrl {
>  	struct imx_sc_rpc_msg hdr;
>  	u32 val;
> -} __packed;
> +} __packed __aligned(4);
>  
>  /*
>   * This function sets a miscellaneous control value.
>   *
>   * @param[in]     ipc         IPC handle
> diff --git a/drivers/firmware/imx/scu-pd.c b/drivers/firmware/imx/scu-pd.c
> index b556612207e5..af3ae0087de4 100644
> --- a/drivers/firmware/imx/scu-pd.c
> +++ b/drivers/firmware/imx/scu-pd.c
> @@ -59,11 +59,11 @@
>  /* SCU Power Mode Protocol definition */
>  struct imx_sc_msg_req_set_resource_power_mode {
>  	struct imx_sc_rpc_msg hdr;
>  	u16 resource;
>  	u8 mode;
> -} __packed;
> +} __packed __aligned(4);
>  
>  #define IMX_SCU_PD_NAME_SIZE 20
>  struct imx_sc_pm_domain {
>  	struct generic_pm_domain pd;
>  	char name[IMX_SCU_PD_NAME_SIZE];
> diff --git a/drivers/pinctrl/freescale/pinctrl-scu.c b/drivers/pinctrl/freescale/pinctrl-scu.c
> index 73bf1d9f9cc6..23cf04bdfc55 100644
> --- a/drivers/pinctrl/freescale/pinctrl-scu.c
> +++ b/drivers/pinctrl/freescale/pinctrl-scu.c
> @@ -21,16 +21,16 @@ enum pad_func_e {
>  
>  struct imx_sc_msg_req_pad_set {
>  	struct imx_sc_rpc_msg hdr;
>  	u32 val;
>  	u16 pad;
> -} __packed;
> +} __packed __aligned(4);
>  
>  struct imx_sc_msg_req_pad_get {
>  	struct imx_sc_rpc_msg hdr;
>  	u16 pad;
> -} __packed;
> +} __packed __aligned(4);
>  
>  struct imx_sc_msg_resp_pad_get {
>  	struct imx_sc_rpc_msg hdr;
>  	u32 val;
>  } __packed;
> diff --git a/drivers/rtc/rtc-imx-sc.c b/drivers/rtc/rtc-imx-sc.c
> index cf2c12107f2b..a5f59e6f862e 100644
> --- a/drivers/rtc/rtc-imx-sc.c
> +++ b/drivers/rtc/rtc-imx-sc.c
> @@ -35,11 +35,11 @@ struct imx_sc_msg_timer_rtc_set_alarm {
>  	u8 mon;
>  	u8 day;
>  	u8 hour;
>  	u8 min;
>  	u8 sec;
> -} __packed;
> +} __packed __aligned(4);
>  
>  static int imx_sc_rtc_read_time(struct device *dev, struct rtc_time *tm)
>  {
>  	struct imx_sc_msg_timer_get_rtc_time msg;
>  	struct imx_sc_rpc_msg *hdr = &msg.hdr;
> diff --git a/drivers/soc/imx/soc-imx-scu.c b/drivers/soc/imx/soc-imx-scu.c
> index fb70b8a3f7c5..20d37eaeb5f2 100644
> --- a/drivers/soc/imx/soc-imx-scu.c
> +++ b/drivers/soc/imx/soc-imx-scu.c
> @@ -23,11 +23,11 @@ struct imx_sc_msg_misc_get_soc_id {
>  		} __packed req;
>  		struct {
>  			u32 id;
>  		} resp;
>  	} data;
> -} __packed;
> +} __packed __aligned(4);
>  
>  struct imx_sc_msg_misc_get_soc_uid {
>  	struct imx_sc_rpc_msg hdr;
>  	u32 uid_low;
>  	u32 uid_high;
> -- 
> 2.17.1
> 

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] firmware: imx: Align imx SC msg structs to 4
  2020-02-17  6:21 ` Shawn Guo
@ 2020-02-17 20:37   ` Leonard Crestez
  2020-02-18  9:18     ` Shawn Guo
  0 siblings, 1 reply; 9+ messages in thread
From: Leonard Crestez @ 2020-02-17 20:37 UTC (permalink / raw)
  To: Shawn Guo, Aisheng Dong
  Cc: Fabio Estevam, Michael Turquette, Stephen Boyd, Stefan Agner,
	Linus Walleij, Alessandro Zummo, Alexandre Belloni, Anson Huang,
	Abel Vesa, Franck Lenormand, kernel, dl-linux-imx,
	linux-arm-kernel, open list:COMMON CLK FRAMEWORK,
	open list:PIN CONTROLLER - FREESCALE,
	open list:REAL TIME CLOCK (RTC) SUBSYSTEM

On 17.02.2020 08:21, Shawn Guo wrote:
> On Tue, Feb 11, 2020 at 11:24:33PM +0200, Leonard Crestez wrote:
>> The imx SC api strongly assumes that messages are composed out of
>> 4-bytes words but some of our message structs have sizeof "6" and "7".
>>
>> This produces many oopses with CONFIG_KASAN=y:
>>
>> 	BUG: KASAN: stack-out-of-bounds in imx_mu_send_data+0x108/0x1f0
>>
>> It shouldn't cause an issues in normal use because these structs are
>> always allocated on the stack.
>>
>> Cc: stable@vger.kernel.org
> 
> Should we have a fixes tag and send it for -rc?

I haven't check but this would probably have to be split into multiple 
patches because the structs were not added all at once.

> Shawn
> 
>> Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com>
>> Reported-by: Iuliana Prodan <iuliana.prodan@nxp.com>
>> ---
>>   drivers/clk/imx/clk-scu.c               | 8 ++++----
>>   drivers/firmware/imx/misc.c             | 8 ++++----
>>   drivers/firmware/imx/scu-pd.c           | 2 +-
>>   drivers/pinctrl/freescale/pinctrl-scu.c | 4 ++--
>>   drivers/rtc/rtc-imx-sc.c                | 2 +-
>>   drivers/soc/imx/soc-imx-scu.c           | 2 +-
>>   6 files changed, 13 insertions(+), 13 deletions(-)
>>
>> diff --git a/drivers/clk/imx/clk-scu.c b/drivers/clk/imx/clk-scu.c
>> index fbef740704d0..b8b2072742a5 100644
>> --- a/drivers/clk/imx/clk-scu.c
>> +++ b/drivers/clk/imx/clk-scu.c
>> @@ -41,16 +41,16 @@ struct clk_scu {
>>   struct imx_sc_msg_req_set_clock_rate {
>>   	struct imx_sc_rpc_msg hdr;
>>   	__le32 rate;
>>   	__le16 resource;
>>   	u8 clk;
>> -} __packed;
>> +} __packed __aligned(4);
>>   
>>   struct req_get_clock_rate {
>>   	__le16 resource;
>>   	u8 clk;
>> -} __packed;
>> +} __packed __aligned(4);
>>   
>>   struct resp_get_clock_rate {
>>   	__le32 rate;
>>   };
>>   
>> @@ -82,11 +82,11 @@ struct imx_sc_msg_get_clock_parent {
>>   	struct imx_sc_rpc_msg hdr;
>>   	union {
>>   		struct req_get_clock_parent {
>>   			__le16 resource;
>>   			u8 clk;
>> -		} __packed req;
>> +		} __packed __aligned(4) req;
>>   		struct resp_get_clock_parent {
>>   			u8 parent;
>>   		} resp;
>>   	} data;
>>   };
>> @@ -119,11 +119,11 @@ struct imx_sc_msg_req_clock_enable {
>>   	struct imx_sc_rpc_msg hdr;
>>   	__le16 resource;
>>   	u8 clk;
>>   	u8 enable;
>>   	u8 autog;
>> -} __packed;
>> +} __packed __aligned(4);
>>   
>>   static inline struct clk_scu *to_clk_scu(struct clk_hw *hw)
>>   {
>>   	return container_of(hw, struct clk_scu, hw);
>>   }
>> diff --git a/drivers/firmware/imx/misc.c b/drivers/firmware/imx/misc.c
>> index 4b56a587dacd..d073cb3ce699 100644
>> --- a/drivers/firmware/imx/misc.c
>> +++ b/drivers/firmware/imx/misc.c
>> @@ -14,30 +14,30 @@
>>   struct imx_sc_msg_req_misc_set_ctrl {
>>   	struct imx_sc_rpc_msg hdr;
>>   	u32 ctrl;
>>   	u32 val;
>>   	u16 resource;
>> -} __packed;
>> +} __packed __aligned(4);
>>   
>>   struct imx_sc_msg_req_cpu_start {
>>   	struct imx_sc_rpc_msg hdr;
>>   	u32 address_hi;
>>   	u32 address_lo;
>>   	u16 resource;
>>   	u8 enable;
>> -} __packed;
>> +} __packed __aligned(4);
>>   
>>   struct imx_sc_msg_req_misc_get_ctrl {
>>   	struct imx_sc_rpc_msg hdr;
>>   	u32 ctrl;
>>   	u16 resource;
>> -} __packed;
>> +} __packed __aligned(4);
>>   
>>   struct imx_sc_msg_resp_misc_get_ctrl {
>>   	struct imx_sc_rpc_msg hdr;
>>   	u32 val;
>> -} __packed;
>> +} __packed __aligned(4);
>>   
>>   /*
>>    * This function sets a miscellaneous control value.
>>    *
>>    * @param[in]     ipc         IPC handle
>> diff --git a/drivers/firmware/imx/scu-pd.c b/drivers/firmware/imx/scu-pd.c
>> index b556612207e5..af3ae0087de4 100644
>> --- a/drivers/firmware/imx/scu-pd.c
>> +++ b/drivers/firmware/imx/scu-pd.c
>> @@ -59,11 +59,11 @@
>>   /* SCU Power Mode Protocol definition */
>>   struct imx_sc_msg_req_set_resource_power_mode {
>>   	struct imx_sc_rpc_msg hdr;
>>   	u16 resource;
>>   	u8 mode;
>> -} __packed;
>> +} __packed __aligned(4);
>>   
>>   #define IMX_SCU_PD_NAME_SIZE 20
>>   struct imx_sc_pm_domain {
>>   	struct generic_pm_domain pd;
>>   	char name[IMX_SCU_PD_NAME_SIZE];
>> diff --git a/drivers/pinctrl/freescale/pinctrl-scu.c b/drivers/pinctrl/freescale/pinctrl-scu.c
>> index 73bf1d9f9cc6..23cf04bdfc55 100644
>> --- a/drivers/pinctrl/freescale/pinctrl-scu.c
>> +++ b/drivers/pinctrl/freescale/pinctrl-scu.c
>> @@ -21,16 +21,16 @@ enum pad_func_e {
>>   
>>   struct imx_sc_msg_req_pad_set {
>>   	struct imx_sc_rpc_msg hdr;
>>   	u32 val;
>>   	u16 pad;
>> -} __packed;
>> +} __packed __aligned(4);
>>   
>>   struct imx_sc_msg_req_pad_get {
>>   	struct imx_sc_rpc_msg hdr;
>>   	u16 pad;
>> -} __packed;
>> +} __packed __aligned(4);
>>   
>>   struct imx_sc_msg_resp_pad_get {
>>   	struct imx_sc_rpc_msg hdr;
>>   	u32 val;
>>   } __packed;
>> diff --git a/drivers/rtc/rtc-imx-sc.c b/drivers/rtc/rtc-imx-sc.c
>> index cf2c12107f2b..a5f59e6f862e 100644
>> --- a/drivers/rtc/rtc-imx-sc.c
>> +++ b/drivers/rtc/rtc-imx-sc.c
>> @@ -35,11 +35,11 @@ struct imx_sc_msg_timer_rtc_set_alarm {
>>   	u8 mon;
>>   	u8 day;
>>   	u8 hour;
>>   	u8 min;
>>   	u8 sec;
>> -} __packed;
>> +} __packed __aligned(4);
>>   
>>   static int imx_sc_rtc_read_time(struct device *dev, struct rtc_time *tm)
>>   {
>>   	struct imx_sc_msg_timer_get_rtc_time msg;
>>   	struct imx_sc_rpc_msg *hdr = &msg.hdr;
>> diff --git a/drivers/soc/imx/soc-imx-scu.c b/drivers/soc/imx/soc-imx-scu.c
>> index fb70b8a3f7c5..20d37eaeb5f2 100644
>> --- a/drivers/soc/imx/soc-imx-scu.c
>> +++ b/drivers/soc/imx/soc-imx-scu.c
>> @@ -23,11 +23,11 @@ struct imx_sc_msg_misc_get_soc_id {
>>   		} __packed req;
>>   		struct {
>>   			u32 id;
>>   		} resp;
>>   	} data;
>> -} __packed;
>> +} __packed __aligned(4);
>>   
>>   struct imx_sc_msg_misc_get_soc_uid {
>>   	struct imx_sc_rpc_msg hdr;
>>   	u32 uid_low;
>>   	u32 uid_high;
>> -- 
>> 2.17.1
>>
> 


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] firmware: imx: Align imx SC msg structs to 4
  2020-02-17 20:37   ` Leonard Crestez
@ 2020-02-18  9:18     ` Shawn Guo
  2020-02-18 17:48       ` Leonard Crestez
  0 siblings, 1 reply; 9+ messages in thread
From: Shawn Guo @ 2020-02-18  9:18 UTC (permalink / raw)
  To: Leonard Crestez
  Cc: Aisheng Dong, Fabio Estevam, Michael Turquette, Stephen Boyd,
	Stefan Agner, Linus Walleij, Alessandro Zummo, Alexandre Belloni,
	Anson Huang, Abel Vesa, Franck Lenormand, kernel, dl-linux-imx,
	linux-arm-kernel, open list:COMMON CLK FRAMEWORK,
	open list:PIN CONTROLLER - FREESCALE,
	open list:REAL TIME CLOCK (RTC) SUBSYSTEM

On Mon, Feb 17, 2020 at 08:37:45PM +0000, Leonard Crestez wrote:
> On 17.02.2020 08:21, Shawn Guo wrote:
> > On Tue, Feb 11, 2020 at 11:24:33PM +0200, Leonard Crestez wrote:
> >> The imx SC api strongly assumes that messages are composed out of
> >> 4-bytes words but some of our message structs have sizeof "6" and "7".
> >>
> >> This produces many oopses with CONFIG_KASAN=y:
> >>
> >> 	BUG: KASAN: stack-out-of-bounds in imx_mu_send_data+0x108/0x1f0
> >>
> >> It shouldn't cause an issues in normal use because these structs are
> >> always allocated on the stack.
> >>
> >> Cc: stable@vger.kernel.org
> > 
> > Should we have a fixes tag and send it for -rc?
> 
> I haven't check but this would probably have to be split into multiple 
> patches because the structs were not added all at once.

Or maybe we can just drop the stable tag, as it addresses a corner
case issue which could concern very few people?

Shawn

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] firmware: imx: Align imx SC msg structs to 4
  2020-02-11 21:24 [PATCH] firmware: imx: Align imx SC msg structs to 4 Leonard Crestez
  2020-02-17  6:21 ` Shawn Guo
@ 2020-02-18  9:52 ` Alexandre Belloni
  2020-02-19 23:57 ` Stephen Boyd
  2 siblings, 0 replies; 9+ messages in thread
From: Alexandre Belloni @ 2020-02-18  9:52 UTC (permalink / raw)
  To: Leonard Crestez
  Cc: Shawn Guo, Dong Aisheng, Fabio Estevam, Michael Turquette,
	Stephen Boyd, Stefan Agner, Linus Walleij, Alessandro Zummo,
	Anson Huang, Abel Vesa, Franck LENORMAND, kernel, linux-imx,
	linux-arm-kernel, open list:COMMON CLK FRAMEWORK,
	open list:PIN CONTROLLER - FREESCALE,
	open list:REAL TIME CLOCK (RTC) SUBSYSTEM

On 11/02/2020 23:24:33+0200, Leonard Crestez wrote:
> The imx SC api strongly assumes that messages are composed out of
> 4-bytes words but some of our message structs have sizeof "6" and "7".
> 
> This produces many oopses with CONFIG_KASAN=y:
> 
> 	BUG: KASAN: stack-out-of-bounds in imx_mu_send_data+0x108/0x1f0
> 
> It shouldn't cause an issues in normal use because these structs are
> always allocated on the stack.
> 
> Cc: stable@vger.kernel.org
> Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com>
> Reported-by: Iuliana Prodan <iuliana.prodan@nxp.com>
Acked-by: Alexandre Belloni <alexandre.belloni@bootlin.com>

> ---
>  drivers/clk/imx/clk-scu.c               | 8 ++++----
>  drivers/firmware/imx/misc.c             | 8 ++++----
>  drivers/firmware/imx/scu-pd.c           | 2 +-
>  drivers/pinctrl/freescale/pinctrl-scu.c | 4 ++--
>  drivers/rtc/rtc-imx-sc.c                | 2 +-
>  drivers/soc/imx/soc-imx-scu.c           | 2 +-
>  6 files changed, 13 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/clk/imx/clk-scu.c b/drivers/clk/imx/clk-scu.c
> index fbef740704d0..b8b2072742a5 100644
> --- a/drivers/clk/imx/clk-scu.c
> +++ b/drivers/clk/imx/clk-scu.c
> @@ -41,16 +41,16 @@ struct clk_scu {
>  struct imx_sc_msg_req_set_clock_rate {
>  	struct imx_sc_rpc_msg hdr;
>  	__le32 rate;
>  	__le16 resource;
>  	u8 clk;
> -} __packed;
> +} __packed __aligned(4);
>  
>  struct req_get_clock_rate {
>  	__le16 resource;
>  	u8 clk;
> -} __packed;
> +} __packed __aligned(4);
>  
>  struct resp_get_clock_rate {
>  	__le32 rate;
>  };
>  
> @@ -82,11 +82,11 @@ struct imx_sc_msg_get_clock_parent {
>  	struct imx_sc_rpc_msg hdr;
>  	union {
>  		struct req_get_clock_parent {
>  			__le16 resource;
>  			u8 clk;
> -		} __packed req;
> +		} __packed __aligned(4) req;
>  		struct resp_get_clock_parent {
>  			u8 parent;
>  		} resp;
>  	} data;
>  };
> @@ -119,11 +119,11 @@ struct imx_sc_msg_req_clock_enable {
>  	struct imx_sc_rpc_msg hdr;
>  	__le16 resource;
>  	u8 clk;
>  	u8 enable;
>  	u8 autog;
> -} __packed;
> +} __packed __aligned(4);
>  
>  static inline struct clk_scu *to_clk_scu(struct clk_hw *hw)
>  {
>  	return container_of(hw, struct clk_scu, hw);
>  }
> diff --git a/drivers/firmware/imx/misc.c b/drivers/firmware/imx/misc.c
> index 4b56a587dacd..d073cb3ce699 100644
> --- a/drivers/firmware/imx/misc.c
> +++ b/drivers/firmware/imx/misc.c
> @@ -14,30 +14,30 @@
>  struct imx_sc_msg_req_misc_set_ctrl {
>  	struct imx_sc_rpc_msg hdr;
>  	u32 ctrl;
>  	u32 val;
>  	u16 resource;
> -} __packed;
> +} __packed __aligned(4);
>  
>  struct imx_sc_msg_req_cpu_start {
>  	struct imx_sc_rpc_msg hdr;
>  	u32 address_hi;
>  	u32 address_lo;
>  	u16 resource;
>  	u8 enable;
> -} __packed;
> +} __packed __aligned(4);
>  
>  struct imx_sc_msg_req_misc_get_ctrl {
>  	struct imx_sc_rpc_msg hdr;
>  	u32 ctrl;
>  	u16 resource;
> -} __packed;
> +} __packed __aligned(4);
>  
>  struct imx_sc_msg_resp_misc_get_ctrl {
>  	struct imx_sc_rpc_msg hdr;
>  	u32 val;
> -} __packed;
> +} __packed __aligned(4);
>  
>  /*
>   * This function sets a miscellaneous control value.
>   *
>   * @param[in]     ipc         IPC handle
> diff --git a/drivers/firmware/imx/scu-pd.c b/drivers/firmware/imx/scu-pd.c
> index b556612207e5..af3ae0087de4 100644
> --- a/drivers/firmware/imx/scu-pd.c
> +++ b/drivers/firmware/imx/scu-pd.c
> @@ -59,11 +59,11 @@
>  /* SCU Power Mode Protocol definition */
>  struct imx_sc_msg_req_set_resource_power_mode {
>  	struct imx_sc_rpc_msg hdr;
>  	u16 resource;
>  	u8 mode;
> -} __packed;
> +} __packed __aligned(4);
>  
>  #define IMX_SCU_PD_NAME_SIZE 20
>  struct imx_sc_pm_domain {
>  	struct generic_pm_domain pd;
>  	char name[IMX_SCU_PD_NAME_SIZE];
> diff --git a/drivers/pinctrl/freescale/pinctrl-scu.c b/drivers/pinctrl/freescale/pinctrl-scu.c
> index 73bf1d9f9cc6..23cf04bdfc55 100644
> --- a/drivers/pinctrl/freescale/pinctrl-scu.c
> +++ b/drivers/pinctrl/freescale/pinctrl-scu.c
> @@ -21,16 +21,16 @@ enum pad_func_e {
>  
>  struct imx_sc_msg_req_pad_set {
>  	struct imx_sc_rpc_msg hdr;
>  	u32 val;
>  	u16 pad;
> -} __packed;
> +} __packed __aligned(4);
>  
>  struct imx_sc_msg_req_pad_get {
>  	struct imx_sc_rpc_msg hdr;
>  	u16 pad;
> -} __packed;
> +} __packed __aligned(4);
>  
>  struct imx_sc_msg_resp_pad_get {
>  	struct imx_sc_rpc_msg hdr;
>  	u32 val;
>  } __packed;
> diff --git a/drivers/rtc/rtc-imx-sc.c b/drivers/rtc/rtc-imx-sc.c
> index cf2c12107f2b..a5f59e6f862e 100644
> --- a/drivers/rtc/rtc-imx-sc.c
> +++ b/drivers/rtc/rtc-imx-sc.c
> @@ -35,11 +35,11 @@ struct imx_sc_msg_timer_rtc_set_alarm {
>  	u8 mon;
>  	u8 day;
>  	u8 hour;
>  	u8 min;
>  	u8 sec;
> -} __packed;
> +} __packed __aligned(4);
>  
>  static int imx_sc_rtc_read_time(struct device *dev, struct rtc_time *tm)
>  {
>  	struct imx_sc_msg_timer_get_rtc_time msg;
>  	struct imx_sc_rpc_msg *hdr = &msg.hdr;
> diff --git a/drivers/soc/imx/soc-imx-scu.c b/drivers/soc/imx/soc-imx-scu.c
> index fb70b8a3f7c5..20d37eaeb5f2 100644
> --- a/drivers/soc/imx/soc-imx-scu.c
> +++ b/drivers/soc/imx/soc-imx-scu.c
> @@ -23,11 +23,11 @@ struct imx_sc_msg_misc_get_soc_id {
>  		} __packed req;
>  		struct {
>  			u32 id;
>  		} resp;
>  	} data;
> -} __packed;
> +} __packed __aligned(4);
>  
>  struct imx_sc_msg_misc_get_soc_uid {
>  	struct imx_sc_rpc_msg hdr;
>  	u32 uid_low;
>  	u32 uid_high;
> -- 
> 2.17.1
> 

-- 
Alexandre Belloni, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] firmware: imx: Align imx SC msg structs to 4
  2020-02-18  9:18     ` Shawn Guo
@ 2020-02-18 17:48       ` Leonard Crestez
  2020-02-18 19:02         ` Sasha Levin
  0 siblings, 1 reply; 9+ messages in thread
From: Leonard Crestez @ 2020-02-18 17:48 UTC (permalink / raw)
  To: Shawn Guo, Sasha Levin
  Cc: Aisheng Dong, Fabio Estevam, Michael Turquette, Stephen Boyd,
	Stefan Agner, Linus Walleij, Alessandro Zummo, Alexandre Belloni,
	Anson Huang, Abel Vesa, Franck Lenormand, kernel, dl-linux-imx,
	linux-arm-kernel, open list:COMMON CLK FRAMEWORK,
	open list:PIN CONTROLLER - FREESCALE,
	open list:REAL TIME CLOCK (RTC) SUBSYSTEM

On 18.02.2020 11:18, Shawn Guo wrote:
> On Mon, Feb 17, 2020 at 08:37:45PM +0000, Leonard Crestez wrote:
>> On 17.02.2020 08:21, Shawn Guo wrote:
>>> On Tue, Feb 11, 2020 at 11:24:33PM +0200, Leonard Crestez wrote:
>>>> The imx SC api strongly assumes that messages are composed out of
>>>> 4-bytes words but some of our message structs have sizeof "6" and "7".
>>>>
>>>> This produces many oopses with CONFIG_KASAN=y:
>>>>
>>>> 	BUG: KASAN: stack-out-of-bounds in imx_mu_send_data+0x108/0x1f0
>>>>
>>>> It shouldn't cause an issues in normal use because these structs are
>>>> always allocated on the stack.
>>>>
>>>> Cc: stable@vger.kernel.org
>>>
>>> Should we have a fixes tag and send it for -rc?
>>
>> I haven't check but this would probably have to be split into multiple
>> patches because the structs were not added all at once.
> 
> Or maybe we can just drop the stable tag, as it addresses a corner
> case issue which could concern very few people?

I think that "kernel does not boot with KASAN=y" is an issue worth fixing.

I will split and resend with appropriate Fixes: tags.

It seems likely that this will be picked up for -stable anyway via 
Sasha's automation scripts and those scripts benefit from Fixes: tags.

--
Regards,
Leonard


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] firmware: imx: Align imx SC msg structs to 4
  2020-02-18 17:48       ` Leonard Crestez
@ 2020-02-18 19:02         ` Sasha Levin
  0 siblings, 0 replies; 9+ messages in thread
From: Sasha Levin @ 2020-02-18 19:02 UTC (permalink / raw)
  To: Leonard Crestez
  Cc: Shawn Guo, Aisheng Dong, Fabio Estevam, Michael Turquette,
	Stephen Boyd, Stefan Agner, Linus Walleij, Alessandro Zummo,
	Alexandre Belloni, Anson Huang, Abel Vesa, Franck Lenormand,
	kernel, dl-linux-imx, linux-arm-kernel,
	open list:COMMON CLK FRAMEWORK,
	open list:PIN CONTROLLER - FREESCALE,
	open list:REAL TIME CLOCK (RTC) SUBSYSTEM

On Tue, Feb 18, 2020 at 05:48:50PM +0000, Leonard Crestez wrote:
>On 18.02.2020 11:18, Shawn Guo wrote:
>> On Mon, Feb 17, 2020 at 08:37:45PM +0000, Leonard Crestez wrote:
>>> On 17.02.2020 08:21, Shawn Guo wrote:
>>>> On Tue, Feb 11, 2020 at 11:24:33PM +0200, Leonard Crestez wrote:
>>>>> The imx SC api strongly assumes that messages are composed out of
>>>>> 4-bytes words but some of our message structs have sizeof "6" and "7".
>>>>>
>>>>> This produces many oopses with CONFIG_KASAN=y:
>>>>>
>>>>> 	BUG: KASAN: stack-out-of-bounds in imx_mu_send_data+0x108/0x1f0
>>>>>
>>>>> It shouldn't cause an issues in normal use because these structs are
>>>>> always allocated on the stack.
>>>>>
>>>>> Cc: stable@vger.kernel.org
>>>>
>>>> Should we have a fixes tag and send it for -rc?
>>>
>>> I haven't check but this would probably have to be split into multiple
>>> patches because the structs were not added all at once.
>>
>> Or maybe we can just drop the stable tag, as it addresses a corner
>> case issue which could concern very few people?
>
>I think that "kernel does not boot with KASAN=y" is an issue worth fixing.
>
>I will split and resend with appropriate Fixes: tags.
>
>It seems likely that this will be picked up for -stable anyway via
>Sasha's automation scripts and those scripts benefit from Fixes: tags.

Even if not, we realy very much on KASAN working on stable kernels, so
please do fix this :)

-- 
Thanks,
Sasha

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] firmware: imx: Align imx SC msg structs to 4
  2020-02-11 21:24 [PATCH] firmware: imx: Align imx SC msg structs to 4 Leonard Crestez
  2020-02-17  6:21 ` Shawn Guo
  2020-02-18  9:52 ` Alexandre Belloni
@ 2020-02-19 23:57 ` Stephen Boyd
  2020-02-20 12:25   ` Leonard Crestez
  2 siblings, 1 reply; 9+ messages in thread
From: Stephen Boyd @ 2020-02-19 23:57 UTC (permalink / raw)
  To: Dong Aisheng, Leonard Crestez, Shawn Guo
  Cc: Fabio Estevam, Michael Turquette, Stefan Agner, Linus Walleij,
	Alessandro Zummo, Alexandre Belloni, Anson Huang, Abel Vesa,
	Franck LENORMAND, kernel, linux-imx, linux-arm-kernel, linux-clk,
	linux-gpio, linux-rtc

Quoting Leonard Crestez (2020-02-11 13:24:33)
> The imx SC api strongly assumes that messages are composed out of
> 4-bytes words but some of our message structs have sizeof "6" and "7".
> 
> This produces many oopses with CONFIG_KASAN=y:
> 
>         BUG: KASAN: stack-out-of-bounds in imx_mu_send_data+0x108/0x1f0

Can you share the full kasan bug report instead of the single line?

> 
> It shouldn't cause an issues in normal use because these structs are
> always allocated on the stack.

Is packed necessary for these? I thought that if the beginning of the
struct was naturally aligned and there was sometimes a byte or two at
the end then having __packed wasn't useful. So maybe it's better to just
drop __packed on all these structs and let the compiler decide it can
add some padding on the stack? Or do we have arrays of these structs
sitting in memory all next to each other and they need to be that way to
be sent through the mailbox?

> 
> Cc: stable@vger.kernel.org
> Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com>
> Reported-by: Iuliana Prodan <iuliana.prodan@nxp.com>
> ---

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] firmware: imx: Align imx SC msg structs to 4
  2020-02-19 23:57 ` Stephen Boyd
@ 2020-02-20 12:25   ` Leonard Crestez
  0 siblings, 0 replies; 9+ messages in thread
From: Leonard Crestez @ 2020-02-20 12:25 UTC (permalink / raw)
  To: Stephen Boyd
  Cc: Aisheng Dong, Shawn Guo, Fabio Estevam, Michael Turquette,
	Stefan Agner, Linus Walleij, Alessandro Zummo, Alexandre Belloni,
	Anson Huang, Abel Vesa, Franck Lenormand, kernel, dl-linux-imx,
	linux-arm-kernel, linux-clk, linux-gpio, linux-rtc

On 20.02.2020 01:57, Stephen Boyd wrote:
> Quoting Leonard Crestez (2020-02-11 13:24:33)
>> The imx SC api strongly assumes that messages are composed out of
>> 4-bytes words but some of our message structs have sizeof "6" and "7".
>>
>> This produces many oopses with CONFIG_KASAN=y:
>>
>>          BUG: KASAN: stack-out-of-bounds in imx_mu_send_data+0x108/0x1f0
> 
> Can you share the full kasan bug report instead of the single line?

[    1.606708] imx-scu scu: NXP i.MX SCU Initialized
[    1.635265] random: fast init done
[    1.652200] 
==================================================================
[    1.659118] BUG: KASAN: stack-out-of-bounds in 
imx_mu_send_data+0x108/0x1f0
[    1.666046] Read of size 4 at addr ffff0008c80e6bc4 by task swapper/0/1
[    1.672642]
[    1.674134] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 
5.4.3-03848-g13efcd6 #54
[    1.681335] Hardware name: Freescale i.MX8QM MEK (DT)
[    1.686373] Call trace:
[    1.688815]  dump_backtrace+0x0/0x1e8
[    1.692458]  show_stack+0x14/0x20
[    1.695766]  dump_stack+0xe8/0x140
[    1.699155]  print_address_description.isra.11+0x64/0x348
[    1.704532]  __kasan_report+0x11c/0x230
[    1.708356]  kasan_report+0xc/0x18
[    1.711743]  __asan_load4+0x90/0xb0
[    1.715218]  imx_mu_send_data+0x108/0x1f0
[    1.719215]  msg_submit+0x104/0x180
[    1.722689]  mbox_send_message+0xa8/0x1a0
[    1.726696]  imx_scu_call_rpc+0x168/0x310
[    1.730679]  imx_sc_pd_power+0x180/0x1e0
[    1.734589]  imx_sc_pd_power_on+0x10/0x18
[    1.738598]  genpd_power_on.part.23+0x118/0x2a8
[    1.743105]  genpd_runtime_resume+0x138/0x320
[    1.747454]  __rpm_callback+0xb0/0x1a0
[    1.751184]  rpm_callback+0x34/0xe0
[    1.754659]  rpm_resume+0x5b8/0x7e8
[    1.758137]  __pm_runtime_resume+0x38/0x90
[    1.762222]  imx_clk_scu_probe+0x5c/0x1c8
[    1.766218]  platform_drv_probe+0x6c/0xc8
[    1.770217]  really_probe+0x148/0x428
[    1.773861]  driver_probe_device+0x74/0x130
[    1.778031]  __device_attach_driver+0xc4/0xe8
[    1.782380]  bus_for_each_drv+0xf0/0x158
[    1.786283]  __device_attach+0x158/0x1d8
[    1.790195]  device_initial_probe+0x10/0x18
[    1.794362]  bus_probe_device+0xe0/0xf0
[    1.798185]  device_add+0x660/0x998
[    1.801659]  platform_device_add+0x198/0x340
[    1.805916]  imx_clk_scu_alloc_dev+0x1b8/0x1e8
[    1.810347]  imx8qxp_clk_probe+0x19d0/0x28b8
[    1.814601]  platform_drv_probe+0x6c/0xc8
[    1.818601]  really_probe+0x148/0x428
[    1.822250]  driver_probe_device+0x74/0x130
[    1.826423]  __device_attach_driver+0xc4/0xe8
[    1.830763]  bus_for_each_drv+0xf0/0x158
[    1.834675]  __device_attach+0x158/0x1d8
[    1.838583]  device_initial_probe+0x10/0x18
[    1.842752]  bus_probe_device+0xe0/0xf0
[    1.846572]  device_add+0x660/0x998
[    1.850058]  of_device_add+0x74/0x98
[    1.853610]  of_platform_device_create_pdata+0x11c/0x178
[    1.858908]  of_platform_bus_create+0x404/0x4f0
[    1.863425]  of_platform_populate+0x74/0x110
[    1.867688]  devm_of_platform_populate+0x54/0xb8
[    1.872291]  imx_scu_probe+0x1b8/0x220
[    1.876022]  platform_drv_probe+0x6c/0xc8
[    1.880021]  really_probe+0x148/0x428
[    1.883671]  driver_probe_device+0x74/0x130
[    1.887841]  device_driver_attach+0x94/0xa0
[    1.892010]  __driver_attach+0x70/0x110
[    1.895832]  bus_for_each_dev+0xe8/0x158
[    1.899741]  driver_attach+0x30/0x40
[    1.903303]  bus_add_driver+0x1b0/0x2b8
[    1.907129]  driver_register+0xbc/0x1d0
[    1.910947]  __platform_driver_register+0x7c/0x88
[    1.915653]  imx_scu_driver_init+0x18/0x20
[    1.919725]  do_one_initcall+0xd4/0x244
[    1.923552]  kernel_init_freeable+0x238/0x2d4
[    1.927889]  kernel_init+0x10/0x114
[    1.931365]  ret_from_fork+0x10/0x18
[    1.934917]
[    1.936399] The buggy address belongs to the page:
[    1.941184] page:fffffe0023003980 refcount:0 mapcount:0 
mapping:0000000000000000 index:0x0
[    1.949447] raw: 1ffff00000000000 fffffe0023003988 fffffe0023003988 
0000000000000000
[    1.957170] raw: 0000000000000000 0000000000000000 00000000ffffffff 
0000000000000000
[    1.964894] page dumped because: kasan: bad access detected
[    1.970449]
[    1.971933] addr ffff0008c80e6bc4 is located in stack of task 
swapper/0/1 at offset 36 in frame:
[    1.980708]  imx_sc_pd_power+0x0/0x1e0
[    1.984442]
[    1.985917] this frame has 1 object:
[    1.989481]  [32, 39) 'msg'
[    1.989484]
[    1.993732] Memory state around the buggy address:
[    1.998520]  ffff0008c80e6a80: 00 00 00 00 00 00 00 00 00 00 f1 f1 f1 
f1 00 00
[    2.005730]  ffff0008c80e6b00: 00 00 f3 f3 f3 f3 00 00 00 00 00 00 00 
00 00 00
[    2.012940] >ffff0008c80e6b80: 00 00 00 00 f1 f1 f1 f1 07 f2 f2 f2 00 
00 00 00
[    2.020141]                                            ^
[    2.025448]  ffff0008c80e6c00: 00 00 00 00 00 00 00 00 00 00 00 00 00 
00 00 00
[    2.032659]  ffff0008c80e6c80: 00 00 00 00 00 00 00 00 00 00 00 00 00 
00 00 00
[    2.039862] 
==================================================================

This is actually from an NXP release branch but code is very close up 
upstream.

>> It shouldn't cause an issues in normal use because these structs are
>> always allocated on the stack.
> 
> Is packed necessary for these? I thought that if the beginning of the
> struct was naturally aligned and there was sometimes a byte or two at
> the end then having __packed wasn't useful. So maybe it's better to just
> drop __packed on all these structs and let the compiler decide it can
> add some padding on the stack? Or do we have arrays of these structs
> sitting in memory all next to each other and they need to be that way to
> be sent through the mailbox?

I'm not sure I understand the question: the structs are __packed because 
they represent the binary protocol for communicating with the "System 
Controller".

Without __packed the compiler could insert padding inside the structs 
and break the protocol.

As far as I understand compilers are still allowed to use padding on 
stack since that padding is outside the message struct itself.

--
Regards,
Leonard

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, back to index

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-02-11 21:24 [PATCH] firmware: imx: Align imx SC msg structs to 4 Leonard Crestez
2020-02-17  6:21 ` Shawn Guo
2020-02-17 20:37   ` Leonard Crestez
2020-02-18  9:18     ` Shawn Guo
2020-02-18 17:48       ` Leonard Crestez
2020-02-18 19:02         ` Sasha Levin
2020-02-18  9:52 ` Alexandre Belloni
2020-02-19 23:57 ` Stephen Boyd
2020-02-20 12:25   ` Leonard Crestez

Linux-RTC Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-rtc/0 linux-rtc/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-rtc linux-rtc/ https://lore.kernel.org/linux-rtc \
		linux-rtc@vger.kernel.org
	public-inbox-index linux-rtc

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-rtc


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git