* [PATCH] mmc-utils: mmc_cmds: fix type-punned warning on &ext_csd[] casting @ 2024-03-05 10:59 Giulio Benetti 2024-03-05 14:52 ` Avri Altman 0 siblings, 1 reply; 7+ messages in thread From: Giulio Benetti @ 2024-03-05 10:59 UTC (permalink / raw) To: linux-mmc; +Cc: Giulio Benetti When building with -Werror=strict-aliasing error is thrown: mmc_cmds.c: In function 'do_ffu': mmc_cmds.c:2972:2: error: dereferencing type-punned pointer will break strict-aliasing rules [-Werror=strict-aliasing] sect_done = htole32(*((__u32 *)&ext_csd[EXT_CSD_NUM_OF_FW_SEC_PROG_0])); ^ cc1: all warnings being treated as errors Let's fix type-punned breaking strict-aliasing by memcpy() the variable. Signed-off-by: Giulio Benetti <giulio.benetti@benettiengineering.com> --- mmc_cmds.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/mmc_cmds.c b/mmc_cmds.c index ae7b876..dcd60aa 100644 --- a/mmc_cmds.c +++ b/mmc_cmds.c @@ -2968,7 +2968,8 @@ do_retry: } /* Test if we need to restart the download */ - sect_done = htole32(*((__u32 *)&ext_csd[EXT_CSD_NUM_OF_FW_SEC_PROG_0])); + memcpy(§_done, &ext_csd[EXT_CSD_NUM_OF_FW_SEC_PROG_0], sizeof(__u32)); + sect_done = htole32(sect_done); /* By spec, host should re-start download from the first sector if sect_done is 0 */ if (sect_done == 0) { if (retry--) { -- 2.34.1 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* RE: [PATCH] mmc-utils: mmc_cmds: fix type-punned warning on &ext_csd[] casting 2024-03-05 10:59 [PATCH] mmc-utils: mmc_cmds: fix type-punned warning on &ext_csd[] casting Giulio Benetti @ 2024-03-05 14:52 ` Avri Altman 2024-03-05 15:01 ` Avri Altman 0 siblings, 1 reply; 7+ messages in thread From: Avri Altman @ 2024-03-05 14:52 UTC (permalink / raw) To: Giulio Benetti, linux-mmc@vger.kernel.org > When building with -Werror=strict-aliasing error is thrown: > > mmc_cmds.c: In function 'do_ffu': > mmc_cmds.c:2972:2: error: dereferencing type-punned pointer will break strict- > aliasing rules [-Werror=strict-aliasing] > sect_done = htole32(*((__u32 > *)&ext_csd[EXT_CSD_NUM_OF_FW_SEC_PROG_0])); > ^ > cc1: all warnings being treated as errors > > Let's fix type-punned breaking strict-aliasing by memcpy() the variable. If any, I would just revert commit a1b594ca735e. Thanks, Avri > > Signed-off-by: Giulio Benetti <giulio.benetti@benettiengineering.com> > --- > mmc_cmds.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/mmc_cmds.c b/mmc_cmds.c > index ae7b876..dcd60aa 100644 > --- a/mmc_cmds.c > +++ b/mmc_cmds.c > @@ -2968,7 +2968,8 @@ do_retry: > } > > /* Test if we need to restart the download */ > - sect_done = htole32(*((__u32 > *)&ext_csd[EXT_CSD_NUM_OF_FW_SEC_PROG_0])); > + memcpy(§_done, &ext_csd[EXT_CSD_NUM_OF_FW_SEC_PROG_0], > sizeof(__u32)); > + sect_done = htole32(sect_done); > /* By spec, host should re-start download from the first sector if sect_done > is 0 */ > if (sect_done == 0) { > if (retry--) { > -- > 2.34.1 > ^ permalink raw reply [flat|nested] 7+ messages in thread
* RE: [PATCH] mmc-utils: mmc_cmds: fix type-punned warning on &ext_csd[] casting 2024-03-05 14:52 ` Avri Altman @ 2024-03-05 15:01 ` Avri Altman 2024-03-05 18:45 ` [PATCH v2] " Giulio Benetti 2024-03-05 18:46 ` [PATCH] " Giulio Benetti 0 siblings, 2 replies; 7+ messages in thread From: Avri Altman @ 2024-03-05 15:01 UTC (permalink / raw) To: Giulio Benetti, linux-mmc@vger.kernel.org > > When building with -Werror=strict-aliasing error is thrown: > > > > mmc_cmds.c: In function 'do_ffu': > > mmc_cmds.c:2972:2: error: dereferencing type-punned pointer will break > > strict- aliasing rules [-Werror=strict-aliasing] > > sect_done = htole32(*((__u32 > > *)&ext_csd[EXT_CSD_NUM_OF_FW_SEC_PROG_0])); > > ^ > > cc1: all warnings being treated as errors > > > > Let's fix type-punned breaking strict-aliasing by memcpy() the variable. > If any, I would just revert commit a1b594ca735e. > > Thanks, > Avri Or, how about this untested option. Thanks, Avri index ae7b876..10f7532 100644 --- a/mmc_cmds.c +++ b/mmc_cmds.c @@ -58,6 +58,11 @@ #define WPTYPE_PWRON 2 #define WPTYPE_PERM 3 +static inline __u32 strict_htole32(__u8 *arr) +{ + return arr[0] | arr[1] << 8 | arr[2] << 16 | arr[3] << 24; +} + int read_extcsd(int fd, __u8 *ext_csd) { int ret = 0; @@ -2807,7 +2812,7 @@ static void set_ffu_single_cmd(struct mmc_ioc_multi_cmd *multi_cmd, __u8 *ext_csd, unsigned int bytes, __u8 *buf, off_t offset) { - __u32 arg = htole32(*((__u32 *)&ext_csd[EXT_CSD_FFU_ARG_0])); + __u32 arg = safe_htole32(&ext_csd[EXT_CSD_FFU_ARG_0]); /* send block count */ set_single_cmd(&multi_cmd->cmds[1], MMC_SET_BLOCK_COUNT, 0, 0, @@ -2827,7 +2832,7 @@ static void set_ffu_single_cmd(struct mmc_ioc_multi_cmd *multi_cmd, int do_ffu(int nargs, char **argv) { int dev_fd, img_fd; - int sect_done = 0, retry = 3, ret = -EINVAL; + int retry = 3, ret = -EINVAL; unsigned int sect_size; __u8 ext_csd[512]; __u8 *buf = NULL; @@ -2835,6 +2840,7 @@ int do_ffu(int nargs, char **argv) char *device; struct mmc_ioc_multi_cmd *multi_cmd = NULL; unsigned int default_chunk = MMC_IOC_MAX_BYTES; + __u32 sect_done = 0; assert (nargs == 3 || nargs == 4); @@ -2968,7 +2974,7 @@ do_retry: } /* Test if we need to restart the download */ - sect_done = htole32(*((__u32 *)&ext_csd[EXT_CSD_NUM_OF_FW_SEC_PROG_0])); + sect_done = strict_htole32(&ext_csd[EXT_CSD_NUM_OF_FW_SEC_PROG_0]); /* By spec, host should re-start download from the first sector if sect_done is 0 */ if (sect_done == 0) { if (retry--) { ^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH v2] mmc-utils: mmc_cmds: fix type-punned warning on &ext_csd[] casting 2024-03-05 15:01 ` Avri Altman @ 2024-03-05 18:45 ` Giulio Benetti 2024-03-05 19:48 ` Avri Altman 2024-03-07 11:02 ` Ulf Hansson 2024-03-05 18:46 ` [PATCH] " Giulio Benetti 1 sibling, 2 replies; 7+ messages in thread From: Giulio Benetti @ 2024-03-05 18:45 UTC (permalink / raw) To: Avri Altman; +Cc: linux-mmc, Giulio Benetti When building with -Werror=strict-aliasing error is thrown: mmc_cmds.c: In function 'do_ffu': mmc_cmds.c:2972:2: error: dereferencing type-punned pointer will break strict-aliasing rules [-Werror=strict-aliasing] sect_done = htole32(*((__u32 *)&ext_csd[EXT_CSD_NUM_OF_FW_SEC_PROG_0])); ^ cc1: all warnings being treated as errors Let's fix type-punned breaking strict-aliasing by implementing local per_byte_htole32(__u8 *) and use it for the 2 present occurences of htole32(). Let's also change sect_done type to __u32 for consistency. Suggested-by: Avri Altman <Avri.Altman@wdc.com> Fixes: a1b594ca735e ("mmc-utils: ffu: Simplify ext_csd bytes parsing") Signed-off-by: Giulio Benetti <giulio.benetti@benettiengineering.com> --- V1->V2: * implement local per_byte_htole32() as suggested by Avril Altman --- mmc_cmds.c | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/mmc_cmds.c b/mmc_cmds.c index ae7b876..936e0c5 100644 --- a/mmc_cmds.c +++ b/mmc_cmds.c @@ -58,6 +58,11 @@ #define WPTYPE_PWRON 2 #define WPTYPE_PERM 3 +static inline __u32 per_byte_htole32(__u8 *arr) +{ + return arr[0] | arr[1] << 8 | arr[2] << 16 | arr[3] << 24; +} + int read_extcsd(int fd, __u8 *ext_csd) { int ret = 0; @@ -2807,7 +2812,7 @@ static void set_ffu_single_cmd(struct mmc_ioc_multi_cmd *multi_cmd, __u8 *ext_csd, unsigned int bytes, __u8 *buf, off_t offset) { - __u32 arg = htole32(*((__u32 *)&ext_csd[EXT_CSD_FFU_ARG_0])); + __u32 arg = per_byte_htole32(&ext_csd[EXT_CSD_FFU_ARG_0]); /* send block count */ set_single_cmd(&multi_cmd->cmds[1], MMC_SET_BLOCK_COUNT, 0, 0, @@ -2827,7 +2832,7 @@ static void set_ffu_single_cmd(struct mmc_ioc_multi_cmd *multi_cmd, int do_ffu(int nargs, char **argv) { int dev_fd, img_fd; - int sect_done = 0, retry = 3, ret = -EINVAL; + int retry = 3, ret = -EINVAL; unsigned int sect_size; __u8 ext_csd[512]; __u8 *buf = NULL; @@ -2835,6 +2840,7 @@ int do_ffu(int nargs, char **argv) char *device; struct mmc_ioc_multi_cmd *multi_cmd = NULL; unsigned int default_chunk = MMC_IOC_MAX_BYTES; + __u32 sect_done = 0; assert (nargs == 3 || nargs == 4); @@ -2968,7 +2974,7 @@ do_retry: } /* Test if we need to restart the download */ - sect_done = htole32(*((__u32 *)&ext_csd[EXT_CSD_NUM_OF_FW_SEC_PROG_0])); + sect_done = per_byte_htole32(&ext_csd[EXT_CSD_NUM_OF_FW_SEC_PROG_0]); /* By spec, host should re-start download from the first sector if sect_done is 0 */ if (sect_done == 0) { if (retry--) { -- 2.34.1 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* RE: [PATCH v2] mmc-utils: mmc_cmds: fix type-punned warning on &ext_csd[] casting 2024-03-05 18:45 ` [PATCH v2] " Giulio Benetti @ 2024-03-05 19:48 ` Avri Altman 2024-03-07 11:02 ` Ulf Hansson 1 sibling, 0 replies; 7+ messages in thread From: Avri Altman @ 2024-03-05 19:48 UTC (permalink / raw) To: Giulio Benetti; +Cc: linux-mmc@vger.kernel.org > When building with -Werror=strict-aliasing error is thrown: > > mmc_cmds.c: In function 'do_ffu': > mmc_cmds.c:2972:2: error: dereferencing type-punned pointer will break strict- > aliasing rules [-Werror=strict-aliasing] > sect_done = htole32(*((__u32 > *)&ext_csd[EXT_CSD_NUM_OF_FW_SEC_PROG_0])); > ^ > cc1: all warnings being treated as errors > > Let's fix type-punned breaking strict-aliasing by implementing local > per_byte_htole32(__u8 *) and use it for the 2 present occurences of > htole32(). Let's also change sect_done type to __u32 for consistency. > > Suggested-by: Avri Altman <Avri.Altman@wdc.com> > Fixes: a1b594ca735e ("mmc-utils: ffu: Simplify ext_csd bytes parsing") > Signed-off-by: Giulio Benetti <giulio.benetti@benettiengineering.com> Reviewed-by: Avri Altman <avri.altman@wdc.com> > --- > V1->V2: > * implement local per_byte_htole32() as suggested by Avril Altman > --- > mmc_cmds.c | 12 +++++++++--- > 1 file changed, 9 insertions(+), 3 deletions(-) > > diff --git a/mmc_cmds.c b/mmc_cmds.c > index ae7b876..936e0c5 100644 > --- a/mmc_cmds.c > +++ b/mmc_cmds.c > @@ -58,6 +58,11 @@ > #define WPTYPE_PWRON 2 > #define WPTYPE_PERM 3 > > +static inline __u32 per_byte_htole32(__u8 *arr) > +{ > + return arr[0] | arr[1] << 8 | arr[2] << 16 | arr[3] << 24; > +} > + > int read_extcsd(int fd, __u8 *ext_csd) > { > int ret = 0; > @@ -2807,7 +2812,7 @@ static void set_ffu_single_cmd(struct > mmc_ioc_multi_cmd *multi_cmd, > __u8 *ext_csd, unsigned int bytes, __u8 *buf, > off_t offset) > { > - __u32 arg = htole32(*((__u32 *)&ext_csd[EXT_CSD_FFU_ARG_0])); > + __u32 arg = per_byte_htole32(&ext_csd[EXT_CSD_FFU_ARG_0]); > > /* send block count */ > set_single_cmd(&multi_cmd->cmds[1], MMC_SET_BLOCK_COUNT, 0, 0, > @@ -2827,7 +2832,7 @@ static void set_ffu_single_cmd(struct > mmc_ioc_multi_cmd *multi_cmd, > int do_ffu(int nargs, char **argv) > { > int dev_fd, img_fd; > - int sect_done = 0, retry = 3, ret = -EINVAL; > + int retry = 3, ret = -EINVAL; > unsigned int sect_size; > __u8 ext_csd[512]; > __u8 *buf = NULL; > @@ -2835,6 +2840,7 @@ int do_ffu(int nargs, char **argv) > char *device; > struct mmc_ioc_multi_cmd *multi_cmd = NULL; > unsigned int default_chunk = MMC_IOC_MAX_BYTES; > + __u32 sect_done = 0; > > assert (nargs == 3 || nargs == 4); > > @@ -2968,7 +2974,7 @@ do_retry: > } > > /* Test if we need to restart the download */ > - sect_done = htole32(*((__u32 > *)&ext_csd[EXT_CSD_NUM_OF_FW_SEC_PROG_0])); > + sect_done = > per_byte_htole32(&ext_csd[EXT_CSD_NUM_OF_FW_SEC_PROG_0]); > /* By spec, host should re-start download from the first sector if sect_done > is 0 */ > if (sect_done == 0) { > if (retry--) { > -- > 2.34.1 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2] mmc-utils: mmc_cmds: fix type-punned warning on &ext_csd[] casting 2024-03-05 18:45 ` [PATCH v2] " Giulio Benetti 2024-03-05 19:48 ` Avri Altman @ 2024-03-07 11:02 ` Ulf Hansson 1 sibling, 0 replies; 7+ messages in thread From: Ulf Hansson @ 2024-03-07 11:02 UTC (permalink / raw) To: Giulio Benetti; +Cc: Avri Altman, linux-mmc On Tue, 5 Mar 2024 at 19:52, Giulio Benetti <giulio.benetti@benettiengineering.com> wrote: > > When building with -Werror=strict-aliasing error is thrown: > > mmc_cmds.c: In function 'do_ffu': > mmc_cmds.c:2972:2: error: dereferencing type-punned pointer will break strict-aliasing rules [-Werror=strict-aliasing] > sect_done = htole32(*((__u32 *)&ext_csd[EXT_CSD_NUM_OF_FW_SEC_PROG_0])); > ^ > cc1: all warnings being treated as errors > > Let's fix type-punned breaking strict-aliasing by implementing local > per_byte_htole32(__u8 *) and use it for the 2 present occurences of > htole32(). Let's also change sect_done type to __u32 for consistency. > > Suggested-by: Avri Altman <Avri.Altman@wdc.com> > Fixes: a1b594ca735e ("mmc-utils: ffu: Simplify ext_csd bytes parsing") > Signed-off-by: Giulio Benetti <giulio.benetti@benettiengineering.com> Applied to git.kernel.org/pub/scm/utils/mmc/mmc-utils.git master, thanks! Kind regards Uffe > --- > V1->V2: > * implement local per_byte_htole32() as suggested by Avril Altman > --- > mmc_cmds.c | 12 +++++++++--- > 1 file changed, 9 insertions(+), 3 deletions(-) > > diff --git a/mmc_cmds.c b/mmc_cmds.c > index ae7b876..936e0c5 100644 > --- a/mmc_cmds.c > +++ b/mmc_cmds.c > @@ -58,6 +58,11 @@ > #define WPTYPE_PWRON 2 > #define WPTYPE_PERM 3 > > +static inline __u32 per_byte_htole32(__u8 *arr) > +{ > + return arr[0] | arr[1] << 8 | arr[2] << 16 | arr[3] << 24; > +} > + > int read_extcsd(int fd, __u8 *ext_csd) > { > int ret = 0; > @@ -2807,7 +2812,7 @@ static void set_ffu_single_cmd(struct mmc_ioc_multi_cmd *multi_cmd, > __u8 *ext_csd, unsigned int bytes, __u8 *buf, > off_t offset) > { > - __u32 arg = htole32(*((__u32 *)&ext_csd[EXT_CSD_FFU_ARG_0])); > + __u32 arg = per_byte_htole32(&ext_csd[EXT_CSD_FFU_ARG_0]); > > /* send block count */ > set_single_cmd(&multi_cmd->cmds[1], MMC_SET_BLOCK_COUNT, 0, 0, > @@ -2827,7 +2832,7 @@ static void set_ffu_single_cmd(struct mmc_ioc_multi_cmd *multi_cmd, > int do_ffu(int nargs, char **argv) > { > int dev_fd, img_fd; > - int sect_done = 0, retry = 3, ret = -EINVAL; > + int retry = 3, ret = -EINVAL; > unsigned int sect_size; > __u8 ext_csd[512]; > __u8 *buf = NULL; > @@ -2835,6 +2840,7 @@ int do_ffu(int nargs, char **argv) > char *device; > struct mmc_ioc_multi_cmd *multi_cmd = NULL; > unsigned int default_chunk = MMC_IOC_MAX_BYTES; > + __u32 sect_done = 0; > > assert (nargs == 3 || nargs == 4); > > @@ -2968,7 +2974,7 @@ do_retry: > } > > /* Test if we need to restart the download */ > - sect_done = htole32(*((__u32 *)&ext_csd[EXT_CSD_NUM_OF_FW_SEC_PROG_0])); > + sect_done = per_byte_htole32(&ext_csd[EXT_CSD_NUM_OF_FW_SEC_PROG_0]); > /* By spec, host should re-start download from the first sector if sect_done is 0 */ > if (sect_done == 0) { > if (retry--) { > -- > 2.34.1 > > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] mmc-utils: mmc_cmds: fix type-punned warning on &ext_csd[] casting 2024-03-05 15:01 ` Avri Altman 2024-03-05 18:45 ` [PATCH v2] " Giulio Benetti @ 2024-03-05 18:46 ` Giulio Benetti 1 sibling, 0 replies; 7+ messages in thread From: Giulio Benetti @ 2024-03-05 18:46 UTC (permalink / raw) To: Avri Altman, linux-mmc@vger.kernel.org On 05/03/24 16:01, Avri Altman wrote: >>> When building with -Werror=strict-aliasing error is thrown: >>> >>> mmc_cmds.c: In function 'do_ffu': >>> mmc_cmds.c:2972:2: error: dereferencing type-punned pointer will break >>> strict- aliasing rules [-Werror=strict-aliasing] >>> sect_done = htole32(*((__u32 >>> *)&ext_csd[EXT_CSD_NUM_OF_FW_SEC_PROG_0])); >>> ^ >>> cc1: all warnings being treated as errors >>> >>> Let's fix type-punned breaking strict-aliasing by memcpy() the variable. >> If any, I would just revert commit a1b594ca735e. >> >> Thanks, >> Avri > Or, how about this untested option. This is a good solution, just re-spin and renamed to per_byte_htole32(__u8 *). Hope it sounds good the name to you. Thank you Best regards -- Giulio Benetti CEO&CTO@Benetti Engineering sas > Thanks, > Avri > > > index ae7b876..10f7532 100644 > --- a/mmc_cmds.c > +++ b/mmc_cmds.c > @@ -58,6 +58,11 @@ > #define WPTYPE_PWRON 2 > #define WPTYPE_PERM 3 > > +static inline __u32 strict_htole32(__u8 *arr) > +{ > + return arr[0] | arr[1] << 8 | arr[2] << 16 | arr[3] << 24; > +} > + > int read_extcsd(int fd, __u8 *ext_csd) > { > int ret = 0; > @@ -2807,7 +2812,7 @@ static void set_ffu_single_cmd(struct mmc_ioc_multi_cmd *multi_cmd, > __u8 *ext_csd, unsigned int bytes, __u8 *buf, > off_t offset) > { > - __u32 arg = htole32(*((__u32 *)&ext_csd[EXT_CSD_FFU_ARG_0])); > + __u32 arg = safe_htole32(&ext_csd[EXT_CSD_FFU_ARG_0]); > > /* send block count */ > set_single_cmd(&multi_cmd->cmds[1], MMC_SET_BLOCK_COUNT, 0, 0, > @@ -2827,7 +2832,7 @@ static void set_ffu_single_cmd(struct mmc_ioc_multi_cmd *multi_cmd, > int do_ffu(int nargs, char **argv) > { > int dev_fd, img_fd; > - int sect_done = 0, retry = 3, ret = -EINVAL; > + int retry = 3, ret = -EINVAL; > unsigned int sect_size; > __u8 ext_csd[512]; > __u8 *buf = NULL; > @@ -2835,6 +2840,7 @@ int do_ffu(int nargs, char **argv) > char *device; > struct mmc_ioc_multi_cmd *multi_cmd = NULL; > unsigned int default_chunk = MMC_IOC_MAX_BYTES; > + __u32 sect_done = 0; > > assert (nargs == 3 || nargs == 4); > > @@ -2968,7 +2974,7 @@ do_retry: > } > > /* Test if we need to restart the download */ > - sect_done = htole32(*((__u32 *)&ext_csd[EXT_CSD_NUM_OF_FW_SEC_PROG_0])); > + sect_done = strict_htole32(&ext_csd[EXT_CSD_NUM_OF_FW_SEC_PROG_0]); > /* By spec, host should re-start download from the first sector if sect_done is 0 */ > if (sect_done == 0) { > if (retry--) { ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2024-03-07 11:03 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2024-03-05 10:59 [PATCH] mmc-utils: mmc_cmds: fix type-punned warning on &ext_csd[] casting Giulio Benetti 2024-03-05 14:52 ` Avri Altman 2024-03-05 15:01 ` Avri Altman 2024-03-05 18:45 ` [PATCH v2] " Giulio Benetti 2024-03-05 19:48 ` Avri Altman 2024-03-07 11:02 ` Ulf Hansson 2024-03-05 18:46 ` [PATCH] " Giulio Benetti
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox