Re: [PATCH v3 2/3] mmc: sdio: fix of node refcount leak in sdio_add_func()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, 9 Nov 2022 at 14:23, Yang Yingliang <yangyingliang@xxxxxxxxxx> wrote:
>
>
> On 2022/11/9 20:27, Ulf Hansson wrote:
> > On Wed, 9 Nov 2022 at 03:53, Yang Yingliang <yangyingliang@xxxxxxxxxx> 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@xxxxxxxxxx>
> >> ---
> >>   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



[Index of Archives]     [Linux Memonry Technology]     [Linux USB Devel]     [Linux Media]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux