Re: [PATCH] driver core: Fix size calculation of symlink name for devlink_(add|remove)_symlinks()

[Saravana Kannan]

On Sun, Jul 7, 2024 at 6:24 AM Zijun Hu <zijun_hu@xxxxxxxxxx> wrote:
>
> From: Zijun Hu <quic_zijuhu@xxxxxxxxxxx>
>
> devlink_(add|remove)_symlinks() wants to kzalloc() memory to save symlink
> name for either supplier or consumer, but forget to consider consumer
> prefix when calclulate memory size, fixed by considering prefix for both
> supplier and consumer.

No, I didn't forget to take "consumer" into account :) Both supplier
and consumer are the same length. So I didn't bother doing both. I
don't see a point behind this patch.

>
> Fixes: 287905e68dd2 ("driver core: Expose device link details in sysfs")

It's definitely not "Fixing" anything because nothing is broken.

Nack.

If you really want this in, remove this tag and send it again. I won't
ack or review it though as I don't think it adds much value. Greg can
take it if he thinks he likes it.

-Saravana

> Signed-off-by: Zijun Hu <quic_zijuhu@xxxxxxxxxxx>
> ---
>  drivers/base/core.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/base/core.c b/drivers/base/core.c
> index 2b4c0624b704..f14cfe5c97b7 100644
> --- a/drivers/base/core.c
> +++ b/drivers/base/core.c
> @@ -572,7 +572,7 @@ static int devlink_add_symlinks(struct device *dev)
>         len = max(strlen(dev_bus_name(sup)) + strlen(dev_name(sup)),
>                   strlen(dev_bus_name(con)) + strlen(dev_name(con)));
>         len += strlen(":");
> -       len += strlen("supplier:") + 1;
> +       len += max(strlen("supplier:"), strlen("consumer:")) + 1;
>         buf = kzalloc(len, GFP_KERNEL);
>         if (!buf)
>                 return -ENOMEM;
> @@ -623,7 +623,7 @@ static void devlink_remove_symlinks(struct device *dev)
>         len = max(strlen(dev_bus_name(sup)) + strlen(dev_name(sup)),
>                   strlen(dev_bus_name(con)) + strlen(dev_name(con)));
>         len += strlen(":");
> -       len += strlen("supplier:") + 1;
> +       len += max(strlen("supplier:"), strlen("consumer:")) + 1;
>         buf = kzalloc(len, GFP_KERNEL);
>         if (!buf) {
>                 WARN(1, "Unable to properly free device link symlinks!\n");
>
> ---
> base-commit: c6653f49e4fd3b0d52c12a1fc814d6c5b234ea15
> change-id: 20240707-devlink_fix-0fa46dedfe95
>
> Best regards,
> --
> Zijun Hu <quic_zijuhu@xxxxxxxxxxx>
>