mbox series

[v3,0/3] mmc: sdio: fixes some leaks

Message ID 20221109025142.1565445-1-yangyingliang@huawei.com
Headers show
Series mmc: sdio: fixes some leaks | expand

Message

Yang Yingliang Nov. 9, 2022, 2:51 a.m. UTC
This patch a refcount leak and two memory leaks about
SDIO function.

v2 -> v3:
 Change to call of_node_put() in remove() function to
 fix node refcount leak.

v1 -> v2:
  Fix compile error in patch #2.

Yang Yingliang (3):
  mmc: sdio: fix possible memory leak in sdio_init_func()
  mmc: sdio: fix of node refcount leak in sdio_add_func()
  mmc: sdio: fix possible memory leak in mmc_attach_sdio()

 drivers/mmc/core/sdio.c     | 7 ++-----
 drivers/mmc/core/sdio_bus.c | 6 +++---
 2 files changed, 5 insertions(+), 8 deletions(-)

Comments

Ulf Hansson Nov. 9, 2022, 2:38 p.m. UTC | #1
On Wed, 9 Nov 2022 at 14:23, Yang Yingliang <yangyingliang@huawei.com> wrote:
>
>
> On 2022/11/9 20:27, Ulf Hansson wrote:
> > On Wed, 9 Nov 2022 at 03:53, Yang Yingliang <yangyingliang@huawei.com> wrote:
> >> If device_add() returns error in sdio_add_func(), sdio function is not
> >> presented, so the node refcount that hold in sdio_set_of_node() can not
> >> be put in sdio_remove_func() which is called from error path. Fix this
> >> by moving of_node_put() before present check in remove() function.
> >>
> >> Fixes: 25185f3f31c9 ("mmc: Add SDIO function devicetree subnode parsing")
> >> Signed-off-by: Yang Yingliang <yangyingliang@huawei.com>
> >> ---
> >>   drivers/mmc/core/sdio_bus.c | 2 +-
> >>   1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/mmc/core/sdio_bus.c b/drivers/mmc/core/sdio_bus.c
> >> index babf21a0adeb..266639504a94 100644
> >> --- a/drivers/mmc/core/sdio_bus.c
> >> +++ b/drivers/mmc/core/sdio_bus.c
> >> @@ -377,11 +377,11 @@ int sdio_add_func(struct sdio_func *func)
> >>    */
> >>   void sdio_remove_func(struct sdio_func *func)
> >>   {
> >> +       of_node_put(func->dev.of_node);
> >>          if (!sdio_func_present(func))
> >>                  return;
> >>
> >>          device_del(&func->dev);
> >> -       of_node_put(func->dev.of_node);
> >>          put_device(&func->dev);
> > Seems like we should call put_device() even if sdio_func_present()
> > returns false, don't you think?
> >
> > In this way, the corresponding sdio_release_func() will help to manage
> In sdio_release_func(), sdio_free_fun_cis() is called, it put refcount of
> 'func->card->dev', but the refcount isn't get until sdio_init_func()
> is successful. In this way, it's no need to put refcount of
> 'func->card->dev',
> so we can not call sdio_release_func() in patch1, and patch1 is needed.

I see.

However, in that case, it seems like we need to fix this slightly
differently. In patch1, we should not do the kfree() thing immediately
in the error patch, but rather rely on the reference counting (but in
a more clever way).

Kind regards
Uffe