Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
MIME-Version: 1.0
References: <20200303174349.075101355@linuxfoundation.org> <20200303174349.401386271@linuxfoundation.org>
 <20200304151316.GA2367@duo.ucw.cz> <20200304171817.GC1852712@kroah.com> <20200309101410.GA18031@duo.ucw.cz>
In-Reply-To: <20200309101410.GA18031@duo.ucw.cz>
From:   Doug Anderson <dianders@chromium.org>
Date:   Tue, 10 Mar 2020 14:03:55 -0700
Message-ID: <CAD=FV=X4u90Q3-QLHqUSGQkNZOXO4HZ_euVXCSUncQj0bhH3dw@mail.gmail.com>
Subject: Re: [PATCH] drm/msm: fix leaks if initialization fails
To:     Pavel Machek <pavel@denx.de>, Rob Clark <robdclark@gmail.com>,
        Sean Paul <seanpaul@chromium.org>
Cc:     Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        LKML <linux-kernel@vger.kernel.org>,
        "# 4.0+" <stable@vger.kernel.org>,
        Stephen Boyd <swboyd@chromium.org>,
        Sai Prakash Ranjan <saiprakash.ranjan@codeaurora.org>,
        Sasha Levin <sashal@kernel.org>
Content-Type: text/plain; charset="UTF-8"
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

Hi,

On Mon, Mar 9, 2020 at 3:14 AM Pavel Machek <pavel@denx.de> wrote:
>
> We should free resources in unlikely case of allocation failure.
>
> Signed-off-by: Pavel Machek <pavel@denx.de>
>
> ---
>
> > Can you submit a patch to fix it?
>
> Here it is.
>
> Best regards,
>                                                                 Pavel
>
>
> diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c
> index 83a0000eecb3..f5c1495cc4b9 100644
> --- a/drivers/gpu/drm/msm/msm_drv.c
> +++ b/drivers/gpu/drm/msm/msm_drv.c
> @@ -444,8 +444,10 @@ static int msm_drm_init(struct device *dev, struct drm_driver *drv)
>         if (!dev->dma_parms) {
>                 dev->dma_parms = devm_kzalloc(dev, sizeof(*dev->dma_parms),
>                                               GFP_KERNEL);
> -               if (!dev->dma_parms)
> -                       return -ENOMEM;
> +               if (!dev->dma_parms) {
> +                       ret = -ENOMEM;
> +                       goto err_msm_uninit;
> +               }
>         }
>         dma_set_max_seg_size(dev, DMA_BIT_MASK(32));

Looks good.  Error cases both above and below your "goto" both already
go to err_msm_uninit(), so it makes sense it would also be the
appropriate place for you to go to.  ...and no extra cleanup was
needed for dma_parms allocation since it was devm.  Thus:

Fixes: db735fc4036b ("drm/msm: Set dma maximum segment size for mdss")
Reviewed-by: Douglas Anderson <dianders@chromium.org>

Thanks for noticing and fixing!

-Doug