From: Paulo Miguel Almeida <paulo.miguel.almeida.rodenas@gmail.com>
To: "Arnd Bergmann" <arnd@arndb.de>,
"Greg Kroah-Hartman" <gregkh@linuxfoundation.org>,
"Ilpo Järvinen" <ilpo.jarvinen@linux.intel.com>,
"Andy Shevchenko" <andy.shevchenko@gmail.com>,
"Jiri Slaby" <jirislaby@kernel.org>,
"Haowen Bai" <baihaowen@meizu.com>
Cc: linux-kernel@vger.kernel.org, linux-hardening@vger.kernel.org,
paulo.miguel.almeida.rodenas@gmail.com
Subject: [PATCH v3] [next] pcmcia: synclink_cs: replace 1-element array with flex-array member
Date: Sat, 17 Dec 2022 11:59:06 +1300 [thread overview]
Message-ID: <Y5z4Og3XmCGQwTO9@mail.google.com> (raw)
In-Reply-To: <Y5uN9Rr3v1uWH765@mail.google.com>
One-element arrays are deprecated, and we are replacing them with
flexible array members instead. So, replace one-element array with
flexible-array member in struct RXBUF and refactor the rest of the code
accordingly. While at it, fix an edge case which could cause
rx_buf_count to be 0 when max_frame_size was set to the maximum
allowed value (65535).
It's worth mentioning that struct RXBUF was allocating 1 byte "too much"
for what is required (ignoring bytes added by padding).
This helps with the ongoing efforts to tighten the FORTIFY_SOURCE
routines on memcpy() and help us make progress towards globally
enabling -fstrict-flex-arrays=3 [1].
Link: https://github.com/KSPP/linux/issues/79
Link: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101836 [1]
Signed-off-by: Paulo Miguel Almeida <paulo.miguel.almeida.rodenas@gmail.com>
---
Changelog:
- v3:
fix size calculation mistakes using overflow.h macros: (Req: Andy
Shevchenko, Kees Cook)
add notes struct RXBUF size (Kees Cook)
- v2: removed changes to how the size of RXBUF was calculated. I
changed my mind after thinking about the existing padding in the
struct. Happy to discuss it if anyone sees it differently.
- v1: https://lore.kernel.org/lkml/Y5mMWEtHWKOiPVU+@mail.google.com/
---
drivers/char/pcmcia/synclink_cs.c | 33 +++++++++++++++++++------------
1 file changed, 20 insertions(+), 13 deletions(-)
diff --git a/drivers/char/pcmcia/synclink_cs.c b/drivers/char/pcmcia/synclink_cs.c
index b2735be81ab2..eee6772a0978 100644
--- a/drivers/char/pcmcia/synclink_cs.c
+++ b/drivers/char/pcmcia/synclink_cs.c
@@ -105,7 +105,7 @@ static MGSL_PARAMS default_params = {
typedef struct {
int count;
unsigned char status;
- char data[1];
+ char data[];
} RXBUF;
/* The queue of BH actions to be performed */
@@ -229,12 +229,18 @@ typedef struct _mgslpc_info {
} MGSLPC_INFO;
#define MGSLPC_MAGIC 0x5402
+#define MGSLPC_MAX_FRAME_SIZE 65535
+#define MGSLPC_MIN_FRAME_SIZE 4096
/*
* The size of the serial xmit buffer is 1 page, or 4096 bytes
*/
#define TXBUFSIZE 4096
+/*
+ * RXBUF accommodates at least 1 buffer (header+data) of MGSLPC_MAX_FRAME_SIZE
+ */
+#define RXBUF_MAX_SIZE (sizeof(RXBUF) + MGSLPC_MAX_FRAME_SIZE)
#define CHA 0x00 /* channel A offset */
#define CHB 0x40 /* channel B offset */
@@ -529,7 +535,7 @@ static int mgslpc_probe(struct pcmcia_device *link)
tty_port_init(&info->port);
info->port.ops = &mgslpc_port_ops;
INIT_WORK(&info->task, bh_handler);
- info->max_frame_size = 4096;
+ info->max_frame_size = MGSLPC_MIN_FRAME_SIZE;
init_waitqueue_head(&info->status_event_wait_q);
init_waitqueue_head(&info->event_wait_q);
spin_lock_init(&info->lock);
@@ -2611,19 +2617,20 @@ static int mgslpc_proc_show(struct seq_file *m, void *v)
static int rx_alloc_buffers(MGSLPC_INFO *info)
{
/* each buffer has header and data */
- info->rx_buf_size = sizeof(RXBUF) + info->max_frame_size;
+ if (check_add_overflow(sizeof(RXBUF), info->max_frame_size, &info->rx_buf_size))
+ return -EINVAL;
- /* calculate total allocation size for 8 buffers */
- info->rx_buf_total_size = info->rx_buf_size * 8;
+ /* try to alloc as many buffers that can fit within RXBUF_MAX_SIZE (up to 8) */
+ if (check_mul_overflow(info->rx_buf_size, 8, &info->rx_buf_total_size))
+ return -EINVAL;
- /* limit total allocated memory */
- if (info->rx_buf_total_size > 0x10000)
- info->rx_buf_total_size = 0x10000;
+ if (info->rx_buf_total_size > RXBUF_MAX_SIZE)
+ info->rx_buf_total_size = RXBUF_MAX_SIZE;
/* calculate number of buffers */
info->rx_buf_count = info->rx_buf_total_size / info->rx_buf_size;
- info->rx_buf = kmalloc(info->rx_buf_total_size, GFP_KERNEL);
+ info->rx_buf = kcalloc(info->rx_buf_count, info->rx_buf_size, GFP_KERNEL);
if (info->rx_buf == NULL)
return -ENOMEM;
@@ -2695,10 +2702,10 @@ static int mgslpc_add_device(MGSLPC_INFO *info)
current_dev->next_device = info;
}
- if (info->max_frame_size < 4096)
- info->max_frame_size = 4096;
- else if (info->max_frame_size > 65535)
- info->max_frame_size = 65535;
+ if (info->max_frame_size < MGSLPC_MIN_FRAME_SIZE)
+ info->max_frame_size = MGSLPC_MIN_FRAME_SIZE;
+ else if (info->max_frame_size > MGSLPC_MAX_FRAME_SIZE)
+ info->max_frame_size = MGSLPC_MAX_FRAME_SIZE;
printk("SyncLink PC Card %s:IO=%04X IRQ=%d\n",
info->device_name, info->io_base, info->irq_level);
--
2.38.1
next prev parent reply other threads:[~2022-12-16 22:59 UTC|newest]
Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-12-14 8:42 [PATCH] [next] pcmcia: synclink_cs: replace 1-element array with flex-array member Paulo Miguel Almeida
2022-12-14 8:58 ` [PATCH v2] " Paulo Miguel Almeida
2022-12-14 10:43 ` Andy Shevchenko
2022-12-14 20:19 ` Paulo Miguel Almeida
2022-12-14 19:29 ` [PATCH] " Kees Cook
2022-12-14 20:09 ` Paulo Miguel Almeida
2022-12-14 20:26 ` Kees Cook
2022-12-14 20:39 ` Andy Shevchenko
2022-12-14 21:49 ` Kees Cook
2022-12-14 22:06 ` Andy Shevchenko
2022-12-15 4:29 ` Paulo Miguel Almeida
2022-12-15 6:35 ` Paulo Miguel Almeida
2022-12-15 8:57 ` Andy Shevchenko
2022-12-15 21:13 ` Paulo Miguel Almeida
2022-12-16 22:59 ` Paulo Miguel Almeida [this message]
2022-12-16 23:42 ` [PATCH v3] " Kees Cook
2022-12-17 0:11 ` Paulo Miguel Almeida
2022-12-17 11:43 ` Andy Shevchenko
2022-12-17 20:05 ` Paulo Miguel Almeida
2022-12-14 20:14 ` [PATCH] " Paulo Miguel Almeida
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=Y5z4Og3XmCGQwTO9@mail.google.com \
--to=paulo.miguel.almeida.rodenas@gmail.com \
--cc=andy.shevchenko@gmail.com \
--cc=arnd@arndb.de \
--cc=baihaowen@meizu.com \
--cc=gregkh@linuxfoundation.org \
--cc=ilpo.jarvinen@linux.intel.com \
--cc=jirislaby@kernel.org \
--cc=linux-hardening@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.