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

[Saravana Kannan]

On Tue, Jul 9, 2024 at 7:13 AM Zijun Hu <zijun_hu@xxxxxxxxxx> wrote:
>
> From: Zijun Hu <quic_zijuhu@xxxxxxxxxxx>
>
> devlink_(add|remove)_symlinks() kzalloc() memory to save symlink name
> for both supplier and consumer, but do not explicitly take into account
> consumer's prefix "consumer:", so cause wrong algorithm for calculating
> memory size, fixed by taking into account consumer's prefix as well.

Again, this makes it sound like you are fixing some bug. No "wrong
algorithm" is happening here. You are improving readability. So, just
say that?

-Saravana

>
> Signed-off-by: Zijun Hu <quic_zijuhu@xxxxxxxxxxx>
> ---
> Changes in v2:
> - Correct commit message and add inline comments
> - Remove fix tag
> - Link to v1: https://lore.kernel.org/r/20240707-devlink_fix-v1-1-623acb431cd8@xxxxxxxxxxx
> ---
>  drivers/base/core.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/base/core.c b/drivers/base/core.c
> index 2b4c0624b704..51209db7ff84 100644
> --- a/drivers/base/core.c
> +++ b/drivers/base/core.c
> @@ -572,7 +572,11 @@ 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;
> +       /*
> +        * we kzalloc() memory for symlink name of both supplier and
> +        * consumer, so explicitly take into account both prefix.
> +        */
> +       len += max(strlen("supplier:"), strlen("consumer:")) + 1;
>         buf = kzalloc(len, GFP_KERNEL);
>         if (!buf)
>                 return -ENOMEM;
> @@ -623,7 +627,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>
>