linux-kernel - Re: [PATCH v1 27/31] serial: 8250: Allow dynamic extension of uart_port attr

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: <CAHp75Vd9+hE9y5JEm2_5WnrK1R2dySikYUdVn_J=e5bX=ZhV0Q@mail.gmail.com>
Date: Mon, 1 Dec 2025 04:30:06 +0200
From: Andy Shevchenko <andy.shevchenko@...il.com>
To: Crescent Hsieh <crescentcy.hsieh@...a.com>
Cc: gregkh@...uxfoundation.org, jirislaby@...nel.org, 
	ilpo.jarvinen@...ux.intel.com, linux-kernel@...r.kernel.org, 
	linux-serial@...r.kernel.org
Subject: Re: [PATCH v1 27/31] serial: 8250: Allow dynamic extension of
 uart_port attr_group

On Sun, Nov 30, 2025 at 12:45 PM Crescent Hsieh
<crescentcy.hsieh@...a.com> wrote:
>
> Currently, uart_port->attr_group can only reference a statically
> defined attribute_group, which prevents drivers from appending their
> own sysfs attributes.
>
> This enables drivers to inject custom sysfs entries without overriding
> the core-provided rxtrig interface.


> +       if (up->port.attr_group) {
> +               upp_attrs = up->port.attr_group->attrs;
> +
> +               while (upp_attrs[upp_attr_num])
> +                       upp_attr_num++;

> +               up->port.attr_group = kcalloc(1, sizeof(struct attribute_group), GFP_KERNEL);

sizeof(*...)

No error check? And why calloc(1)? Shouldn't malloc() be enough?

> +               up->port.attr_group->attrs = kcalloc(upp_attr_num + 2, sizeof(struct attribute *), GFP_KERNEL);

sizeof(*)
Error check?

> +               for (i = 0; i < upp_attr_num; ++i)
> +                       up->port.attr_group->attrs[i] = upp_attrs[i];
> +
> +               if (conf_type->rxtrig_bytes[0])
> +                       up->port.attr_group->attrs[upp_attr_num] = &dev_attr_rx_trig_bytes.attr;
> +
> +               up->port.attr_group_allocated = true;
> +       } else {
> +               if (conf_type->rxtrig_bytes[0])
> +                       up->port.attr_group = &serial8250_dev_attr_group;
> +       }
>  }

...

> struct uart_port {

>         unsigned char           console_reinit;
>         const char              *name;                  /* port name */
>         struct attribute_group  *attr_group;            /* port specific attributes */
> +       bool                    attr_group_allocated;   /* whether attr_group is dynamic allocated */

Is this the best place? Have you run `pahole`?

>         const struct attribute_group **tty_groups;      /* all attributes (serial core use only) */
>         struct serial_rs485     rs485;
>         struct serial_rs485     rs485_supported;        /* Supported mask for serial_rs485 */

-- 
With Best Regards,
Andy Shevchenko