On Wed, Mar 24, 2021 at 2:29 PM Rasmus Villemoes
<li...@rasmusvillemoes.dk> wrote:
> On 24/03/2021 14.07, Arnd Bergmann wrote:
>
> > if (set_settings & HILINK_LINK_SET_SPEED) {
> > speed_level = hinic_ethtool_to_hw_speed_level(speed);
> > err = snprintf(set_link_str, SET_LINK_STR_MAX_LEN,
> > - "%sspeed %d ", set_link_str, speed);
> > - if (err <= 0 || err >= SET_LINK_STR_MAX_LEN) {
> > + "speed %d ", speed);
> > + if (err >= SET_LINK_STR_MAX_LEN) {
> > netif_err(nic_dev, drv, netdev, "Failed to snprintf
> > link speed, function return(%d) and dest_len(%d)\n",
> > err, SET_LINK_STR_MAX_LEN);
> > return -EFAULT;
>
> It's not your invention of course, but this both seems needlessly harsh
> and EFAULT is a weird error to return. It's just a printk() message that
> might be truncated, and now that the format string only has a %d
> specifier, it can actually be verified statically that overflow will
> never happen (though I don't know or think gcc can do that, perhaps
> there's some locale nonsense in the standard that allows using
> utf16-encoded sanskrit runes). So probably that test should just be
> dropped, but that's a separate thing.
I thought about fixing it, but this seemed to be a rabbit hole I didn't
want to get into, as there are other harmless issues in the driver
that could be improved.
I'm fairly sure gcc can indeed warn about the overflow with
-Wformat-truncation, but the warning is disabled at the moment
because it has a ton of false positives:
kernel/workqueue.c: In function 'create_worker':
kernel/workqueue.c:1933:55: error: '%d' directive output may be
truncated writing between 1 and 10 bytes into a region of size between
3 and 13 [-Werror=format-truncation=]
1933 | snprintf(id_buf, sizeof(id_buf), "u%d:%d",
pool->id, id);
Arnd
