Re: [PATCH] PM: EM: Fix use-after-free of perf domain in netlink doit handlers

[Rafael J. Wysocki (Intel)]

On Tue, Sep 22, 2026 at 10:12 AM Fan Wu <fanwu01@xxxxxxxxxx> wrote:
>
> The doit handlers dropped em_pd_list_mutex before dereferencing the
> perf domain, so a concurrent device unregister could free it while the
> reply was being built.
>
> Encode the reply from em_perf_domain_for_id() callbacks, which run
> under the mutex, and allocate the skb outside of it, like the dumpit
> path already does.
>
> This issue was found by an in-house static analysis tool.
>
> Fixes: 380ff27af25e ("PM: EM: Add dump to get-perf-domains in the EM YNL spec")
> Fixes: f2d2946eaa5c ("PM: EM: Implement em_nl_get_pd_table_doit()")
> Cc: stable@xxxxxxxxxxxxxxx
> Cc: Lukasz Luba <lukasz.luba@xxxxxxx>
> Assisted-by: Codex:gpt-5.6
> Co-developed-by: Song Li <songl@xxxxxxxxxx>
> Signed-off-by: Song Li <songl@xxxxxxxxxx>
> Signed-off-by: Fan Wu <fanwu01@xxxxxxxxxx>
> ---
>  kernel/power/em_netlink.c   | 122 +++++++++++++++++++++++-------------
>  kernel/power/em_netlink.h   |   8 ++-
>  kernel/power/energy_model.c |   8 ++-
>  3 files changed, 87 insertions(+), 51 deletions(-)
>
> diff --git a/kernel/power/em_netlink.c b/kernel/power/em_netlink.c
> index 4d4fd29bd2be..1f68d456a713 100644
> --- a/kernel/power/em_netlink.c
> +++ b/kernel/power/em_netlink.c
> @@ -95,41 +95,62 @@ static int __em_nl_get_pd_for_dump(struct em_perf_domain *pd, void *data)
>         return ret;
>  }
>
> +struct em_nl_doit_ctx {
> +       struct genl_info *info;
> +       int cmd;
> +       struct sk_buff *msg;
> +};
> +
> +static int __em_nl_get_pd_doit_fill(struct em_perf_domain *pd, void *data)
> +{
> +       struct em_nl_doit_ctx *ctx = data;
> +       void *hdr;
> +
> +       hdr = genlmsg_put_reply(ctx->msg, ctx->info, &dev_energymodel_nl_family, 0,
> +                               ctx->cmd);
> +       if (!hdr)
> +               return -EMSGSIZE;
> +
> +       if (__em_nl_get_pd(pd, ctx->msg)) {
> +               genlmsg_cancel(ctx->msg, hdr);
> +               return -EMSGSIZE;
> +       }
> +
> +       genlmsg_end(ctx->msg, hdr);
> +       return 0;
> +}
> +
>  int dev_energymodel_nl_get_perf_domains_doit(struct sk_buff *skb,
> -                                             struct genl_info *info)
> +                                            struct genl_info *info)
>  {
> -       int id, ret = -EMSGSIZE, msg_sz = 0;
> -       int cmd = info->genlhdr->cmd;
> -       struct em_perf_domain *pd;
> +       struct em_nl_doit_ctx ctx = {
> +               .info = info,
> +               .cmd = info->genlhdr->cmd,
> +       };
>         struct sk_buff *msg;
> -       void *hdr;
> +       int id, ret, msg_sz = 0;
>
>         if (!info->attrs[DEV_ENERGYMODEL_A_PERF_DOMAIN_PERF_DOMAIN_ID])
>                 return -EINVAL;
>
>         id = nla_get_u32(info->attrs[DEV_ENERGYMODEL_A_PERF_DOMAIN_PERF_DOMAIN_ID]);
> -       pd = em_perf_domain_get_by_id(id);
> -       if (!pd)
> -               return -EINVAL;
>
> -       __em_nl_get_pd_size(pd, &msg_sz);
> +       /* Encode under em_pd_list_mutex, like the dumpit path. */
> +       ret = em_perf_domain_for_id(id, __em_nl_get_pd_size, &msg_sz);
> +       if (ret)
> +               return ret;
> +
>         msg = genlmsg_new(msg_sz, GFP_KERNEL);
>         if (!msg)
>                 return -ENOMEM;
>
> -       hdr = genlmsg_put_reply(msg, info, &dev_energymodel_nl_family, 0, cmd);
> -       if (!hdr)
> -               goto out_free_msg;
> -
> -       ret = __em_nl_get_pd(pd, msg);
> +       ctx.msg = msg;
> +       ret = em_perf_domain_for_id(id, __em_nl_get_pd_doit_fill, &ctx);
>         if (ret)
> -               goto out_cancel_msg;
> -       genlmsg_end(msg, hdr);
> +               goto out_free_msg;
>
>         return genlmsg_reply(msg, info);
>
> -out_cancel_msg:
> -       genlmsg_cancel(msg, hdr);
>  out_free_msg:
>         nlmsg_free(msg);
>         return ret;
> @@ -148,19 +169,6 @@ int dev_energymodel_nl_get_perf_domains_dumpit(struct sk_buff *skb,
>         return for_each_em_perf_domain(__em_nl_get_pd_for_dump, &ctx);
>  }
>
> -static struct em_perf_domain *__em_nl_get_pd_table_id(struct nlattr **attrs)
> -{
> -       struct em_perf_domain *pd;
> -       int id;
> -
> -       if (!attrs[DEV_ENERGYMODEL_A_PERF_TABLE_PERF_DOMAIN_ID])
> -               return NULL;
> -
> -       id = nla_get_u32(attrs[DEV_ENERGYMODEL_A_PERF_TABLE_PERF_DOMAIN_ID]);
> -       pd = em_perf_domain_get_by_id(id);
> -       return pd;
> -}
> -
>  static int __em_nl_get_pd_table_size(const struct em_perf_domain *pd)
>  {
>         int id_sz, ps_sz;
> @@ -245,34 +253,58 @@ int __em_nl_get_pd_table(struct sk_buff *msg, const struct em_perf_domain *pd)
>         return -EMSGSIZE;
>  }
>
> +static int __em_nl_get_pd_table_size_cb(struct em_perf_domain *pd, void *data)
> +{
> +       *(int *)data = __em_nl_get_pd_table_size(pd);
> +       return 0;
> +}
> +
> +static int __em_nl_get_pd_table_doit_fill(struct em_perf_domain *pd,
> +                                         void *data)
> +{
> +       struct em_nl_doit_ctx *ctx = data;
> +       void *hdr;
> +
> +       hdr = genlmsg_put_reply(ctx->msg, ctx->info, &dev_energymodel_nl_family, 0,
> +                               ctx->cmd);
> +       if (!hdr)
> +               return -EMSGSIZE;
> +
> +       if (__em_nl_get_pd_table(ctx->msg, pd))
> +               return -EMSGSIZE;
> +
> +       genlmsg_end(ctx->msg, hdr);
> +       return 0;
> +}
> +
>  int dev_energymodel_nl_get_perf_table_doit(struct sk_buff *skb,
> -                                           struct genl_info *info)
> +                                          struct genl_info *info)
>  {
> -       int cmd = info->genlhdr->cmd;
> -       int msg_sz, ret = -EMSGSIZE;
> -       struct em_perf_domain *pd;
> +       struct em_nl_doit_ctx ctx = {
> +               .info = info,
> +               .cmd = info->genlhdr->cmd,
> +       };
>         struct sk_buff *msg;
> -       void *hdr;
> +       int id, ret, msg_sz;
>
> -       pd = __em_nl_get_pd_table_id(info->attrs);
> -       if (!pd)
> +       if (!info->attrs[DEV_ENERGYMODEL_A_PERF_TABLE_PERF_DOMAIN_ID])
>                 return -EINVAL;
>
> -       msg_sz = __em_nl_get_pd_table_size(pd);
> +       id = nla_get_u32(info->attrs[DEV_ENERGYMODEL_A_PERF_TABLE_PERF_DOMAIN_ID]);
> +
> +       ret = em_perf_domain_for_id(id, __em_nl_get_pd_table_size_cb, &msg_sz);
> +       if (ret)
> +               return ret;
>
>         msg = genlmsg_new(msg_sz, GFP_KERNEL);
>         if (!msg)
>                 return -ENOMEM;
>
> -       hdr = genlmsg_put_reply(msg, info, &dev_energymodel_nl_family, 0, cmd);
> -       if (!hdr)
> -               goto out_free_msg;
> -
> -       ret = __em_nl_get_pd_table(msg, pd);
> +       ctx.msg = msg;
> +       ret = em_perf_domain_for_id(id, __em_nl_get_pd_table_doit_fill, &ctx);
>         if (ret)
>                 goto out_free_msg;
>
> -       genlmsg_end(msg, hdr);
>         return genlmsg_reply(msg, info);
>
>  out_free_msg:
> diff --git a/kernel/power/em_netlink.h b/kernel/power/em_netlink.h
> index 583d7f1c3939..bc98a3c278b9 100644
> --- a/kernel/power/em_netlink.h
> +++ b/kernel/power/em_netlink.h
> @@ -12,7 +12,8 @@
>  #if defined(CONFIG_ENERGY_MODEL) && defined(CONFIG_NET)
>  int for_each_em_perf_domain(int (*cb)(struct em_perf_domain*, void *),
>                             void *data);
> -struct em_perf_domain *em_perf_domain_get_by_id(int id);
> +int em_perf_domain_for_id(int id, int (*cb)(struct em_perf_domain *, void *),
> +                         void *data);
>  void em_notify_pd_created(const struct em_perf_domain *pd);
>  void em_notify_pd_deleted(const struct em_perf_domain *pd);
>  void em_notify_pd_updated(const struct em_perf_domain *pd);
> @@ -24,9 +25,10 @@ int for_each_em_perf_domain(int (*cb)(struct em_perf_domain*, void *),
>         return -EINVAL;
>  }
>  static inline
> -struct em_perf_domain *em_perf_domain_get_by_id(int id)
> +int em_perf_domain_for_id(int id, int (*cb)(struct em_perf_domain *, void *),
> +                         void *data)
>  {
> -       return NULL;
> +       return -EINVAL;
>  }
>
>  static inline void em_notify_pd_created(const struct em_perf_domain *pd) {}
> diff --git a/kernel/power/energy_model.c b/kernel/power/energy_model.c
> index e610cf8e9a06..a76089e2ce8c 100644
> --- a/kernel/power/energy_model.c
> +++ b/kernel/power/energy_model.c
> @@ -1031,7 +1031,9 @@ int for_each_em_perf_domain(int (*cb)(struct em_perf_domain*, void *),
>         return 0;
>  }
>
> -struct em_perf_domain *em_perf_domain_get_by_id(int id)
> +/* Run @cb on the matching domain with em_pd_list_mutex held. */
> +int em_perf_domain_for_id(int id, int (*cb)(struct em_perf_domain *, void *),
> +                         void *data)
>  {
>         struct em_perf_domain *pd;
>
> @@ -1040,9 +1042,9 @@ struct em_perf_domain *em_perf_domain_get_by_id(int id)
>
>         list_for_each_entry(pd, &em_pd_list, node) {
>                 if (pd->id == id)
> -                       return pd;
> +                       return cb(pd, data);
>         }
>
> -       return NULL;
> +       return -EINVAL;
>  }
>  #endif

Applied as 7.4 material, thanks!