lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Sat, 28 Aug 2021 16:31:14 +0900 From: Vincent MAILHOL <mailhol.vincent@...adoo.fr> To: Kees Cook <keescook@...omium.org> Cc: Marc Kleine-Budde <mkl@...gutronix.de>, open list <linux-kernel@...r.kernel.org>, "Gustavo A. R. Silva" <gustavoars@...nel.org>, Arnd Bergmann <arnd@...db.de>, Ayush Sawal <ayush.sawal@...lsio.com>, Vinay Kumar Yadav <vinay.yadav@...lsio.com>, Rohit Maheshwari <rohitm@...lsio.com>, Herbert Xu <herbert@...dor.apana.org.au>, "David S. Miller" <davem@...emloft.net>, Kalle Valo <kvalo@...eaurora.org>, Jakub Kicinski <kuba@...nel.org>, Stanislaw Gruszka <stf_xl@...pl>, Luca Coelho <luciano.coelho@...el.com>, "James E.J. Bottomley" <jejb@...ux.ibm.com>, "Martin K. Petersen" <martin.petersen@...cle.com>, Alexei Starovoitov <ast@...nel.org>, Daniel Borkmann <daniel@...earbox.net>, Andrii Nakryiko <andrii@...nel.org>, Martin KaFai Lau <kafai@...com>, Song Liu <songliubraving@...com>, Yonghong Song <yhs@...com>, John Fastabend <john.fastabend@...il.com>, KP Singh <kpsingh@...nel.org>, Johannes Berg <johannes.berg@...el.com>, Mordechay Goodstein <mordechay.goodstein@...el.com>, Lee Jones <lee.jones@...aro.org>, Wolfgang Grandegger <wg@...ndegger.com>, Arunachalam Santhanam <arunachalam.santhanam@...bosch.com>, Mikulas Patocka <mikulas@...ax.karlin.mff.cuni.cz>, linux-crypto@...r.kernel.org, ath10k@...ts.infradead.org, linux-wireless@...r.kernel.org, netdev <netdev@...r.kernel.org>, linux-scsi@...r.kernel.org, linux-can <linux-can@...r.kernel.org>, bpf@...r.kernel.org, Rasmus Villemoes <linux@...musvillemoes.dk>, Keith Packard <keithp@...thp.com>, Dan Williams <dan.j.williams@...el.com>, Daniel Vetter <daniel.vetter@...ll.ch>, clang-built-linux@...glegroups.com, linux-hardening@...r.kernel.org Subject: Re: [PATCH v2 2/5] treewide: Replace open-coded flex arrays in unions Le sam. 28 août 2021 à 01:17, Kees Cook <keescook@...omium.org> a écrit : > > On Thu, Aug 26, 2021 at 08:24:52AM +0200, Marc Kleine-Budde wrote: > > [...] > > BTW: Is there opportunity for conversion, too? > > > > | drivers/net/can/peak_canfd/peak_pciefd_main.c:146:32: warning: array of flexible structures > > Untested potential solution: > > diff --git a/drivers/net/can/peak_canfd/peak_pciefd_main.c b/drivers/net/can/peak_canfd/peak_pciefd_main.c > index 1df3c4b54f03..efa2b5a52bd7 100644 > --- a/drivers/net/can/peak_canfd/peak_pciefd_main.c > +++ b/drivers/net/can/peak_canfd/peak_pciefd_main.c > @@ -143,7 +143,11 @@ struct pciefd_rx_dma { > __le32 irq_status; > __le32 sys_time_low; > __le32 sys_time_high; > - struct pucan_rx_msg msg[]; > + /* > + * with "msg" being pciefd_irq_rx_cnt(priv->irq_status)-many > + * variable-sized struct pucan_rx_msg following. > + */ > + __le32 msg[]; Isn't u8 msg[] preferable here? > } __packed __aligned(4); > > /* Tx Link record */ > @@ -327,7 +331,7 @@ static irqreturn_t pciefd_irq_handler(int irq, void *arg) > > /* handle rx messages (if any) */ > peak_canfd_handle_msgs_list(&priv->ucan, > - rx_dma->msg, > + (struct pucan_rx_msg *)rx_dma->msg, > pciefd_irq_rx_cnt(priv->irq_status)); > > /* handle tx link interrupt (if any) */ > > > It's not great, but it's also not strictly a flex array, in the sense > that since struct pucan_rx_msg is a variable size, the compiler cannot > reason about the size of struct pciefd_rx_dma based only on the > irq_status encoding... In the same spirit, it is a bit cleaner to change the prototype of handle_msgs_list(). Like that: diff --git a/drivers/net/can/peak_canfd/peak_canfd.c b/drivers/net/can/peak_canf d/peak_canfd.c index d08718e98e11..81a9faa6193f 100644 --- a/drivers/net/can/peak_canfd/peak_canfd.c +++ b/drivers/net/can/peak_canfd/peak_canfd.c @@ -484,9 +484,8 @@ int peak_canfd_handle_msg(struct peak_canfd_priv *priv, /* handle a list of rx_count messages from rx_msg memory address */ int peak_canfd_handle_msgs_list(struct peak_canfd_priv *priv, - struct pucan_rx_msg *msg_list, int msg_count) + void *msg_ptr, int msg_count) { - void *msg_ptr = msg_list; int i, msg_size = 0; for (i = 0; i < msg_count; i++) { diff --git a/drivers/net/can/peak_canfd/peak_canfd_user.h b/drivers/net/can/peak _canfd/peak_canfd_user.h index a72719dc3b74..ef91f92e70c3 100644 --- a/drivers/net/can/peak_canfd/peak_canfd_user.h +++ b/drivers/net/can/peak_canfd/peak_canfd_user.h @@ -42,5 +42,5 @@ struct net_device *alloc_peak_canfd_dev(int sizeof_priv, int index, int peak_canfd_handle_msg(struct peak_canfd_priv *priv, struct pucan_rx_msg *msg); int peak_canfd_handle_msgs_list(struct peak_canfd_priv *priv, - struct pucan_rx_msg *rx_msg, int rx_count); + void *msg_ptr, int rx_count); #endif diff --git a/drivers/net/can/peak_canfd/peak_pciefd_main.c b/drivers/net/can/peak_canfd/peak_pciefd_main.c index 1df3c4b54f03..c1de1e3dc4bc 100644 --- a/drivers/net/can/peak_canfd/peak_pciefd_main.c +++ b/drivers/net/can/peak_canfd/peak_pciefd_main.c @@ -143,7 +143,11 @@ struct pciefd_rx_dma { __le32 irq_status; __le32 sys_time_low; __le32 sys_time_high; - struct pucan_rx_msg msg[]; + /* + * with "msg" being pciefd_irq_rx_cnt(priv->irq_status)-many + * variable-sized struct pucan_rx_msg following. + */ + u8 msg[]; } __packed __aligned(4); /* Tx Link record */ Another solution would be to declare a maximum length for struct pucan_rx_msg::d. Because these are CAN FD messages, I suppose that maximum length would be CANFD_MAX_DLEN. struct canfd_frame from the UAPI uses the same pattern. N.B. This solution is not exclusive from the above one (actually, I think that using both would be the best solution). diff --git a/include/linux/can/dev/peak_canfd.h b/include/linux/can/dev/peak_canfd.h index f38772fd0c07..a048359db430 100644 --- a/include/linux/can/dev/peak_canfd.h +++ b/include/linux/can/dev/peak_canfd.h @@ -189,7 +189,7 @@ struct __packed pucan_rx_msg { u8 client; __le16 flags; __le32 can_id; - u8 d[]; + u8 d[CANFD_MAX_DLEN]; }; /* uCAN error types */ I only tested for compilation. Yours sincerely, Vincent
Powered by blists - more mailing lists