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: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAHp75VebCZckUrNraYQj9k=Mrn2kbYs1Lx26f5-8rKJ3RXeh-w@mail.gmail.com>
Date:   Thu, 7 Dec 2023 20:24:45 +0200
From:   Andy Shevchenko <andy.shevchenko@...il.com>
To:     Hugo Villeneuve <hugo@...ovil.com>
Cc:     kernel test robot <lkp@...el.com>, gregkh@...uxfoundation.org,
        jirislaby@...nel.org, hvilleneuve@...onoff.com,
        oe-kbuild-all@...ts.linux.dev, linux-kernel@...r.kernel.org,
        linux-serial@...r.kernel.org, stable@...r.kernel.org
Subject: Re: [PATCH 1/7] serial: sc16is7xx: fix snprintf format specifier in sc16is7xx_regmap_name()

On Thu, Dec 7, 2023 at 7:52 PM Hugo Villeneuve <hugo@...ovil.com> wrote:
> On Wed, 6 Dec 2023 14:29:39 +0800
> kernel test robot <lkp@...el.com> wrote:

...

> >    drivers/tty/serial/sc16is7xx.c: In function 'sc16is7xx_i2c_probe':
> > >> drivers/tty/serial/sc16is7xx.c:1703:41: warning: '%u' directive output may be truncated writing between 1 and 10 bytes into a region of size 2 [-Wformat-truncation=]
> >     1703 |         snprintf(buf, sizeof(buf), "port%u", port_id);
> >          |                                         ^~
> >    In function 'sc16is7xx_regmap_name',
> >        inlined from 'sc16is7xx_i2c_probe' at drivers/tty/serial/sc16is7xx.c:1805:17:
> >    drivers/tty/serial/sc16is7xx.c:1703:36: note: directive argument in the range [0, 4294967294]
> >     1703 |         snprintf(buf, sizeof(buf), "port%u", port_id);
> >          |                                    ^~~~~~~~
> >    drivers/tty/serial/sc16is7xx.c:1703:9: note: 'snprintf' output between 6 and 15 bytes into a destination of size 6
> >     1703 |         snprintf(buf, sizeof(buf), "port%u", port_id);
> >          |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>
> Hi,
> the only solution I could find is to add this line just before snprintf:
>
>     BUG_ON(port_id > MAX310X_MAX_PORTS);
>
> it allows us to have the smallest buffer size possible.
>
> One other solution would be to change port_id from "unsigned int"
> to "u8", and increase the buffer by an additional 2 bytes to silence
> the warning, but then wasting 2 bytes for each channel, like so:

I didn't get this. It's a buffer that is rewritten on each port (why
is it even static?). Just make sure it's enough for any given number
and drop the static.

...

While at it, can you look at the following items to improve?
- sc16is7xx_alloc_line() can be updated to use IDA framework
- move return xxx; to the default cases in a few functions
- if (div > 0xffff) { --> if (div >= BIT(16)) { as it better shows why
the limit is that (we have only 16 bits for the divider)
- do {} while (0) in the sc16is7xx_port_irq, WTH?!
- while (1) { -- do { } while (keep_polling); in sc16is7xx_irq()
- use in_range() in sc16is7xx_setup_mctrl_ports() ? (maybe not, dunno)
- for (i--; i >= 0; i--) { --> while (i--) {
- use spi_get_device_match_data() and i2c_get_match_data()
- 15000000 --> 15 * HZ_PER_MHZ ?
- dropping MODULE_ALIAS (and fix the ID tables, _if_ needed)
- split the code to the core / main + SPI + I2C glue drivers

* These just come on the first glance at the code, perhaps there is
more room to improve.

--
With Best Regards,
Andy Shevchenko

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ