linux-kernel - Re: [RFC PATCH 1/2] tty: serial/8250: Fix build warning in serial8250_probe

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

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <f765eae4-f83e-4c25-9697-2705afd7b9b8@app.fastmail.com>
Date: Wed, 06 Aug 2025 09:01:46 +0200
From: "Arnd Bergmann" <arnd@...db.de>
To: "Abinash Singh" <abinashsinghlalotra@...il.com>,
 "Greg Kroah-Hartman" <gregkh@...uxfoundation.org>
Cc: "Jiri Slaby" <jirislaby@...nel.org>,
 "Andy Shevchenko" <andriy.shevchenko@...ux.intel.com>,
 "Sunil V L" <sunilvl@...tanamicro.com>,
 Uwe Kleine-König <u.kleine-koenig@...libre.com>,
 linux-kernel@...r.kernel.org, linux-serial@...r.kernel.org
Subject: Re: [RFC PATCH 1/2] tty: serial/8250: Fix build warning in
 serial8250_probe_acpi()

On Tue, Aug 5, 2025, at 21:51, Abinash Singh wrote:
> The function serial8250_probe_acpi() in 8250_platform.c triggered a
>     frame size warning:
> drivers/tty/serial/8250/8250_platform.c: In function 
> ‘serial8250_probe_acpi’:
> drivers/tty/serial/8250/8250_platform.c:152:1: warning: the frame size 
> of 1160 bytes is larger than 1024 bytes [-Wframe-larger-than=]

Hi Abinash,

I've seen this one as well in some configurations, thanks
for helping out addressing it!

> This patch reduces the stack usage by dynamically allocating the
>     `uart` structure using kmalloc(), rather than placing it on
>     the stack. This eliminates the overflow warning and improves kernel
>     robustness.

The same problem does show up in a lot of 8250 driver variants,
plus a couple that have it in theory but don't run into the
1024 byte limit:

drivers/char/mwave/mwavedd.c-static int register_serial_portandirq(unsigned int port, int irq)
drivers/char/mwave/mwavedd.c-{
drivers/char/mwave/mwavedd.c:   struct uart_8250_port uart;

drivers/ptp/ptp_ocp.c-ptp_ocp_serial_line(struct ptp_ocp *bp, struct ocp_resource *r)
drivers/ptp/ptp_ocp.c-{
drivers/ptp/ptp_ocp.c-  struct pci_dev *pdev = bp->pdev;
drivers/ptp/ptp_ocp.c:  struct uart_8250_port uart;

drivers/tty/serial/8250/8250_acorn.c-serial_card_probe(struct expansion_card *ec, const struct ecard_id *id)
drivers/tty/serial/8250/8250_acorn.c-{
drivers/tty/serial/8250/8250_acorn.c:   struct uart_8250_port uart;

drivers/tty/serial/8250/8250_bcm2835aux.c-static int bcm2835aux_serial_probe(struct platform_device *pdev)
drivers/tty/serial/8250/8250_bcm2835aux.c-{
drivers/tty/serial/8250/8250_bcm2835aux.c:      struct uart_8250_port up = { };

In total, I see 34 drivers using this exact pattern for 
their probe function, and ideally we would to to fix them
all to do it some other way.

Note how serial8250_register_8250_port() ands up just copying
individual members of the passed uart_8250_port structure into
the global array of the same type, so one way of addressing
this would be to use a structure for initialization that only
contains a subset of the uart_8250_port members and can still
be allocated on the stack, or possibly be constant.

There is already a 'struct plat_serial8250_port', which some
(mostly ancient) drivers use to register a 8250 compatible
uart through the 8250_platform driver. That driver has
a number of problems, so I don't really want to expand its
usage, but it may be possible to use a single structure
for both purposes.

      Arnd