[PATCH] clk: bcm2835: add missing release if devm_clk_hw_register fails

[Navid Emamdoost]

In the implementation of bcm2835_register_pll(), the allocated pll is
leaked if devm_clk_hw_register() fails to register hw. Release pll if
devm_clk_hw_register() fails.

Signed-off-by: Navid Emamdoost 
---
 drivers/clk/bcm/clk-bcm2835.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/clk/bcm/clk-bcm2835.c b/drivers/clk/bcm/clk-bcm2835.c
index 6bb7efa12037..12b950d50252 100644
--- a/drivers/clk/bcm/clk-bcm2835.c
+++ b/drivers/clk/bcm/clk-bcm2835.c
@@ -1321,8 +1321,10 @@ static struct clk_hw *bcm2835_register_pll(struct 
bcm2835_cprman *cprman,
pll->hw.init = 
 
ret = devm_clk_hw_register(cprman->dev, >hw);
-   if (ret)
+   if (ret) {
+   kfree(pll);
return NULL;
+   }
return >hw;
 }
 
-- 
2.17.1

[PATCH] scsi: qla2xxx: fix memory leak if qlt_add_target fails

[Navid Emamdoost]

In the implementation of qla2x00_probe_one() the allocated and
initialized ha is leaked if it fails to add target via qlt_add_target().
Go to error handling path if qlt_add_target() fails.

Signed-off-by: Navid Emamdoost 
---
 drivers/scsi/qla2xxx/qla_os.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/scsi/qla2xxx/qla_os.c b/drivers/scsi/qla2xxx/qla_os.c
index e92fad99338c..81a58ae54909 100644
--- a/drivers/scsi/qla2xxx/qla_os.c
+++ b/drivers/scsi/qla2xxx/qla_os.c
@@ -3457,7 +3457,9 @@ qla2x00_probe_one(struct pci_dev *pdev, const struct 
pci_device_id *id)
base_vha->host_no,
ha->isp_ops->fw_version_str(base_vha, fw_str, sizeof(fw_str)));
 
-   qlt_add_target(ha, base_vha);
+   ret = qlt_add_target(ha, base_vha);
+   if (ret)
+   goto probe_failed;
 
clear_bit(PFLG_DRIVER_PROBING, _vha->pci_flags);
 
-- 
2.17.1

[PATCH v3] cxgb4: add missing release on skb in uld_send()

[Navid Emamdoost]

In the implementation of uld_send(), the skb is consumed on all
execution paths except one. Release skb when returning NET_XMIT_DROP.

Signed-off-by: Navid Emamdoost 
---
v3:
- fixed the base problem, and used kfree_skb
---
 drivers/net/ethernet/chelsio/cxgb4/sge.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/ethernet/chelsio/cxgb4/sge.c 
b/drivers/net/ethernet/chelsio/cxgb4/sge.c
index 32a45dc51ed7..92eee66cbc84 100644
--- a/drivers/net/ethernet/chelsio/cxgb4/sge.c
+++ b/drivers/net/ethernet/chelsio/cxgb4/sge.c
@@ -2938,6 +2938,7 @@ static inline int uld_send(struct adapter *adap, struct 
sk_buff *skb,
txq_info = adap->sge.uld_txq_info[tx_uld_type];
if (unlikely(!txq_info)) {
WARN_ON(true);
+   kfree_skb(skb);
return NET_XMIT_DROP;
}
 
-- 
2.17.1

[PATCH v2] cxgb4: add missing release on skb in uld_send()

[Navid Emamdoost]

In the implementation of uld_send(), the skb is consumed on all
execution paths except one. Release skb when returning NET_XMIT_DROP.

Signed-off-by: Navid Emamdoost 
---
changes in v2:
- using kfree_skb() based on David Miller suggestion.
---
 drivers/net/ethernet/chelsio/cxgb4/sge.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/chelsio/cxgb4/sge.c 
b/drivers/net/ethernet/chelsio/cxgb4/sge.c
index d8c37fd4b808..92eee66cbc84 100644
--- a/drivers/net/ethernet/chelsio/cxgb4/sge.c
+++ b/drivers/net/ethernet/chelsio/cxgb4/sge.c
@@ -2938,7 +2938,7 @@ static inline int uld_send(struct adapter *adap, struct 
sk_buff *skb,
txq_info = adap->sge.uld_txq_info[tx_uld_type];
if (unlikely(!txq_info)) {
WARN_ON(true);
-   consume_skb(skb);
+   kfree_skb(skb);
return NET_XMIT_DROP;
}
 
-- 
2.17.1

[PATCH] mt76: mt76u: add missing release on skb in __mt76x02u_mcu_send_msg

[Navid Emamdoost]

In the implementation of __mt76x02u_mcu_send_msg() the skb is consumed
all execution paths except one. Release skb before returning if
test_bit() fails.

Signed-off-by: Navid Emamdoost 
---
 drivers/net/wireless/mediatek/mt76/mt76x02_usb_mcu.c | 7 +--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wireless/mediatek/mt76/mt76x02_usb_mcu.c 
b/drivers/net/wireless/mediatek/mt76/mt76x02_usb_mcu.c
index a30bb536fc8a..e43d13d7c988 100644
--- a/drivers/net/wireless/mediatek/mt76/mt76x02_usb_mcu.c
+++ b/drivers/net/wireless/mediatek/mt76/mt76x02_usb_mcu.c
@@ -87,8 +87,10 @@ __mt76x02u_mcu_send_msg(struct mt76_dev *dev, struct sk_buff 
*skb,
u32 info;
int ret;
 
-   if (test_bit(MT76_REMOVED, >phy.state))
-   return 0;
+   if (test_bit(MT76_REMOVED, >phy.state)) {
+   ret = 0;
+   goto out;
+   }
 
if (wait_resp) {
seq = ++dev->mcu.msg_seq & 0xf;
@@ -111,6 +113,7 @@ __mt76x02u_mcu_send_msg(struct mt76_dev *dev, struct 
sk_buff *skb,
if (wait_resp)
ret = mt76x02u_mcu_wait_resp(dev, seq);
 
+out:
consume_skb(skb);
 
return ret;
-- 
2.17.1

[PATCH] nfc: s3fwrn5: add missing release on skb in s3fwrn5_recv_frame

[Navid Emamdoost]

The implementation of s3fwrn5_recv_frame() is supposed to consume skb on
all execution paths. Release skb before returning -ENODEV.

Signed-off-by: Navid Emamdoost 
---
 drivers/nfc/s3fwrn5/core.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/nfc/s3fwrn5/core.c b/drivers/nfc/s3fwrn5/core.c
index 91d4d5b28a7d..ba6c486d6465 100644
--- a/drivers/nfc/s3fwrn5/core.c
+++ b/drivers/nfc/s3fwrn5/core.c
@@ -198,6 +198,7 @@ int s3fwrn5_recv_frame(struct nci_dev *ndev, struct sk_buff 
*skb,
case S3FWRN5_MODE_FW:
return s3fwrn5_fw_recv_frame(ndev, skb);
default:
+   kfree_skb(skb);
return -ENODEV;
}
 }
-- 
2.17.1

[PATCH] mt7601u: add missing release on skb in mt7601u_mcu_msg_send

[Navid Emamdoost]

In the implementation of mt7601u_mcu_msg_send(), skb is supposed to be
consumed on all execution paths. Release skb before returning if
test_bit() fails.

Signed-off-by: Navid Emamdoost 
---
 drivers/net/wireless/mediatek/mt7601u/mcu.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/net/wireless/mediatek/mt7601u/mcu.c 
b/drivers/net/wireless/mediatek/mt7601u/mcu.c
index af55ed82b96f..1b5cc271a9e1 100644
--- a/drivers/net/wireless/mediatek/mt7601u/mcu.c
+++ b/drivers/net/wireless/mediatek/mt7601u/mcu.c
@@ -116,8 +116,10 @@ mt7601u_mcu_msg_send(struct mt7601u_dev *dev, struct 
sk_buff *skb,
int sent, ret;
u8 seq = 0;
 
-   if (test_bit(MT7601U_STATE_REMOVED, >state))
+   if (test_bit(MT7601U_STATE_REMOVED, >state)) {
+   consume_skb(skb);
return 0;
+   }
 
mutex_lock(>mcu.mutex);
 
-- 
2.17.1

[PATCH] cxgb4: add missing release on skb in uld_send()

[Navid Emamdoost]

In the implementation of uld_send(), the skb is consumed on all
execution paths except one. Release skb when returning NET_XMIT_DROP.

Signed-off-by: Navid Emamdoost 
---
 drivers/net/ethernet/chelsio/cxgb4/sge.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/ethernet/chelsio/cxgb4/sge.c 
b/drivers/net/ethernet/chelsio/cxgb4/sge.c
index 32a45dc51ed7..d8c37fd4b808 100644
--- a/drivers/net/ethernet/chelsio/cxgb4/sge.c
+++ b/drivers/net/ethernet/chelsio/cxgb4/sge.c
@@ -2938,6 +2938,7 @@ static inline int uld_send(struct adapter *adap, struct 
sk_buff *skb,
txq_info = adap->sge.uld_txq_info[tx_uld_type];
if (unlikely(!txq_info)) {
WARN_ON(true);
+   consume_skb(skb);
return NET_XMIT_DROP;
}
 
-- 
2.17.1

Re: [PATCH] ALSA: trident: Fix a memory leak in snd_trident_create

[Navid Emamdoost]

On Sat, Jul 11, 2020 at 4:04 AM Takashi Iwai  wrote:
>
> On Sat, 11 Jul 2020 09:08:30 +0200,
> Navid Emamdoost wrote:
> >
> > In the implementation of snd_trident_create(), the allocated trident is
> > leaked if snd_trident_mixer() fails. Release via snd_trident_free().
>
> No, this patch would result in double-free.
>
> The manual release of trident object isn't needed once after it gets
> added via snd_device_new().  Then it'll be automatically released at
> the error path (via snd_trident_dev_free()).

Thanks for the clarification.

>
>
> thanks,
>
> Takashi
>
> >
> > Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> > Signed-off-by: Navid Emamdoost 
> > ---
> >  sound/pci/trident/trident_main.c | 5 -
> >  1 file changed, 4 insertions(+), 1 deletion(-)
> >
> > diff --git a/sound/pci/trident/trident_main.c 
> > b/sound/pci/trident/trident_main.c
> > index 6e50376163a2..e98c692f6aa9 100644
> > --- a/sound/pci/trident/trident_main.c
> > +++ b/sound/pci/trident/trident_main.c
> > @@ -3582,8 +3582,11 @@ int snd_trident_create(struct snd_card *card,
> >   return err;
> >   }
> >
> > - if ((err = snd_trident_mixer(trident, pcm_spdif_device)) < 0)
> > + err = snd_trident_mixer(trident, pcm_spdif_device);
> > + if (err < 0) {
> > + snd_trident_free(trident);
> >   return err;
> > + }
> >
> >   /* initialise synth voices */
> >   for (i = 0; i < 64; i++) {
> > --
> > 2.17.1
> >



-- 
Navid.

[PATCH] ALSA: trident: Fix a memory leak in snd_trident_create

[Navid Emamdoost]

In the implementation of snd_trident_create(), the allocated trident is
leaked if snd_trident_mixer() fails. Release via snd_trident_free().

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Signed-off-by: Navid Emamdoost 
---
 sound/pci/trident/trident_main.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/sound/pci/trident/trident_main.c b/sound/pci/trident/trident_main.c
index 6e50376163a2..e98c692f6aa9 100644
--- a/sound/pci/trident/trident_main.c
+++ b/sound/pci/trident/trident_main.c
@@ -3582,8 +3582,11 @@ int snd_trident_create(struct snd_card *card,
return err;
}
 
-   if ((err = snd_trident_mixer(trident, pcm_spdif_device)) < 0)
+   err = snd_trident_mixer(trident, pcm_spdif_device);
+   if (err < 0) {
+   snd_trident_free(trident);
return err;
+   }

/* initialise synth voices */
for (i = 0; i < 64; i++) {
-- 
2.17.1

Re: [PATCH] dmaengine: stm32-dmamux: fix pm_runtime_get_sync fialure cases

[Navid Emamdoost]

On Wed, Jun 24, 2020 at 2:40 AM Vinod Koul  wrote:
>
> On 03-06-20, 14:36, Navid Emamdoost wrote:
>
> s/fialure/failure
>
> > Calling pm_runtime_get_sync increments the counter even in case of
> > failure, causing incorrect ref count. Call pm_runtime_put_sync if
> > pm_runtime_get_sync fails.
> >
> > Signed-off-by: Navid Emamdoost 
> > ---
> >  drivers/dma/stm32-dmamux.c | 5 -
> >  1 file changed, 4 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/dma/stm32-dmamux.c b/drivers/dma/stm32-dmamux.c
> > index 12f7637e13a1..ab250d7eed29 100644
> > --- a/drivers/dma/stm32-dmamux.c
> > +++ b/drivers/dma/stm32-dmamux.c
> > @@ -140,6 +140,7 @@ static void *stm32_dmamux_route_allocate(struct 
> > of_phandle_args *dma_spec,
> >   ret = pm_runtime_get_sync(>dev);
> >   if (ret < 0) {
> >   spin_unlock_irqrestore(>lock, flags);
> > + pm_runtime_put_sync(>dev);
>
> why put_sync()
>
> >   goto error;
> >   }
> >   spin_unlock_irqrestore(>lock, flags);
> > @@ -340,8 +341,10 @@ static int stm32_dmamux_suspend(struct device *dev)
> >   int i, ret;
> >
> >   ret = pm_runtime_get_sync(dev);
> > - if (ret < 0)
> > + if (ret < 0) {
> > + pm_runtime_put_sync(dev);
>
> here too

Is put_noidle() better?

>
> >   return ret;
> > + }
> >
> >   for (i = 0; i < stm32_dmamux->dma_requests; i++)
> >   stm32_dmamux->ccr[i] = stm32_dmamux_read(stm32_dmamux->iomem,
> > --
> > 2.17.1
>
> --
> ~Vinod



-- 
Navid.

Re: [PATCH] engine: stm32-dma: call pm_runtime_put if pm_runtime_get_sync fails

[Navid Emamdoost]

Hi,

On Wed, Jun 24, 2020 at 2:39 AM Vinod Koul  wrote:
>
> On 03-06-20, 13:34, Navid Emamdoost wrote:
> > Calling pm_runtime_get_sync increments the counter even in case of
> > failure, causing incorrect ref count. Call pm_runtime_put if
> > pm_runtime_get_sync fails.
>
> pls fix subsystem name as dmaengine: ...
> >
> > Signed-off-by: Navid Emamdoost 
> > ---
> >  drivers/dma/stm32-dma.c | 8 ++--
> >  1 file changed, 6 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/dma/stm32-dma.c b/drivers/dma/stm32-dma.c
> > index 0ddbaa4b4f0b..0aab86bd97fe 100644
> > --- a/drivers/dma/stm32-dma.c
> > +++ b/drivers/dma/stm32-dma.c
> > @@ -1169,8 +1169,10 @@ static int stm32_dma_alloc_chan_resources(struct 
> > dma_chan *c)
> >   chan->config_init = false;
> >
> >   ret = pm_runtime_get_sync(dmadev->ddev.dev);
> > - if (ret < 0)
> > + if (ret < 0) {
> > + pm_runtime_put(dmadev->ddev.dev);
> >   return ret;
> > + }
> >
> >   ret = stm32_dma_disable_chan(chan);
> >   if (ret < 0)
> > @@ -1439,8 +1441,10 @@ static int stm32_dma_suspend(struct device *dev)
> >   int id, ret, scr;
> >
> >   ret = pm_runtime_get_sync(dev);
> > - if (ret < 0)
> > + if (ret < 0) {
> > + pm_runtime_put_sync(dev);
>
> why put_sync()

My bad! I will fix it.

> >   return ret;
> > + }
> >
> >   for (id = 0; id < STM32_DMA_MAX_CHANNELS; id++) {
> >   scr = stm32_dma_read(dmadev, STM32_DMA_SCR(id));
> > --
> > 2.17.1
>
> --
> ~Vinod



-- 
Navid.

Re: [PATCH v2] drm/etnaviv: fix ref count leak via pm_runtime_get_sync

[Navid Emamdoost]

Hi Lucas,


On Wed, Jun 17, 2020 at 4:53 AM Lucas Stach  wrote:
>
> Hi Navid,
>
> Am Montag, den 15.06.2020, 01:12 -0500 schrieb Navid Emamdoost:
> > in etnaviv_gpu_submit, etnaviv_gpu_recover_hang, etnaviv_gpu_debugfs,
> > and etnaviv_gpu_init the call to pm_runtime_get_sync increments the
> > counter even in case of failure, leading to incorrect ref count.
> > In case of failure, decrement the ref count before returning.
>
> While that change is correct with the current API, may I ask the
> question why the way this API works is considered reasonable? A API
> call that fails, but still changes internal state and expects the
> caller to clean up the mess it not really what I would consider fool-
> proof API design. Is there a specific reason why it is done this way
> and not handled internally?

I share the same concern with you on the way this API is working now.
To the best of my knowledge, there are ongoing discussions on this
issue:

https://lkml.org/lkml/2020/6/14/76
https://patchwork.ozlabs.org/project/linux-tegra/patch/20200520095148.10995-1-dinghao....@zju.edu.cn/

>
> Regards,
> Lucas
>
> > Signed-off-by: Navid Emamdoost 
> > ---
> > Changes in v2:
> >   - replace pm_runtime_put with  pm_runtime_put_noidle
> > ---
> >  drivers/gpu/drm/etnaviv/etnaviv_gpu.c | 14 ++
> >  1 file changed, 10 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c 
> > b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
> > index a31eeff2b297..7c9f3f9ba123 100644
> > --- a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
> > +++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
> > @@ -722,7 +722,7 @@ int etnaviv_gpu_init(struct etnaviv_gpu *gpu)
> >   ret = pm_runtime_get_sync(gpu->dev);
> >   if (ret < 0) {
> >   dev_err(gpu->dev, "Failed to enable GPU power domain\n");
> > - return ret;
> > + goto pm_put;
> >   }
> >
> >   etnaviv_hw_identify(gpu);
> > @@ -819,6 +819,7 @@ int etnaviv_gpu_init(struct etnaviv_gpu *gpu)
> >
> >  fail:
> >   pm_runtime_mark_last_busy(gpu->dev);
> > +pm_put:
> >   pm_runtime_put_autosuspend(gpu->dev);
> >
> >   return ret;
> > @@ -859,7 +860,7 @@ int etnaviv_gpu_debugfs(struct etnaviv_gpu *gpu, struct 
> > seq_file *m)
> >
> >   ret = pm_runtime_get_sync(gpu->dev);
> >   if (ret < 0)
> > - return ret;
> > + goto pm_put;
> >
> >   dma_lo = gpu_read(gpu, VIVS_FE_DMA_LOW);
> >   dma_hi = gpu_read(gpu, VIVS_FE_DMA_HIGH);
> > @@ -1003,6 +1004,7 @@ int etnaviv_gpu_debugfs(struct etnaviv_gpu *gpu, 
> > struct seq_file *m)
> >   ret = 0;
> >
> >   pm_runtime_mark_last_busy(gpu->dev);
> > +pm_put:
> >   pm_runtime_put_autosuspend(gpu->dev);
> >
> >   return ret;
> > @@ -1016,7 +1018,7 @@ void etnaviv_gpu_recover_hang(struct etnaviv_gpu *gpu)
> >   dev_err(gpu->dev, "recover hung GPU!\n");
> >
> >   if (pm_runtime_get_sync(gpu->dev) < 0)
> > - return;
> > + goto pm_put;
> >
> >   mutex_lock(>lock);
> >
> > @@ -1035,6 +1037,7 @@ void etnaviv_gpu_recover_hang(struct etnaviv_gpu *gpu)
> >
> >   mutex_unlock(>lock);
> >   pm_runtime_mark_last_busy(gpu->dev);
> > +pm_put:
> >   pm_runtime_put_autosuspend(gpu->dev);
> >  }
> >
> > @@ -1308,8 +1311,10 @@ struct dma_fence *etnaviv_gpu_submit(struct 
> > etnaviv_gem_submit *submit)
> >
> >   if (!submit->runtime_resumed) {
> >   ret = pm_runtime_get_sync(gpu->dev);
> > - if (ret < 0)
> > + if (ret < 0) {
> > + pm_runtime_put_noidle(gpu->dev);
> >   return NULL;
> > + }
> >   submit->runtime_resumed = true;
> >   }
> >
> > @@ -1326,6 +1331,7 @@ struct dma_fence *etnaviv_gpu_submit(struct 
> > etnaviv_gem_submit *submit)
> >   ret = event_alloc(gpu, nr_events, event);
> >   if (ret) {
> >   DRM_ERROR("no free events\n");
> > + pm_runtime_put_noidle(gpu->dev);
> >   return NULL;
> >   }
> >
>


--
Navid.

Re: [PATCH] gpio: rcar: handle pm_runtime_get_sync failure case

[Navid Emamdoost]

Hi Geert,

On Wed, Jun 17, 2020 at 2:26 AM Geert Uytterhoeven  wrote:
>
> Hi Navid,
>
> On Fri, Jun 5, 2020 at 4:50 AM Navid Emamdoost
>  wrote:
> > Calling pm_runtime_get_sync increments the counter even in case of
> > failure, causing incorrect ref count. Call pm_runtime_put if
> > pm_runtime_get_sync fails.
> >
> > Signed-off-by: Navid Emamdoost 
>
> Thanks for your patch!
>
> > --- a/drivers/gpio/gpio-rcar.c
> > +++ b/drivers/gpio/gpio-rcar.c
> > @@ -250,8 +250,10 @@ static int gpio_rcar_request(struct gpio_chip *chip, 
> > unsigned offset)
> > int error;
> >
> > error = pm_runtime_get_sync(p->dev);
> > -   if (error < 0)
> > +   if (error < 0) {
> > +   pm_runtime_put(p->dev);
>
> As per [1], I would like to see a call to pm_runtime_put_noidle() instead.

V2 was sent per your suggestion, thanks.

>
> [1] 
> http://lore.kernel.org/r/cajz5v0i87ngcy9+kxubscdpdybyr8ypqwcggbfn+v-wdd69...@mail.gmail.com
>
> > return error;
> > +   }
> >
> > error = pinctrl_gpio_request(chip->base + offset);
> > if (error)
>
> Gr{oetje,eeting}s,
>
> Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- 
> ge...@linux-m68k.org
>
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like 
> that.
> -- Linus Torvalds



-- 
Navid.

[PATCH v2] gpio: rcar: handle pm_runtime_get_sync failure case

[Navid Emamdoost]

Calling pm_runtime_get_sync increments the counter even in case of
failure, causing incorrect ref count. Call pm_runtime_put if
pm_runtime_get_sync fails.

Signed-off-by: Navid Emamdoost 
---
Changes in v2:
- use pm_runtime_put_noidle
---
 drivers/gpio/gpio-rcar.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/gpio/gpio-rcar.c b/drivers/gpio/gpio-rcar.c
index 7284473c9fe3..8fb9079a1d60 100644
--- a/drivers/gpio/gpio-rcar.c
+++ b/drivers/gpio/gpio-rcar.c
@@ -250,8 +250,10 @@ static int gpio_rcar_request(struct gpio_chip *chip, 
unsigned offset)
int error;
 
error = pm_runtime_get_sync(p->dev);
-   if (error < 0)
+   if (error < 0) {
+   pm_runtime_put_noidle(p->dev);
return error;
+   }
 
error = pinctrl_gpio_request(chip->base + offset);
if (error)
-- 
2.17.1

Re: [PATCH] Input: bma150: fix ref count leak in bma150_open

[Navid Emamdoost]

On Sun, Jun 14, 2020 at 4:27 AM Andy Shevchenko
 wrote:
>
> On Sun, Jun 14, 2020 at 8:58 AM Navid Emamdoost
>  wrote:
> >
> > in bma150_open, pm_runtime_get_sync is called which
> > increments the counter even in case of failure, leading to incorrect
> > ref count. In case of failure, decrement the ref count before returning.
>
> ...
>
> > error = pm_runtime_get_sync(>client->dev);
> > if (error < 0 && error != -ENOSYS)
> > -   return error;
> > +   goto out;
>
> So, what will happen in case of -ENOSYS?
I'm not sure!

>
> ...
>
> > +   pm_runtime_put(>client->dev);
>
> Slightly better to use _put_noidle(). (More consistency with error path)

v2 is sent.

>
> --
> With Best Regards,
> Andy Shevchenko



-- 
Navid.

[PATCH v2] Input: bma150: fix ref count leak in bma150_open

[Navid Emamdoost]

in bma150_open, pm_runtime_get_sync is called which
increments the counter even in case of failure, leading to incorrect
ref count. In case of failure, decrement the ref count before returning.

Signed-off-by: Navid Emamdoost 
---
Changes in v2:
-- repplace pm_runtime_put with pm_runtime_put_noidle
---
 drivers/input/misc/bma150.c | 7 +--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/input/misc/bma150.c b/drivers/input/misc/bma150.c
index a9d984da95f3..ec394b4faa14 100644
--- a/drivers/input/misc/bma150.c
+++ b/drivers/input/misc/bma150.c
@@ -348,7 +348,7 @@ static int bma150_open(struct input_dev *input)
 
error = pm_runtime_get_sync(>client->dev);
if (error < 0 && error != -ENOSYS)
-   return error;
+   goto out;
 
/*
 * See if runtime PM woke up the device. If runtime PM
@@ -357,10 +357,13 @@ static int bma150_open(struct input_dev *input)
if (bma150->mode != BMA150_MODE_NORMAL) {
error = bma150_set_mode(bma150, BMA150_MODE_NORMAL);
if (error)
-   return error;
+   goto out;
}
 
return 0;
+out:
+   pm_runtime_put_noidle(>client->dev);
+   return error;
 }
 
 static void bma150_close(struct input_dev *input)
-- 
2.17.1

Re: [PATCH] drm/etnaviv: fix ref count leak via pm_runtime_get_sync

[Navid Emamdoost]

On Sun, Jun 14, 2020 at 4:48 AM Andy Shevchenko
 wrote:
>
> On Sun, Jun 14, 2020 at 9:48 AM Navid Emamdoost
>  wrote:
>
> ...
>
> > +   if (ret < 0) {
>
> > +   pm_runtime_put(gpu->dev);
>
> Please, in all your patches fix this to be _put_noidle(). We wouldn't
> bear the flag day of fixing these parts again.
> Yes, I know that *now* behaviour is the same, but calling put here is
> slightly inconsistent.

v2 is sent.

>
> ...
>
> > +   pm_runtime_put(gpu->dev);
>
> --
> With Best Regards,
> Andy Shevchenko



-- 
Navid.

[PATCH v2] drm/etnaviv: fix ref count leak via pm_runtime_get_sync

[Navid Emamdoost]

in etnaviv_gpu_submit, etnaviv_gpu_recover_hang, etnaviv_gpu_debugfs,
and etnaviv_gpu_init the call to pm_runtime_get_sync increments the
counter even in case of failure, leading to incorrect ref count.
In case of failure, decrement the ref count before returning.

Signed-off-by: Navid Emamdoost 
---
Changes in v2:
- replace pm_runtime_put with  pm_runtime_put_noidle
---
 drivers/gpu/drm/etnaviv/etnaviv_gpu.c | 14 ++
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c 
b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
index a31eeff2b297..7c9f3f9ba123 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
@@ -722,7 +722,7 @@ int etnaviv_gpu_init(struct etnaviv_gpu *gpu)
ret = pm_runtime_get_sync(gpu->dev);
if (ret < 0) {
dev_err(gpu->dev, "Failed to enable GPU power domain\n");
-   return ret;
+   goto pm_put;
}
 
etnaviv_hw_identify(gpu);
@@ -819,6 +819,7 @@ int etnaviv_gpu_init(struct etnaviv_gpu *gpu)
 
 fail:
pm_runtime_mark_last_busy(gpu->dev);
+pm_put:
pm_runtime_put_autosuspend(gpu->dev);
 
return ret;
@@ -859,7 +860,7 @@ int etnaviv_gpu_debugfs(struct etnaviv_gpu *gpu, struct 
seq_file *m)
 
ret = pm_runtime_get_sync(gpu->dev);
if (ret < 0)
-   return ret;
+   goto pm_put;
 
dma_lo = gpu_read(gpu, VIVS_FE_DMA_LOW);
dma_hi = gpu_read(gpu, VIVS_FE_DMA_HIGH);
@@ -1003,6 +1004,7 @@ int etnaviv_gpu_debugfs(struct etnaviv_gpu *gpu, struct 
seq_file *m)
ret = 0;
 
pm_runtime_mark_last_busy(gpu->dev);
+pm_put:
pm_runtime_put_autosuspend(gpu->dev);
 
return ret;
@@ -1016,7 +1018,7 @@ void etnaviv_gpu_recover_hang(struct etnaviv_gpu *gpu)
dev_err(gpu->dev, "recover hung GPU!\n");
 
if (pm_runtime_get_sync(gpu->dev) < 0)
-   return;
+   goto pm_put;
 
mutex_lock(>lock);
 
@@ -1035,6 +1037,7 @@ void etnaviv_gpu_recover_hang(struct etnaviv_gpu *gpu)
 
mutex_unlock(>lock);
pm_runtime_mark_last_busy(gpu->dev);
+pm_put:
pm_runtime_put_autosuspend(gpu->dev);
 }
 
@@ -1308,8 +1311,10 @@ struct dma_fence *etnaviv_gpu_submit(struct 
etnaviv_gem_submit *submit)
 
if (!submit->runtime_resumed) {
ret = pm_runtime_get_sync(gpu->dev);
-   if (ret < 0)
+   if (ret < 0) {
+   pm_runtime_put_noidle(gpu->dev);
return NULL;
+   }
submit->runtime_resumed = true;
}
 
@@ -1326,6 +1331,7 @@ struct dma_fence *etnaviv_gpu_submit(struct 
etnaviv_gem_submit *submit)
ret = event_alloc(gpu, nr_events, event);
if (ret) {
DRM_ERROR("no free events\n");
+   pm_runtime_put_noidle(gpu->dev);
return NULL;
}
 
-- 
2.17.1

Re: [PATCH] drm/exynos: fix ref count leak in mic_pre_enable

[Navid Emamdoost]

On Sun, Jun 14, 2020 at 7:47 PM Inki Dae  wrote:
>
> Hi,
>
> 20. 6. 14. 오후 3:23에 Navid Emamdoost 이(가) 쓴 글:
> > in mic_pre_enable, pm_runtime_get_sync is called which
> > increments the counter even in case of failure, leading to incorrect
> > ref count. In case of failure, decrement the ref count before returning.
> >
> > Signed-off-by: Navid Emamdoost 
> > ---
> >  drivers/gpu/drm/exynos/exynos_drm_mic.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/exynos/exynos_drm_mic.c 
> > b/drivers/gpu/drm/exynos/exynos_drm_mic.c
> > index a86abc173605..69ff74c2ceb5 100644
> > --- a/drivers/gpu/drm/exynos/exynos_drm_mic.c
> > +++ b/drivers/gpu/drm/exynos/exynos_drm_mic.c
> > @@ -270,7 +270,7 @@ static void mic_pre_enable(struct drm_bridge *bridge)
> >
> >   ret = pm_runtime_get_sync(mic->dev);
> >   if (ret < 0)
> > - goto unlock;
> > + goto turn_off;
>
> How about just calling pm_runtime_put_noidle()?
>
> if (ret < 0) {
> pm_runtime_put_noidle(mic->dev);
> goto unlock;
> }
>
v2 was sent.

> Thanks,
> Inki Dae
>
> >
> >   mic_set_path(mic, 1);
> >
> >



-- 
Navid.

[PATCH v2] drm/exynos: fix ref count leak in mic_pre_enable

[Navid Emamdoost]

in mic_pre_enable, pm_runtime_get_sync is called which
increments the counter even in case of failure, leading to incorrect
ref count. In case of failure, decrement the ref count before returning.

Signed-off-by: Navid Emamdoost 
---
Changes in v2:
- reuse the unlock label and call pm_runtime_put_noidle
---
---
 drivers/gpu/drm/exynos/exynos_drm_mic.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/exynos/exynos_drm_mic.c 
b/drivers/gpu/drm/exynos/exynos_drm_mic.c
index a86abc173605..3821ea76a703 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_mic.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_mic.c
@@ -269,8 +269,10 @@ static void mic_pre_enable(struct drm_bridge *bridge)
goto unlock;
 
ret = pm_runtime_get_sync(mic->dev);
-   if (ret < 0)
+   if (ret < 0) {
+   pm_runtime_put_noidle(mic->dev);
goto unlock;
+   }
 
mic_set_path(mic, 1);
 
-- 
2.17.1

Re: [PATCH] drm/vc4: fix ref count leak in vc4_dsi_encoder_enable

[Navid Emamdoost]

On Sun, Jun 14, 2020 at 7:32 AM Andy Shevchenko
 wrote:
>
> On Sun, Jun 14, 2020 at 9:55 AM Navid Emamdoost
>  wrote:
> >
> > in vc4_dsi_encoder_enable, the call to pm_runtime_get_sync increments
> > the counter even in case of failure, leading to incorrect
> > ref count. In case of failure, decrement the ref count before returning.
>
> ...
>
> > +out:
> > +   pm_runtime_put(dev);
>
> Better to use pm_runtime_put_noidle() for error case.
> And here is a change of semantics, i.e. before your patch there was no
> put at all. How did you test this?
I had no way to test this but looked to me like a miscalculation of
ref count when there is a get and an error happens then the ref count
should be restored. Does that look incorrect?

>
> --
> With Best Regards,
> Andy Shevchenko



-- 
Navid.

[PATCH] drm/amd/display: fix ref count leak in amdgpu_drm_ioctl

[Navid Emamdoost]

in amdgpu_drm_ioctl the call to pm_runtime_get_sync increments the
counter even in case of failure, leading to incorrect
ref count. In case of failure, decrement the ref count before returning.

Signed-off-by: Navid Emamdoost 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
index 126e74758a34..d73924e35a57 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
@@ -1373,11 +1373,12 @@ long amdgpu_drm_ioctl(struct file *filp,
dev = file_priv->minor->dev;
ret = pm_runtime_get_sync(dev->dev);
if (ret < 0)
-   return ret;
+   goto out;
 
ret = drm_ioctl(filp, cmd, arg);
 
pm_runtime_mark_last_busy(dev->dev);
+out:
pm_runtime_put_autosuspend(dev->dev);
return ret;
 }
-- 
2.17.1

[PATCH] drm/amdgpu: fix ref count leak in amdgpu_driver_open_kms

[Navid Emamdoost]

in amdgpu_driver_open_kms the call to pm_runtime_get_sync increments the
counter even in case of failure, leading to incorrect
ref count. In case of failure, decrement the ref count before returning.

Signed-off-by: Navid Emamdoost 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
index d7e17e34fee1..bd40aa307462 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
@@ -991,7 +991,7 @@ int amdgpu_driver_open_kms(struct drm_device *dev, struct 
drm_file *file_priv)
 
r = pm_runtime_get_sync(dev->dev);
if (r < 0)
-   return r;
+   goto pm_put;
 
fpriv = kzalloc(sizeof(*fpriv), GFP_KERNEL);
if (unlikely(!fpriv)) {
@@ -1042,6 +1042,7 @@ int amdgpu_driver_open_kms(struct drm_device *dev, struct 
drm_file *file_priv)
 
 out_suspend:
pm_runtime_mark_last_busy(dev->dev);
+pm_put:
pm_runtime_put_autosuspend(dev->dev);
 
return r;
-- 
2.17.1

[PATCH] drm/amdgpu: fix ref count leak in amdgpu_display_crtc_set_config

[Navid Emamdoost]

in amdgpu_display_crtc_set_config, the call to pm_runtime_get_sync
increments the counter even in case of failure, leading to incorrect
ref count. In case of failure, decrement the ref count before returning.

Signed-off-by: Navid Emamdoost 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_display.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
index f7143d927b6d..5e51f0acf744 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
@@ -282,7 +282,7 @@ int amdgpu_display_crtc_set_config(struct drm_mode_set *set,
 
ret = pm_runtime_get_sync(dev->dev);
if (ret < 0)
-   return ret;
+   goto out;
 
ret = drm_crtc_helper_set_config(set, ctx);
 
@@ -297,7 +297,7 @@ int amdgpu_display_crtc_set_config(struct drm_mode_set *set,
   take the current one */
if (active && !adev->have_disp_power_ref) {
adev->have_disp_power_ref = true;
-   return ret;
+   goto out;
}
/* if we have no active crtcs, then drop the power ref
   we got before */
@@ -306,6 +306,7 @@ int amdgpu_display_crtc_set_config(struct drm_mode_set *set,
adev->have_disp_power_ref = false;
}
 
+out:
/* drop the power reference we got coming in here */
pm_runtime_put_autosuspend(dev->dev);
return ret;
-- 
2.17.1

[PATCH] drm/amdgpu/display: fix ref count leak when pm_runtime_get_sync fails

[Navid Emamdoost]

The call to pm_runtime_get_sync increments the counter even in case of
failure, leading to incorrect ref count.
In case of failure, decrement the ref count before returning.

Signed-off-by: Navid Emamdoost 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c | 16 
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c
index f355d9a752d2..a1aec205435d 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c
@@ -716,8 +716,10 @@ amdgpu_connector_lvds_detect(struct drm_connector 
*connector, bool force)
 
if (!drm_kms_helper_is_poll_worker()) {
r = pm_runtime_get_sync(connector->dev->dev);
-   if (r < 0)
+   if (r < 0) {
+   pm_runtime_put_autosuspend(connector->dev->dev);
return connector_status_disconnected;
+   }
}
 
if (encoder) {
@@ -854,8 +856,10 @@ amdgpu_connector_vga_detect(struct drm_connector 
*connector, bool force)
 
if (!drm_kms_helper_is_poll_worker()) {
r = pm_runtime_get_sync(connector->dev->dev);
-   if (r < 0)
+   if (r < 0) {
+   pm_runtime_put_autosuspend(connector->dev->dev);
return connector_status_disconnected;
+   }
}
 
encoder = amdgpu_connector_best_single_encoder(connector);
@@ -977,8 +981,10 @@ amdgpu_connector_dvi_detect(struct drm_connector 
*connector, bool force)
 
if (!drm_kms_helper_is_poll_worker()) {
r = pm_runtime_get_sync(connector->dev->dev);
-   if (r < 0)
+   if (r < 0) {
+   pm_runtime_put_autosuspend(connector->dev->dev);
return connector_status_disconnected;
+   }
}
 
if (!force && amdgpu_connector_check_hpd_status_unchanged(connector)) {
@@ -1328,8 +1334,10 @@ amdgpu_connector_dp_detect(struct drm_connector 
*connector, bool force)
 
if (!drm_kms_helper_is_poll_worker()) {
r = pm_runtime_get_sync(connector->dev->dev);
-   if (r < 0)
+   if (r < 0) {
+   pm_runtime_put_autosuspend(connector->dev->dev);
return connector_status_disconnected;
+   }
}
 
if (!force && amdgpu_connector_check_hpd_status_unchanged(connector)) {
-- 
2.17.1

[PATCH] drm/vc4: fix ref count leak in vc4_vec_encoder_enable

[Navid Emamdoost]

in vc4_vec_encoder_enable, the call to pm_runtime_get_sync increments
the counter even in case of failure, leading to incorrect
ref count. In case of failure, decrement the ref count before returning.

Signed-off-by: Navid Emamdoost 
---
 drivers/gpu/drm/vc4/vc4_vec.c | 8 +---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/vc4/vc4_vec.c b/drivers/gpu/drm/vc4/vc4_vec.c
index bd5b8eb58b18..706fdc7758f9 100644
--- a/drivers/gpu/drm/vc4/vc4_vec.c
+++ b/drivers/gpu/drm/vc4/vc4_vec.c
@@ -406,7 +406,7 @@ static void vc4_vec_encoder_enable(struct drm_encoder 
*encoder)
ret = pm_runtime_get_sync(>pdev->dev);
if (ret < 0) {
DRM_ERROR("Failed to retain power domain: %d\n", ret);
-   return;
+   goto out;
}
 
/*
@@ -419,13 +419,13 @@ static void vc4_vec_encoder_enable(struct drm_encoder 
*encoder)
ret = clk_set_rate(vec->clock, 10800);
if (ret) {
DRM_ERROR("Failed to set clock rate: %d\n", ret);
-   return;
+   goto out;
}
 
ret = clk_prepare_enable(vec->clock);
if (ret) {
DRM_ERROR("Failed to turn on core clock: %d\n", ret);
-   return;
+   goto out;
}
 
/* Reset the different blocks */
@@ -464,6 +464,8 @@ static void vc4_vec_encoder_enable(struct drm_encoder 
*encoder)
VEC_WRITE(VEC_DAC_MISC,
  VEC_DAC_MISC_VID_ACT | VEC_DAC_MISC_DAC_RST_N);
VEC_WRITE(VEC_CFG, VEC_CFG_VEC_EN);
+out:
+   pm_runtime_put(>pdev->dev);
 }
 
 
-- 
2.17.1

[PATCH] drm/vc4: fix ref count leak in vc4_v3d_pm_get

[Navid Emamdoost]

in vc4_v3d_pm_get, the call to pm_runtime_get_sync increments the
counter even in case of failure, leading to incorrect
ref count. In case of failure, decrement the ref count before returning.

Signed-off-by: Navid Emamdoost 
---
 drivers/gpu/drm/vc4/vc4_v3d.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/vc4/vc4_v3d.c b/drivers/gpu/drm/vc4/vc4_v3d.c
index cea77a21b205..bb03c1d1bb4d 100644
--- a/drivers/gpu/drm/vc4/vc4_v3d.c
+++ b/drivers/gpu/drm/vc4/vc4_v3d.c
@@ -135,6 +135,7 @@ vc4_v3d_pm_get(struct vc4_dev *vc4)
 
if (ret < 0) {
vc4->power_refcount--;
+   pm_runtime_put(>v3d->pdev->dev);
mutex_unlock(>power_lock);
return ret;
}
-- 
2.17.1

[PATCH] drm/vc4: fix ref count leak in vc4_dsi_encoder_enable

[Navid Emamdoost]

in vc4_dsi_encoder_enable, the call to pm_runtime_get_sync increments
the counter even in case of failure, leading to incorrect
ref count. In case of failure, decrement the ref count before returning.

Signed-off-by: Navid Emamdoost 
---
 drivers/gpu/drm/vc4/vc4_dsi.c | 10 ++
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/vc4/vc4_dsi.c b/drivers/gpu/drm/vc4/vc4_dsi.c
index eaf276978ee7..e651de9d1c7d 100644
--- a/drivers/gpu/drm/vc4/vc4_dsi.c
+++ b/drivers/gpu/drm/vc4/vc4_dsi.c
@@ -838,7 +838,7 @@ static void vc4_dsi_encoder_enable(struct drm_encoder 
*encoder)
ret = pm_runtime_get_sync(dev);
if (ret) {
DRM_ERROR("Failed to runtime PM enable on DSI%d\n", dsi->port);
-   return;
+   goto out;
}
 
if (debug_dump_regs) {
@@ -916,13 +916,13 @@ static void vc4_dsi_encoder_enable(struct drm_encoder 
*encoder)
ret = clk_prepare_enable(dsi->escape_clock);
if (ret) {
DRM_ERROR("Failed to turn on DSI escape clock: %d\n", ret);
-   return;
+   goto out;
}
 
ret = clk_prepare_enable(dsi->pll_phy_clock);
if (ret) {
DRM_ERROR("Failed to turn on DSI PLL: %d\n", ret);
-   return;
+   goto out;
}
 
hs_clock = clk_get_rate(dsi->pll_phy_clock);
@@ -944,7 +944,7 @@ static void vc4_dsi_encoder_enable(struct drm_encoder 
*encoder)
ret = clk_prepare_enable(dsi->pixel_clock);
if (ret) {
DRM_ERROR("Failed to turn on DSI pixel clock: %d\n", ret);
-   return;
+   goto out;
}
 
/* How many ns one DSI unit interval is.  Note that the clock
@@ -1088,6 +1088,8 @@ static void vc4_dsi_encoder_enable(struct drm_encoder 
*encoder)
dev_info(>pdev->dev, "DSI regs after:\n");
drm_print_regset32(, >regset);
}
+out:
+   pm_runtime_put(dev);
 }
 
 static ssize_t vc4_dsi_host_transfer(struct mipi_dsi_host *host,
-- 
2.17.1

[PATCH] drm/etnaviv: fix ref count leak via pm_runtime_get_sync

[Navid Emamdoost]

in etnaviv_gpu_submit, etnaviv_gpu_recover_hang, etnaviv_gpu_debugfs,
and etnaviv_gpu_init the call to pm_runtime_get_sync increments the
counter even in case of failure, leading to incorrect ref count.
In case of failure, decrement the ref count before returning.

Signed-off-by: Navid Emamdoost 
---
 drivers/gpu/drm/etnaviv/etnaviv_gpu.c | 14 ++
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c 
b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
index a31eeff2b297..16f5bc65771a 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
@@ -722,7 +722,7 @@ int etnaviv_gpu_init(struct etnaviv_gpu *gpu)
ret = pm_runtime_get_sync(gpu->dev);
if (ret < 0) {
dev_err(gpu->dev, "Failed to enable GPU power domain\n");
-   return ret;
+   goto pm_put;
}
 
etnaviv_hw_identify(gpu);
@@ -819,6 +819,7 @@ int etnaviv_gpu_init(struct etnaviv_gpu *gpu)
 
 fail:
pm_runtime_mark_last_busy(gpu->dev);
+pm_put:
pm_runtime_put_autosuspend(gpu->dev);
 
return ret;
@@ -859,7 +860,7 @@ int etnaviv_gpu_debugfs(struct etnaviv_gpu *gpu, struct 
seq_file *m)
 
ret = pm_runtime_get_sync(gpu->dev);
if (ret < 0)
-   return ret;
+   goto pm_put;
 
dma_lo = gpu_read(gpu, VIVS_FE_DMA_LOW);
dma_hi = gpu_read(gpu, VIVS_FE_DMA_HIGH);
@@ -1003,6 +1004,7 @@ int etnaviv_gpu_debugfs(struct etnaviv_gpu *gpu, struct 
seq_file *m)
ret = 0;
 
pm_runtime_mark_last_busy(gpu->dev);
+pm_put:
pm_runtime_put_autosuspend(gpu->dev);
 
return ret;
@@ -1016,7 +1018,7 @@ void etnaviv_gpu_recover_hang(struct etnaviv_gpu *gpu)
dev_err(gpu->dev, "recover hung GPU!\n");
 
if (pm_runtime_get_sync(gpu->dev) < 0)
-   return;
+   goto pm_put;
 
mutex_lock(>lock);
 
@@ -1035,6 +1037,7 @@ void etnaviv_gpu_recover_hang(struct etnaviv_gpu *gpu)
 
mutex_unlock(>lock);
pm_runtime_mark_last_busy(gpu->dev);
+pm_put:
pm_runtime_put_autosuspend(gpu->dev);
 }
 
@@ -1308,8 +1311,10 @@ struct dma_fence *etnaviv_gpu_submit(struct 
etnaviv_gem_submit *submit)
 
if (!submit->runtime_resumed) {
ret = pm_runtime_get_sync(gpu->dev);
-   if (ret < 0)
+   if (ret < 0) {
+   pm_runtime_put(gpu->dev);
return NULL;
+   }
submit->runtime_resumed = true;
}
 
@@ -1326,6 +1331,7 @@ struct dma_fence *etnaviv_gpu_submit(struct 
etnaviv_gem_submit *submit)
ret = event_alloc(gpu, nr_events, event);
if (ret) {
DRM_ERROR("no free events\n");
+   pm_runtime_put(gpu->dev);
return NULL;
}
 
-- 
2.17.1

[PATCH] drm/panfrost: perfcnt: fix ref count leak in panfrost_perfcnt_enable_locked

[Navid Emamdoost]

in panfrost_perfcnt_enable_locked, pm_runtime_get_sync is called which
increments the counter even in case of failure, leading to incorrect
ref count. In case of failure, decrement the ref count before returning.

Signed-off-by: Navid Emamdoost 
---
 drivers/gpu/drm/panfrost/panfrost_perfcnt.c | 10 +++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/panfrost/panfrost_perfcnt.c 
b/drivers/gpu/drm/panfrost/panfrost_perfcnt.c
index 6913578d5aa7..92c64b20eb29 100644
--- a/drivers/gpu/drm/panfrost/panfrost_perfcnt.c
+++ b/drivers/gpu/drm/panfrost/panfrost_perfcnt.c
@@ -83,11 +83,13 @@ static int panfrost_perfcnt_enable_locked(struct 
panfrost_device *pfdev,
 
ret = pm_runtime_get_sync(pfdev->dev);
if (ret < 0)
-   return ret;
+   goto err_put_pm;
 
bo = drm_gem_shmem_create(pfdev->ddev, perfcnt->bosize);
-   if (IS_ERR(bo))
-   return PTR_ERR(bo);
+   if (IS_ERR(bo)) {
+   ret = PTR_ERR(bo);
+   goto err_put_pm;
+   }
 
/* Map the perfcnt buf in the address space attached to file_priv. */
ret = panfrost_gem_open(>base, file_priv);
@@ -168,6 +170,8 @@ static int panfrost_perfcnt_enable_locked(struct 
panfrost_device *pfdev,
panfrost_gem_close(>base, file_priv);
 err_put_bo:
drm_gem_object_put_unlocked(>base);
+err_put_pm:
+   pm_runtime_put(pfdev->dev);
return ret;
 }
 
-- 
2.17.1

[PATCH] drm/panfrost: fix ref count leak in panfrost_job_hw_submit

[Navid Emamdoost]

in panfrost_job_hw_submit, pm_runtime_get_sync is called which
increments the counter even in case of failure, leading to incorrect
ref count. In case of failure, decrement the ref count before returning.

Signed-off-by: Navid Emamdoost 
---
 drivers/gpu/drm/panfrost/panfrost_job.c | 8 +---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/panfrost/panfrost_job.c 
b/drivers/gpu/drm/panfrost/panfrost_job.c
index 7914b1570841..89ac84667eb1 100644
--- a/drivers/gpu/drm/panfrost/panfrost_job.c
+++ b/drivers/gpu/drm/panfrost/panfrost_job.c
@@ -147,11 +147,10 @@ static void panfrost_job_hw_submit(struct panfrost_job 
*job, int js)
 
ret = pm_runtime_get_sync(pfdev->dev);
if (ret < 0)
-   return;
+   goto out;
 
if (WARN_ON(job_read(pfdev, JS_COMMAND_NEXT(js {
-   pm_runtime_put_sync_autosuspend(pfdev->dev);
-   return;
+   goto out;
}
 
cfg = panfrost_mmu_as_get(pfdev, >file_priv->mmu);
@@ -184,6 +183,9 @@ static void panfrost_job_hw_submit(struct panfrost_job 
*job, int js)
job, js, jc_head);
 
job_write(pfdev, JS_COMMAND_NEXT(js), JS_COMMAND_START);
+out:
+   pm_runtime_put_sync_autosuspend(pfdev->dev);
+   return;
 }
 
 static void panfrost_acquire_object_fences(struct drm_gem_object **bos,
-- 
2.17.1

[PATCH] drm/exynos: fix ref count leak in mic_pre_enable

[Navid Emamdoost]

in mic_pre_enable, pm_runtime_get_sync is called which
increments the counter even in case of failure, leading to incorrect
ref count. In case of failure, decrement the ref count before returning.

Signed-off-by: Navid Emamdoost 
---
 drivers/gpu/drm/exynos/exynos_drm_mic.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/exynos/exynos_drm_mic.c 
b/drivers/gpu/drm/exynos/exynos_drm_mic.c
index a86abc173605..69ff74c2ceb5 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_mic.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_mic.c
@@ -270,7 +270,7 @@ static void mic_pre_enable(struct drm_bridge *bridge)
 
ret = pm_runtime_get_sync(mic->dev);
if (ret < 0)
-   goto unlock;
+   goto turn_off;
 
mic_set_path(mic, 1);
 
-- 
2.17.1

[PATCH] Input: stmfts: fix ref count leak in stmfts_input_open

[Navid Emamdoost]

in stmfts_input_open, pm_runtime_get_sync is called which
increments the counter even in case of failure, leading to incorrect
ref count. In case of failure, decrement the ref count before returning.

Signed-off-by: Navid Emamdoost 
---
 drivers/input/touchscreen/stmfts.c | 7 +--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/input/touchscreen/stmfts.c 
b/drivers/input/touchscreen/stmfts.c
index b6f95f20f924..1ef282d7cc14 100644
--- a/drivers/input/touchscreen/stmfts.c
+++ b/drivers/input/touchscreen/stmfts.c
@@ -339,11 +339,11 @@ static int stmfts_input_open(struct input_dev *dev)
 
err = pm_runtime_get_sync(>client->dev);
if (err < 0)
-   return err;
+   goto out;
 
err = i2c_smbus_write_byte(sdata->client, STMFTS_MS_MT_SENSE_ON);
if (err)
-   return err;
+   goto out;
 
mutex_lock(>mutex);
sdata->running = true;
@@ -367,6 +367,9 @@ static int stmfts_input_open(struct input_dev *dev)
}
 
return 0;
+out:
+   pm_runtime_put(>client->dev);
+   return err;
 }
 
 static void stmfts_input_close(struct input_dev *dev)
-- 
2.17.1

[PATCH] Input: bma150: fix ref count leak in bma150_open

[Navid Emamdoost]

in bma150_open, pm_runtime_get_sync is called which
increments the counter even in case of failure, leading to incorrect
ref count. In case of failure, decrement the ref count before returning.

Signed-off-by: Navid Emamdoost 
---
 drivers/input/misc/bma150.c | 7 +--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/input/misc/bma150.c b/drivers/input/misc/bma150.c
index a9d984da95f3..e2f1b05fcb2a 100644
--- a/drivers/input/misc/bma150.c
+++ b/drivers/input/misc/bma150.c
@@ -348,7 +348,7 @@ static int bma150_open(struct input_dev *input)
 
error = pm_runtime_get_sync(>client->dev);
if (error < 0 && error != -ENOSYS)
-   return error;
+   goto out;
 
/*
 * See if runtime PM woke up the device. If runtime PM
@@ -357,10 +357,13 @@ static int bma150_open(struct input_dev *input)
if (bma150->mode != BMA150_MODE_NORMAL) {
error = bma150_set_mode(bma150, BMA150_MODE_NORMAL);
if (error)
-   return error;
+   goto out;
}
 
return 0;
+out:
+   pm_runtime_put(>client->dev);
+   return error;
 }
 
 static void bma150_close(struct input_dev *input)
-- 
2.17.1

[PATCH] Input: omap4-keypad: fix ref count leak if pm_runtime_get_sync fails

[Navid Emamdoost]

calling pm_runtime_get_sync increments the counter even in case of
failure, leading to incorrect ref count. In case of failure, decrement
the ref count before returning.

Signed-off-by: Navid Emamdoost 
---
 drivers/input/keyboard/omap4-keypad.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/input/keyboard/omap4-keypad.c 
b/drivers/input/keyboard/omap4-keypad.c
index 94c94d7f5155..35ac788d9fa0 100644
--- a/drivers/input/keyboard/omap4-keypad.c
+++ b/drivers/input/keyboard/omap4-keypad.c
@@ -280,7 +280,7 @@ static int omap4_keypad_probe(struct platform_device *pdev)
error = pm_runtime_get_sync(>dev);
if (error) {
dev_err(>dev, "pm_runtime_get_sync() failed\n");
-   goto err_unmap;
+   goto err_pm_put_sync;
}
rev = __raw_readl(keypad_data->base + OMAP4_KBD_REVISION);
rev &= 0x03 << 30;
@@ -372,7 +372,6 @@ static int omap4_keypad_probe(struct platform_device *pdev)
input_free_device(input_dev);
 err_pm_put_sync:
pm_runtime_put_sync(>dev);
-err_unmap:
iounmap(keypad_data->base);
 err_release_mem:
release_mem_region(res->start, resource_size(res));
-- 
2.17.1

[PATCH] net: macb: fix ref count leaking via pm_runtime_get_sync

[Navid Emamdoost]

in macb_mdio_write, macb_mdio_read, and at91ether_open,
pm_runtime_get_sync is called which increments the counter even in case of
failure, leading to incorrect ref count.
In case of failure, decrement the ref count before returning.

Signed-off-by: Navid Emamdoost 
---
 drivers/net/ethernet/cadence/macb_main.c | 15 ++-
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ethernet/cadence/macb_main.c 
b/drivers/net/ethernet/cadence/macb_main.c
index a0e8c5bbabc0..3646ab5a1e83 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -335,7 +335,7 @@ static int macb_mdio_read(struct mii_bus *bus, int mii_id, 
int regnum)
 
status = pm_runtime_get_sync(>pdev->dev);
if (status < 0)
-   goto mdio_pm_exit;
+   goto mdio_pm_put;
 
status = macb_mdio_wait_for_idle(bp);
if (status < 0)
@@ -374,6 +374,7 @@ static int macb_mdio_read(struct mii_bus *bus, int mii_id, 
int regnum)
 
 mdio_read_exit:
pm_runtime_mark_last_busy(>pdev->dev);
+mdio_pm_put:
pm_runtime_put_autosuspend(>pdev->dev);
 mdio_pm_exit:
return status;
@@ -387,7 +388,7 @@ static int macb_mdio_write(struct mii_bus *bus, int mii_id, 
int regnum,
 
status = pm_runtime_get_sync(>pdev->dev);
if (status < 0)
-   goto mdio_pm_exit;
+   goto mdio_pm_put;
 
status = macb_mdio_wait_for_idle(bp);
if (status < 0)
@@ -426,6 +427,7 @@ static int macb_mdio_write(struct mii_bus *bus, int mii_id, 
int regnum,
 
 mdio_write_exit:
pm_runtime_mark_last_busy(>pdev->dev);
+mdio_pm_put:
pm_runtime_put_autosuspend(>pdev->dev);
 mdio_pm_exit:
return status;
@@ -3817,7 +3819,7 @@ static int at91ether_open(struct net_device *dev)
 
ret = pm_runtime_get_sync(>pdev->dev);
if (ret < 0)
-   return ret;
+   goto out;
 
/* Clear internal statistics */
ctl = macb_readl(lp, NCR);
@@ -3827,7 +3829,7 @@ static int at91ether_open(struct net_device *dev)
 
ret = at91ether_start(dev);
if (ret)
-   return ret;
+   goto out;
 
/* Enable MAC interrupts */
macb_writel(lp, IER, MACB_BIT(RCOMP)|
@@ -3840,11 +3842,14 @@ static int at91ether_open(struct net_device *dev)
 
ret = macb_phylink_connect(lp);
if (ret)
-   return ret;
+   goto out;
 
netif_start_queue(dev);
 
return 0;
+out:
+   pm_runtime_put(>pdev->dev);
+   return ret;
 }
 
 /* Close the interface */
-- 
2.17.1

[PATCH] net: fec: fix ref count leaking when pm_runtime_get_sync fails

[Navid Emamdoost]

in fec_enet_mdio_read, fec_enet_mdio_write, fec_enet_get_regs,
fec_enet_open and fec_drv_remove, pm_runtime_get_sync is called which
increments the counter even in case of failure, leading to incorrect
ref count. In case of failure, decrement the ref count before returning.

Signed-off-by: Navid Emamdoost 
---
 drivers/net/ethernet/freescale/fec_main.c | 19 ++-
 1 file changed, 14 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ethernet/freescale/fec_main.c 
b/drivers/net/ethernet/freescale/fec_main.c
index dc6f8763a5d4..a33012b89cc9 100644
--- a/drivers/net/ethernet/freescale/fec_main.c
+++ b/drivers/net/ethernet/freescale/fec_main.c
@@ -1827,8 +1827,10 @@ static int fec_enet_mdio_read(struct mii_bus *bus, int 
mii_id, int regnum)
bool is_c45 = !!(regnum & MII_ADDR_C45);
 
ret = pm_runtime_get_sync(dev);
-   if (ret < 0)
+   if (ret < 0) {
+   pm_runtime_put_autosuspend(dev);
return ret;
+   }
 
reinit_completion(>mdio_done);
 
@@ -1893,8 +1895,10 @@ static int fec_enet_mdio_write(struct mii_bus *bus, int 
mii_id, int regnum,
bool is_c45 = !!(regnum & MII_ADDR_C45);
 
ret = pm_runtime_get_sync(dev);
-   if (ret < 0)
+   if (ret < 0) {
+   pm_runtime_put_autosuspend(dev);
return ret;
+   }
else
ret = 0;
 
@@ -2258,7 +2262,7 @@ static void fec_enet_get_regs(struct net_device *ndev,
 
ret = pm_runtime_get_sync(dev);
if (ret < 0)
-   return;
+   goto out;
 
regs->version = fec_enet_register_version;
 
@@ -2276,6 +2280,7 @@ static void fec_enet_get_regs(struct net_device *ndev,
}
 
pm_runtime_mark_last_busy(dev);
+out:
pm_runtime_put_autosuspend(dev);
 }
 
@@ -2952,8 +2957,10 @@ fec_enet_open(struct net_device *ndev)
bool reset_again;
 
ret = pm_runtime_get_sync(>pdev->dev);
-   if (ret < 0)
+   if (ret < 0) {
+   pm_runtime_put_autosuspend(>pdev->dev);
return ret;
+   }
 
pinctrl_pm_select_default_state(>pdev->dev);
ret = fec_enet_clk_enable(ndev, true);
@@ -3741,8 +3748,10 @@ fec_drv_remove(struct platform_device *pdev)
int ret;
 
ret = pm_runtime_get_sync(>dev);
-   if (ret < 0)
+   if (ret < 0) {
+   pm_runtime_put_noidle(>dev);
return ret;
+   }
 
cancel_work_sync(>tx_timeout_work);
fec_ptp_stop(pdev);
-- 
2.17.1

Re: [PATCH] io: pressure: zpa2326: handle pm_runtime_get_sync failure

[Navid Emamdoost]

On Sat, Jun 6, 2020 at 2:29 PM Andy Shevchenko
 wrote:
>
>
>
> On Saturday, June 6, 2020, Jonathan Cameron  wrote:
>>
>> On Thu,  4 Jun 2020 21:44:44 -0500
>> Navid Emamdoost  wrote:
>>
>> > Calling pm_runtime_get_sync increments the counter even in case of
>> > failure, causing incorrect ref count. Call pm_runtime_put if
>> > pm_runtime_get_sync fails.
>> >
>> > Signed-off-by: Navid Emamdoost 
>>
>> Hi Navid,
>>
>> This looks to be a fix, be it for a case that we are hopefully
>> unlikely to ever hit.  Please could you add an appropriate
>> Fixes tag so we can work out how far to backport it?
>>
>> Patch looks good to me so if you just reply with a suitable
>> tag I can add it whilst applying.

Hi Jonathan,
Here is the fixes tag:

Fixes: 03b262f2bbf4 ("iio:pressure: initial zpa2326 barometer support")

>
>
>
> Should not be "iio: ..." in the prefix?

Yes! It should be "iio" in the patch name.


>>
>>
>> Thanks,
>>
>> Jonathan
>>
>> > ---
>> >  drivers/iio/pressure/zpa2326.c | 4 +++-
>> >  1 file changed, 3 insertions(+), 1 deletion(-)
>> >
>> > diff --git a/drivers/iio/pressure/zpa2326.c 
>> > b/drivers/iio/pressure/zpa2326.c
>> > index 99dfe33ee402..245f2e2d412b 100644
>> > --- a/drivers/iio/pressure/zpa2326.c
>> > +++ b/drivers/iio/pressure/zpa2326.c
>> > @@ -664,8 +664,10 @@ static int zpa2326_resume(const struct iio_dev 
>> > *indio_dev)
>> >   int err;
>> >
>> >   err = pm_runtime_get_sync(indio_dev->dev.parent);
>> > - if (err < 0)
>> > + if (err < 0) {
>> > + pm_runtime_put(indio_dev->dev.parent);
>> >   return err;
>> > + }
>> >
>> >   if (err > 0) {
>> >   /*
>>
>
>
> --
> With Best Regards,
> Andy Shevchenko
>
>


-- 
Navid.

Re: [PATCH] spi: tegra114: missing put on pm_runtime_get_sync failure

[Navid Emamdoost]

On Fri, Jun 5, 2020 at 1:09 AM Jon Hunter  wrote:
>
>
> On 02/06/2020 05:55, Navid Emamdoost wrote:
> > the call to pm_runtime_get_sync increments the counter even
> > in case of failure leading to incorrect ref count.
> > Call pm_runtime_put if pm_runtime_get_sync fails.
> >
> > Signed-off-by: Navid Emamdoost 
> > ---
> >  drivers/spi/spi-tegra114.c | 3 +++
> >  1 file changed, 3 insertions(+)
> >
> > diff --git a/drivers/spi/spi-tegra114.c b/drivers/spi/spi-tegra114.c
> > index 83edabdb41ad..dccd2ac1a975 100644
> > --- a/drivers/spi/spi-tegra114.c
> > +++ b/drivers/spi/spi-tegra114.c
> > @@ -974,6 +974,7 @@ static int tegra_spi_setup(struct spi_device *spi)
> >   dev_err(tspi->dev, "pm runtime failed, e = %d\n", ret);
> >   if (cdata)
> >   tegra_spi_cleanup(spi);
> > + pm_runtime_put(tspi->dev);
> >   return ret;
> >   }
> >
> > @@ -1398,6 +1399,7 @@ static int tegra_spi_probe(struct platform_device 
> > *pdev)
> >   ret = pm_runtime_get_sync(>dev);
> >   if (ret < 0) {
> >   dev_err(>dev, "pm runtime get failed, e = %d\n", ret);
> > + pm_runtime_put(>dev);
> >   goto exit_pm_disable;
> >   }
>
> I am wondering if it is better we use put_sync() here to ensure that
> this happens before we exit the probe.

To be honest I am not sure when to use different flavors of
pm_runtime_put (like pm_runtime_put_noidle,
pm_runtime_put_autosuspend, pm_runtime_put_sync, pm_runtime_put,
pm_runtime_put_sync_suspend). I'd appreciate it if you could give me a
pointer on how to decide on this.

>
> Jon
>
> --
> nvpublic



-- 
Navid.

[PATCH] can: xilinx_can: handle failure cases of pm_runtime_get_sync

[Navid Emamdoost]

Calling pm_runtime_get_sync increments the counter even in case of
failure, causing incorrect ref count. Call pm_runtime_put if
pm_runtime_get_sync fails.

Signed-off-by: Navid Emamdoost 
---
 drivers/net/can/xilinx_can.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/net/can/xilinx_can.c b/drivers/net/can/xilinx_can.c
index c1dbab8c896d..748ff70f6a7b 100644
--- a/drivers/net/can/xilinx_can.c
+++ b/drivers/net/can/xilinx_can.c
@@ -1391,7 +1391,7 @@ static int xcan_open(struct net_device *ndev)
if (ret < 0) {
netdev_err(ndev, "%s: pm_runtime_get failed(%d)\n",
   __func__, ret);
-   return ret;
+   goto err;
}
 
ret = request_irq(ndev->irq, xcan_interrupt, priv->irq_flags,
@@ -1475,6 +1475,7 @@ static int xcan_get_berr_counter(const struct net_device 
*ndev,
if (ret < 0) {
netdev_err(ndev, "%s: pm_runtime_get failed(%d)\n",
   __func__, ret);
+   pm_runtime_put(priv->dev);
return ret;
}
 
@@ -1789,7 +1790,7 @@ static int xcan_probe(struct platform_device *pdev)
if (ret < 0) {
netdev_err(ndev, "%s: pm_runtime_get failed(%d)\n",
   __func__, ret);
-   goto err_pmdisable;
+   goto err_disableclks;
}
 
if (priv->read_reg(priv, XCAN_SR_OFFSET) != XCAN_SR_CONFIG_MASK) {
@@ -1824,7 +1825,6 @@ static int xcan_probe(struct platform_device *pdev)
 
 err_disableclks:
pm_runtime_put(priv->dev);
-err_pmdisable:
pm_runtime_disable(>dev);
 err_free:
free_candev(ndev);
-- 
2.17.1

[PATCH] wlcore: mesh: handle failure case of pm_runtime_get_sync

[Navid Emamdoost]

Calling pm_runtime_get_sync increments the counter even in case of
failure, causing incorrect ref count. Call pm_runtime_put if
pm_runtime_get_sync fails.

Signed-off-by: Navid Emamdoost 
---
 drivers/net/wireless/ti/wlcore/main.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/net/wireless/ti/wlcore/main.c 
b/drivers/net/wireless/ti/wlcore/main.c
index f140f7d7f553..c7e4f5a80b9e 100644
--- a/drivers/net/wireless/ti/wlcore/main.c
+++ b/drivers/net/wireless/ti/wlcore/main.c
@@ -3662,8 +3662,10 @@ void wlcore_regdomain_config(struct wl1271 *wl)
goto out;
 
ret = pm_runtime_get_sync(wl->dev);
-   if (ret < 0)
+   if (ret < 0) {
+   pm_runtime_put_autosuspend(wl->dev);
goto out;
+   }
 
ret = wlcore_cmd_regdomain_config_locked(wl);
if (ret < 0) {
-- 
2.17.1

[PATCH] PCI: rcar: handle the failure case of pm_runtime_get_sync

[Navid Emamdoost]

Calling pm_runtime_get_sync increments the counter even in case of
failure, causing incorrect ref count. Call pm_runtime_put if
pm_runtime_get_sync fails.

Signed-off-by: Navid Emamdoost 
---
 drivers/pci/controller/pcie-rcar.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/pci/controller/pcie-rcar.c 
b/drivers/pci/controller/pcie-rcar.c
index 759c6542c5c8..6b4181c0710e 100644
--- a/drivers/pci/controller/pcie-rcar.c
+++ b/drivers/pci/controller/pcie-rcar.c
@@ -1137,7 +1137,7 @@ static int rcar_pcie_probe(struct platform_device *pdev)
err = pm_runtime_get_sync(pcie->dev);
if (err < 0) {
dev_err(pcie->dev, "pm_runtime_get_sync failed\n");
-   goto err_pm_disable;
+   goto err_pm_put;
}
 
err = rcar_pcie_get_resources(pcie);
@@ -1208,8 +1208,6 @@ static int rcar_pcie_probe(struct platform_device *pdev)
 
 err_pm_put:
pm_runtime_put(dev);
-
-err_pm_disable:
pm_runtime_disable(dev);
pci_free_resource_list(>resources);
 
-- 
2.17.1

[PATCH] PCI: dwc: pci-dra7xx: handle failure case of pm_runtime_get_sync

[Navid Emamdoost]

Calling pm_runtime_get_sync increments the counter even in case of
failure, causing incorrect ref count. Call pm_runtime_put if
pm_runtime_get_sync fails.

Signed-off-by: Navid Emamdoost 
---
 drivers/pci/controller/dwc/pci-dra7xx.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/pci/controller/dwc/pci-dra7xx.c 
b/drivers/pci/controller/dwc/pci-dra7xx.c
index 3b0e58f2de58..83986f5f2be7 100644
--- a/drivers/pci/controller/dwc/pci-dra7xx.c
+++ b/drivers/pci/controller/dwc/pci-dra7xx.c
@@ -932,7 +932,7 @@ static int __init dra7xx_pcie_probe(struct platform_device 
*pdev)
ret = pm_runtime_get_sync(dev);
if (ret < 0) {
dev_err(dev, "pm_runtime_get_sync failed\n");
-   goto err_get_sync;
+   goto err_gpio;
}
 
reset = devm_gpiod_get_optional(dev, NULL, GPIOD_OUT_HIGH);
@@ -1001,8 +1001,6 @@ static int __init dra7xx_pcie_probe(struct 
platform_device *pdev)
 
 err_gpio:
pm_runtime_put(dev);
-
-err_get_sync:
pm_runtime_disable(dev);
dra7xx_pcie_disable_phy(dra7xx);
 
-- 
2.17.1

[PATCH] PCI: qcom: handle pm_runtime_get_sync failure case

[Navid Emamdoost]

Calling pm_runtime_get_sync increments the counter even in case of
failure, causing incorrect ref count. Call pm_runtime_put if
pm_runtime_get_sync fails.

Signed-off-by: Navid Emamdoost 
---
 drivers/pci/controller/dwc/pcie-qcom.c | 6 ++
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/pci/controller/dwc/pcie-qcom.c 
b/drivers/pci/controller/dwc/pcie-qcom.c
index 138e1a2d21cc..48c434e6e915 100644
--- a/drivers/pci/controller/dwc/pcie-qcom.c
+++ b/drivers/pci/controller/dwc/pcie-qcom.c
@@ -1339,10 +1339,8 @@ static int qcom_pcie_probe(struct platform_device *pdev)
 
pm_runtime_enable(dev);
ret = pm_runtime_get_sync(dev);
-   if (ret < 0) {
-   pm_runtime_disable(dev);
-   return ret;
-   }
+   if (ret < 0)
+   goto err_pm_runtime_put;
 
pci->dev = dev;
pci->ops = _pcie_ops;
-- 
2.17.1

[PATCH] PCI: tegra: handle failure case of pm_runtime_get_sync

[Navid Emamdoost]

Calling pm_runtime_get_sync increments the counter even in case of
failure, causing incorrect ref count. Call pm_runtime_put if
pm_runtime_get_sync fails.

Signed-off-by: Navid Emamdoost 
---
 drivers/pci/controller/pci-tegra.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/pci/controller/pci-tegra.c 
b/drivers/pci/controller/pci-tegra.c
index 3e64ba6a36a8..3d4b448fd8df 100644
--- a/drivers/pci/controller/pci-tegra.c
+++ b/drivers/pci/controller/pci-tegra.c
@@ -2712,6 +2712,7 @@ static int tegra_pcie_probe(struct platform_device *pdev)
err = pm_runtime_get_sync(pcie->dev);
if (err < 0) {
dev_err(dev, "fail to enable pcie controller: %d\n", err);
+   pm_runtime_put_sync(pcie->dev);
goto teardown_msi;
}
 
-- 
2.17.1

[PATCH] sata_rcar: handle pm_runtime_get_sync failure cases

[Navid Emamdoost]

Calling pm_runtime_get_sync increments the counter even in case of
failure, causing incorrect ref count. Call pm_runtime_put if
pm_runtime_get_sync fails.

Signed-off-by: Navid Emamdoost 
---
 drivers/ata/sata_rcar.c | 11 +++
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/drivers/ata/sata_rcar.c b/drivers/ata/sata_rcar.c
index 980aacdbcf3b..141ac600b64c 100644
--- a/drivers/ata/sata_rcar.c
+++ b/drivers/ata/sata_rcar.c
@@ -907,7 +907,7 @@ static int sata_rcar_probe(struct platform_device *pdev)
pm_runtime_enable(dev);
ret = pm_runtime_get_sync(dev);
if (ret < 0)
-   goto err_pm_disable;
+   goto err_pm_put;
 
host = ata_host_alloc(dev, 1);
if (!host) {
@@ -937,7 +937,6 @@ static int sata_rcar_probe(struct platform_device *pdev)
 
 err_pm_put:
pm_runtime_put(dev);
-err_pm_disable:
pm_runtime_disable(dev);
return ret;
 }
@@ -991,8 +990,10 @@ static int sata_rcar_resume(struct device *dev)
int ret;
 
ret = pm_runtime_get_sync(dev);
-   if (ret < 0)
+   if (ret < 0) {
+   pm_runtime_put(dev);
return ret;
+   }
 
if (priv->type == RCAR_GEN3_SATA) {
sata_rcar_init_module(priv);
@@ -1017,8 +1018,10 @@ static int sata_rcar_restore(struct device *dev)
int ret;
 
ret = pm_runtime_get_sync(dev);
-   if (ret < 0)
+   if (ret < 0) {
+   pm_runtime_put(dev);
return ret;
+   }
 
sata_rcar_setup_port(host);
 
-- 
2.17.1

[PATCH] gpio: arizona: put pm_runtime in case of failure

[Navid Emamdoost]

Calling pm_runtime_get_sync increments the counter even in case of
failure, causing incorrect ref count if pm_runtime_put is not called in
error handling paths. Call pm_runtime_put if pm_runtime_get_sync fails.

Signed-off-by: Navid Emamdoost 
---
 drivers/gpio/gpio-arizona.c | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/gpio/gpio-arizona.c b/drivers/gpio/gpio-arizona.c
index 7520a13b4c7c..5bda38e0780f 100644
--- a/drivers/gpio/gpio-arizona.c
+++ b/drivers/gpio/gpio-arizona.c
@@ -64,6 +64,7 @@ static int arizona_gpio_get(struct gpio_chip *chip, unsigned 
offset)
ret = pm_runtime_get_sync(chip->parent);
if (ret < 0) {
dev_err(chip->parent, "Failed to resume: %d\n", ret);
+   pm_runtime_put_autosuspend(chip->parent);
return ret;
}
 
@@ -72,12 +73,15 @@ static int arizona_gpio_get(struct gpio_chip *chip, 
unsigned offset)
if (ret < 0) {
dev_err(chip->parent, "Failed to drop cache: %d\n",
ret);
+   pm_runtime_put_autosuspend(chip->parent);
return ret;
}
 
ret = regmap_read(arizona->regmap, reg, );
-   if (ret < 0)
+   if (ret < 0) {
+   pm_runtime_put_autosuspend(chip->parent);
return ret;
+   }
 
pm_runtime_mark_last_busy(chip->parent);
pm_runtime_put_autosuspend(chip->parent);
-- 
2.17.1

[PATCH] gpio: arizona: handle pm_runtime_get_sync failure case

[Navid Emamdoost]

Calling pm_runtime_get_sync increments the counter even in case of
failure, causing incorrect ref count. Call pm_runtime_put if
pm_runtime_get_sync fails.

Signed-off-by: Navid Emamdoost 
---
 drivers/gpio/gpio-arizona.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpio/gpio-arizona.c b/drivers/gpio/gpio-arizona.c
index 5640efe5e750..7520a13b4c7c 100644
--- a/drivers/gpio/gpio-arizona.c
+++ b/drivers/gpio/gpio-arizona.c
@@ -106,6 +106,7 @@ static int arizona_gpio_direction_out(struct gpio_chip 
*chip,
ret = pm_runtime_get_sync(chip->parent);
if (ret < 0) {
dev_err(chip->parent, "Failed to resume: %d\n", ret);
+   pm_runtime_put(chip->parent);
return ret;
}
}
-- 
2.17.1

[PATCH] gpio: rcar: handle pm_runtime_get_sync failure case

[Navid Emamdoost]

Calling pm_runtime_get_sync increments the counter even in case of
failure, causing incorrect ref count. Call pm_runtime_put if
pm_runtime_get_sync fails.

Signed-off-by: Navid Emamdoost 
---
 drivers/gpio/gpio-rcar.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/gpio/gpio-rcar.c b/drivers/gpio/gpio-rcar.c
index 7284473c9fe3..eac1582c70da 100644
--- a/drivers/gpio/gpio-rcar.c
+++ b/drivers/gpio/gpio-rcar.c
@@ -250,8 +250,10 @@ static int gpio_rcar_request(struct gpio_chip *chip, 
unsigned offset)
int error;
 
error = pm_runtime_get_sync(p->dev);
-   if (error < 0)
+   if (error < 0) {
+   pm_runtime_put(p->dev);
return error;
+   }
 
error = pinctrl_gpio_request(chip->base + offset);
if (error)
-- 
2.17.1

[PATCH] mfd: arizona: handle pm_runtime_get_sync failure case

[Navid Emamdoost]

Calling pm_runtime_get_sync increments the counter even in case of
failure, causing incorrect ref count. Call pm_runtime_put_sync if
pm_runtime_get_sync fails.

Signed-off-by: Navid Emamdoost 
---
 drivers/mfd/arizona-core.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/mfd/arizona-core.c b/drivers/mfd/arizona-core.c
index f73cf76d1373..5b3191b6534a 100644
--- a/drivers/mfd/arizona-core.c
+++ b/drivers/mfd/arizona-core.c
@@ -46,8 +46,10 @@ int arizona_clk32k_enable(struct arizona *arizona)
switch (arizona->pdata.clk32k_src) {
case ARIZONA_32KZ_MCLK1:
ret = pm_runtime_get_sync(arizona->dev);
-   if (ret != 0)
+   if (ret != 0) {
+   pm_runtime_put_sync(arizona->dev);
goto err_ref;
+   }
ret = clk_prepare_enable(arizona->mclk[ARIZONA_MCLK1]);
if (ret != 0) {
pm_runtime_put_sync(arizona->dev);
-- 
2.17.1

[PATCH] io: pressure: zpa2326: handle pm_runtime_get_sync failure

[Navid Emamdoost]

Calling pm_runtime_get_sync increments the counter even in case of
failure, causing incorrect ref count. Call pm_runtime_put if
pm_runtime_get_sync fails.

Signed-off-by: Navid Emamdoost 
---
 drivers/iio/pressure/zpa2326.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/iio/pressure/zpa2326.c b/drivers/iio/pressure/zpa2326.c
index 99dfe33ee402..245f2e2d412b 100644
--- a/drivers/iio/pressure/zpa2326.c
+++ b/drivers/iio/pressure/zpa2326.c
@@ -664,8 +664,10 @@ static int zpa2326_resume(const struct iio_dev *indio_dev)
int err;
 
err = pm_runtime_get_sync(indio_dev->dev.parent);
-   if (err < 0)
+   if (err < 0) {
+   pm_runtime_put(indio_dev->dev.parent);
return err;
+   }
 
if (err > 0) {
/*
-- 
2.17.1

[PATCH] dmaengine: sprd: handle the failure cases of pm_runtime_get_sync

[Navid Emamdoost]

Calling pm_runtime_get_sync increments the counter even in case of
failure, causing incorrect ref count. Call pm_runtime_put if
pm_runtime_get_sync fails.

Signed-off-by: Navid Emamdoost 
---
 drivers/dma/sprd-dma.c | 8 ++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/dma/sprd-dma.c b/drivers/dma/sprd-dma.c
index 0ef5ca81ba4d..0ba4fe1a1905 100644
--- a/drivers/dma/sprd-dma.c
+++ b/drivers/dma/sprd-dma.c
@@ -1204,8 +1204,10 @@ static int sprd_dma_probe(struct platform_device *pdev)
pm_runtime_enable(>dev);
 
ret = pm_runtime_get_sync(>dev);
-   if (ret < 0)
+   if (ret < 0) {
+   pm_runtime_put_noidle(>dev);
goto err_rpm;
+   }
 
ret = dma_async_device_register(>dma_dev);
if (ret < 0) {
@@ -1239,8 +1241,10 @@ static int sprd_dma_remove(struct platform_device *pdev)
int ret;
 
ret = pm_runtime_get_sync(>dev);
-   if (ret < 0)
+   if (ret < 0) {
+   pm_runtime_put_noidle(>dev);
return ret;
+   }
 
/* explicitly free the irq */
if (sdev->irq > 0)
-- 
2.17.1

[PATCH] dmaengine: rcar-dmac: handle pm_runtime_get_sync failure

[Navid Emamdoost]

Calling pm_runtime_get_sync increments the counter even in case of
failure, causing incorrect ref count. Call pm_runtime_put if
pm_runtime_get_sync fails.

Signed-off-by: Navid Emamdoost 
---
 drivers/dma/sh/rcar-dmac.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/dma/sh/rcar-dmac.c b/drivers/dma/sh/rcar-dmac.c
index 59b36ab5d684..dd7ca67c93ed 100644
--- a/drivers/dma/sh/rcar-dmac.c
+++ b/drivers/dma/sh/rcar-dmac.c
@@ -1879,6 +1879,7 @@ static int rcar_dmac_probe(struct platform_device *pdev)
ret = pm_runtime_get_sync(>dev);
if (ret < 0) {
dev_err(>dev, "runtime PM get sync failed (%d)\n", ret);
+   pm_runtime_put(>dev);
return ret;
}
 
-- 
2.17.1

[PATCH] dmaengine: sh: usb-dmac: handle pm_runtime_get_sync failure

[Navid Emamdoost]

Calling pm_runtime_get_sync increments the counter even in case of
failure, causing incorrect ref count. Call pm_runtime_put if
pm_runtime_get_sync fails.

Signed-off-by: Navid Emamdoost 
---
 drivers/dma/sh/usb-dmac.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/dma/sh/usb-dmac.c b/drivers/dma/sh/usb-dmac.c
index b218a013c260..43511434c90c 100644
--- a/drivers/dma/sh/usb-dmac.c
+++ b/drivers/dma/sh/usb-dmac.c
@@ -797,7 +797,7 @@ static int usb_dmac_probe(struct platform_device *pdev)
ret = pm_runtime_get_sync(>dev);
if (ret < 0) {
dev_err(>dev, "runtime PM get sync failed (%d)\n", ret);
-   goto error_pm;
+   goto error_pm_get;
}
 
ret = usb_dmac_init(dmac);
@@ -853,6 +853,7 @@ static int usb_dmac_probe(struct platform_device *pdev)
 
 error:
of_dma_controller_free(pdev->dev.of_node);
+error_pm_get:
pm_runtime_put(>dev);
 error_pm:
pm_runtime_disable(>dev);
-- 
2.17.1

[PATCH] dmaengine: tegra210-adma: handle pm_runtime_get_sync failure cases

[Navid Emamdoost]

Calling pm_runtime_get_sync increments the counter even in case of
failure, causing incorrect ref count. Call pm_runtime_put if
pm_runtime_get_sync fails.

Signed-off-by: Navid Emamdoost 
---
 drivers/dma/tegra210-adma.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/dma/tegra210-adma.c b/drivers/dma/tegra210-adma.c
index c4ce5dfb149b..899eaaf9fc48 100644
--- a/drivers/dma/tegra210-adma.c
+++ b/drivers/dma/tegra210-adma.c
@@ -659,6 +659,7 @@ static int tegra_adma_alloc_chan_resources(struct dma_chan 
*dc)
ret = pm_runtime_get_sync(tdc2dev(tdc));
if (ret < 0) {
free_irq(tdc->irq, tdc);
+   pm_runtime_put(tdc2dev(tdc));
return ret;
}
 
@@ -870,7 +871,7 @@ static int tegra_adma_probe(struct platform_device *pdev)
 
ret = pm_runtime_get_sync(>dev);
if (ret < 0)
-   goto rpm_disable;
+   goto rpm_put;
 
ret = tegra_adma_init(tdma);
if (ret)
-- 
2.17.1

Re: [PATCH] dmaengine: tegra210-adma: fix pm_runtime_get_sync failure

[Navid Emamdoost]

On Thu, Jun 4, 2020 at 12:45 PM Jon Hunter  wrote:
>
>
> On 03/06/2020 19:41, Navid Emamdoost wrote:
> > Calling pm_runtime_get_sync increments the counter even in case of
> > failure, causing incorrect ref count. Call pm_runtime_put if
> > pm_runtime_get_sync fails.
> >
> > Signed-off-by: Navid Emamdoost 
> > ---
> >  drivers/dma/tegra210-adma.c | 1 +
> >  1 file changed, 1 insertion(+)
> >
> > diff --git a/drivers/dma/tegra210-adma.c b/drivers/dma/tegra210-adma.c
> > index c4ce5dfb149b..e8c749cd3fe8 100644
> > --- a/drivers/dma/tegra210-adma.c
> > +++ b/drivers/dma/tegra210-adma.c
> > @@ -659,6 +659,7 @@ static int tegra_adma_alloc_chan_resources(struct 
> > dma_chan *dc)
> >   ret = pm_runtime_get_sync(tdc2dev(tdc));
> >   if (ret < 0) {
> >   free_irq(tdc->irq, tdc);
> > + pm_runtime_put(tdc2dev(tdc));
> >   return ret;
> >   }
>
>
> Please do not send two patches with the same $subject that are fixing
> two different areas of the driver. In fact, please squash these two
> patches into a single fix and resend because they are fixing the same issue.

Sure, I will prepare a version 2 with your suggestions.

>
> Jon
>
> --
> nvpublic



-- 
Navid.

[PATCH] dmaengine: stm32-dmamux: fix pm_runtime_get_sync fialure cases

[Navid Emamdoost]

Calling pm_runtime_get_sync increments the counter even in case of
failure, causing incorrect ref count. Call pm_runtime_put_sync if
pm_runtime_get_sync fails.

Signed-off-by: Navid Emamdoost 
---
 drivers/dma/stm32-dmamux.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/dma/stm32-dmamux.c b/drivers/dma/stm32-dmamux.c
index 12f7637e13a1..ab250d7eed29 100644
--- a/drivers/dma/stm32-dmamux.c
+++ b/drivers/dma/stm32-dmamux.c
@@ -140,6 +140,7 @@ static void *stm32_dmamux_route_allocate(struct 
of_phandle_args *dma_spec,
ret = pm_runtime_get_sync(>dev);
if (ret < 0) {
spin_unlock_irqrestore(>lock, flags);
+   pm_runtime_put_sync(>dev);
goto error;
}
spin_unlock_irqrestore(>lock, flags);
@@ -340,8 +341,10 @@ static int stm32_dmamux_suspend(struct device *dev)
int i, ret;
 
ret = pm_runtime_get_sync(dev);
-   if (ret < 0)
+   if (ret < 0) {
+   pm_runtime_put_sync(dev);
return ret;
+   }
 
for (i = 0; i < stm32_dmamux->dma_requests; i++)
stm32_dmamux->ccr[i] = stm32_dmamux_read(stm32_dmamux->iomem,
-- 
2.17.1

Re: [PATCH] dmaengine: stm32-mdma: call pm_runtime_put if pm_runtime_get_sync fails

[Navid Emamdoost]

On Wed, Jun 3, 2020 at 1:52 PM Markus Elfring  wrote:
>
> > Calling pm_runtime_get_sync increments the counter even in case of
> > failure, causing incorrect ref count. Call pm_runtime_put if
> > pm_runtime_get_sync fails.
>
> Is it appropriate to copy a sentence from the change description
> into the patch subject?
>
> How do you think about a wording variant like the following?
Please stop proposing rewording on my patches!

I will consider updating my patches only if a maintainer asks for it.

>
>The PM runtime reference counter is generally incremented by a call of
>the function “pm_runtime_get_sync”.
>Thus call the function “pm_runtime_put” also in two error cases
>to keep the reference counting consistent.
>
>
> Would you like to add the tag “Fixes” to the commit message?
>
> Regards,
> Markus



-- 
Navid.

[PATCH] dmaengine: tegra210-adma: fix pm_runtime_get_sync failure

[Navid Emamdoost]

Calling pm_runtime_get_sync increments the counter even in case of
failure, causing incorrect ref count. Call pm_runtime_put if
pm_runtime_get_sync fails.

Signed-off-by: Navid Emamdoost 
---
 drivers/dma/tegra210-adma.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/dma/tegra210-adma.c b/drivers/dma/tegra210-adma.c
index c4ce5dfb149b..e8c749cd3fe8 100644
--- a/drivers/dma/tegra210-adma.c
+++ b/drivers/dma/tegra210-adma.c
@@ -659,6 +659,7 @@ static int tegra_adma_alloc_chan_resources(struct dma_chan 
*dc)
ret = pm_runtime_get_sync(tdc2dev(tdc));
if (ret < 0) {
free_irq(tdc->irq, tdc);
+   pm_runtime_put(tdc2dev(tdc));
return ret;
}
 
-- 
2.17.1

[PATCH] dmaengine: tegra210-adma: fix pm_runtime_get_sync failure

[Navid Emamdoost]

Calling pm_runtime_get_sync increments the counter even in case of
failure, causing incorrect ref count. Call pm_runtime_put if
pm_runtime_get_sync fails.

Signed-off-by: Navid Emamdoost 
---
 drivers/dma/tegra210-adma.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/dma/tegra210-adma.c b/drivers/dma/tegra210-adma.c
index c4ce5dfb149b..87f2a1bed3aa 100644
--- a/drivers/dma/tegra210-adma.c
+++ b/drivers/dma/tegra210-adma.c
@@ -869,8 +869,10 @@ static int tegra_adma_probe(struct platform_device *pdev)
pm_runtime_enable(>dev);
 
ret = pm_runtime_get_sync(>dev);
-   if (ret < 0)
+   if (ret < 0) {
+   pm_runtime_put_sync(>dev);
goto rpm_disable;
+   }
 
ret = tegra_adma_init(tdma);
if (ret)
-- 
2.17.1

[PATCH] engine: stm32-dma: call pm_runtime_put if pm_runtime_get_sync fails

[Navid Emamdoost]

Calling pm_runtime_get_sync increments the counter even in case of
failure, causing incorrect ref count. Call pm_runtime_put if
pm_runtime_get_sync fails.

Signed-off-by: Navid Emamdoost 
---
 drivers/dma/stm32-dma.c | 8 ++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/dma/stm32-dma.c b/drivers/dma/stm32-dma.c
index 0ddbaa4b4f0b..0aab86bd97fe 100644
--- a/drivers/dma/stm32-dma.c
+++ b/drivers/dma/stm32-dma.c
@@ -1169,8 +1169,10 @@ static int stm32_dma_alloc_chan_resources(struct 
dma_chan *c)
chan->config_init = false;
 
ret = pm_runtime_get_sync(dmadev->ddev.dev);
-   if (ret < 0)
+   if (ret < 0) {
+   pm_runtime_put(dmadev->ddev.dev);
return ret;
+   }
 
ret = stm32_dma_disable_chan(chan);
if (ret < 0)
@@ -1439,8 +1441,10 @@ static int stm32_dma_suspend(struct device *dev)
int id, ret, scr;
 
ret = pm_runtime_get_sync(dev);
-   if (ret < 0)
+   if (ret < 0) {
+   pm_runtime_put_sync(dev);
return ret;
+   }
 
for (id = 0; id < STM32_DMA_MAX_CHANNELS; id++) {
scr = stm32_dma_read(dmadev, STM32_DMA_SCR(id));
-- 
2.17.1

[PATCH] dmaengine: stm32-mdma: call pm_runtime_put if pm_runtime_get_sync fails

[Navid Emamdoost]

Calling pm_runtime_get_sync increments the counter even in case of
failure, causing incorrect ref count. Call pm_runtime_put if
pm_runtime_get_sync fails.

Signed-off-by: Navid Emamdoost 
---
 drivers/dma/stm32-mdma.c | 8 ++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/dma/stm32-mdma.c b/drivers/dma/stm32-mdma.c
index 5469563703d1..79bee1bb73f6 100644
--- a/drivers/dma/stm32-mdma.c
+++ b/drivers/dma/stm32-mdma.c
@@ -1449,8 +1449,10 @@ static int stm32_mdma_alloc_chan_resources(struct 
dma_chan *c)
}
 
ret = pm_runtime_get_sync(dmadev->ddev.dev);
-   if (ret < 0)
+   if (ret < 0) {
+   pm_runtime_put(dmadev->ddev.dev);
return ret;
+   }
 
ret = stm32_mdma_disable_chan(chan);
if (ret < 0)
@@ -1718,8 +1720,10 @@ static int stm32_mdma_pm_suspend(struct device *dev)
int ret;
 
ret = pm_runtime_get_sync(dev);
-   if (ret < 0)
+   if (ret < 0) {
+   pm_runtime_put_sync(dev);
return ret;
+   }
 
for (id = 0; id < dmadev->nr_channels; id++) {
ccr = stm32_mdma_read(dmadev, STM32_MDMA_CCR(id));
-- 
2.17.1

Re: spi: spi-ti-qspi: call pm_runtime_put on pm_runtime_get failure

[Navid Emamdoost]

On Tue, Jun 2, 2020 at 1:36 PM Mark Brown  wrote:
>
> On Tue, Jun 02, 2020 at 05:05:18PM +0200, Markus Elfring wrote:
> > >> I find this commit message improvable also according to Linux software
> > >> development documentation.
>
> > > Causing people to send out new versions of things for tweaks to the
> > > commit log consumes time for them and everyone they're sending changes to.
>
> > Improving patches (besides source code adjustments) is an usual software
> > development activity, isn't it?
>
> Your updates were not improvements.  The formatting was worse and to my
> native speaker eyes the grammar was worse.  With this sort of stylistic
> thing it's especially important that any review aligns with the needs
> and practices of the subsystem, there is opinion in there and multiple
> opinions just makes things harder for submitters.

Thanks Mark for your constructive opinion,
In most cases, such stylistic comments become confusing and
discouraging to those who are trying to chip in. Personally I think as
long as the patch does not contain typo and is not ambiguous from the
maintainer's perspective, it should be fine to let it go forward.



-- 
Navid.

[PATCH] media: staging: tegra-vde: add missing pm_runtime_put_autosuspend

[Navid Emamdoost]

Call to pm_runtime_get_sync increments counter even in case of
failure leading to incorrect ref count.
Call pm_runtime_put_autosuspend if pm_runtime_get_sync fails.

Signed-off-by: Navid Emamdoost 
---
 drivers/staging/media/tegra-vde/vde.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/media/tegra-vde/vde.c 
b/drivers/staging/media/tegra-vde/vde.c
index d3e63512a765..52cdd4a91e93 100644
--- a/drivers/staging/media/tegra-vde/vde.c
+++ b/drivers/staging/media/tegra-vde/vde.c
@@ -776,8 +776,10 @@ static int tegra_vde_ioctl_decode_h264(struct tegra_vde 
*vde,
goto release_dpb_frames;
 
ret = pm_runtime_get_sync(dev);
-   if (ret < 0)
+   if (ret < 0) {
+   pm_runtime_put_autosuspend(dev);
goto unlock;
+   }
 
/*
 * We rely on the VDE registers reset value, otherwise VDE
-- 
2.17.1

[PATCH] spi: img-spfi: add missing pm_runtime_pu

[Navid Emamdoost]

Call to pm_runtime_get_sync increments counter even in case of
failure leading to incorrect ref count.
Call pm_runtime_put if pm_runtime_get_sync fails.

Signed-off-by: Navid Emamdoost 
---
 drivers/spi/spi-img-spfi.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/spi/spi-img-spfi.c b/drivers/spi/spi-img-spfi.c
index 8543f5ed1099..c3d0452ac78a 100644
--- a/drivers/spi/spi-img-spfi.c
+++ b/drivers/spi/spi-img-spfi.c
@@ -785,8 +785,10 @@ static int img_spfi_resume(struct device *dev)
int ret;
 
ret = pm_runtime_get_sync(dev);
-   if (ret)
+   if (ret) {
+   pm_runtime_put(dev);
return ret;
+   }
spfi_reset(spfi);
pm_runtime_put(dev);
 
-- 
2.17.1

[PATCH] spi: tegra20-slink: add missing pm_runtime_put

[Navid Emamdoost]

Call to pm_runtime_get_sync increments counter even in case of
failure leading to incorrect ref count.
Call pm_runtime_put if pm_runtime_get_sync fails.

Signed-off-by: Navid Emamdoost 
---
 drivers/spi/spi-tegra20-slink.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/spi/spi-tegra20-slink.c b/drivers/spi/spi-tegra20-slink.c
index 7f4d932dade7..0675b36d647b 100644
--- a/drivers/spi/spi-tegra20-slink.c
+++ b/drivers/spi/spi-tegra20-slink.c
@@ -1192,6 +1192,7 @@ static int tegra_slink_resume(struct device *dev)
ret = pm_runtime_get_sync(dev);
if (ret < 0) {
dev_err(dev, "pm runtime failed, e = %d\n", ret);
+   pm_runtime_put(dev);
return ret;
}
tegra_slink_writel(tspi, tspi->command_reg, SLINK_COMMAND);
-- 
2.17.1

[PATCH] spi: tegra20-slink: add missing pm_runtime_put if pm_runtime_get_sync fails

[Navid Emamdoost]

Call to pm_runtime_get_sync increments counter even in case of
failure leading to incorrect ref count.
Call pm_runtime_put if pm_runtime_get_sync fails.

Signed-off-by: Navid Emamdoost 
---
 drivers/spi/spi-tegra20-slink.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/spi/spi-tegra20-slink.c b/drivers/spi/spi-tegra20-slink.c
index 7f4d932dade7..9509b7cb14e4 100644
--- a/drivers/spi/spi-tegra20-slink.c
+++ b/drivers/spi/spi-tegra20-slink.c
@@ -756,6 +756,7 @@ static int tegra_slink_setup(struct spi_device *spi)
ret = pm_runtime_get_sync(tspi->dev);
if (ret < 0) {
dev_err(tspi->dev, "pm runtime failed, e = %d\n", ret);
+   pm_runtime_put(tspi->dev);
return ret;
}
 
-- 
2.17.1

[PATCH] spi: tegra20-slink: call pm_runtime_put if pm_runtime_get_sync fails

[Navid Emamdoost]

Call to pm_runtime_get_sync increments counter even in case of
failure leading to incorrect ref count.
Call pm_runtime_put if pm_runtime_get_sync fails.

Signed-off-by: Navid Emamdoost 
---
 drivers/spi/spi-tegra20-slink.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/spi/spi-tegra20-slink.c b/drivers/spi/spi-tegra20-slink.c
index 7f4d932dade7..15361db00982 100644
--- a/drivers/spi/spi-tegra20-slink.c
+++ b/drivers/spi/spi-tegra20-slink.c
@@ -1118,6 +1118,7 @@ static int tegra_slink_probe(struct platform_device *pdev)
ret = pm_runtime_get_sync(>dev);
if (ret < 0) {
dev_err(>dev, "pm runtime get failed, e = %d\n", ret);
+   pm_runtime_put(>dev);
goto exit_pm_disable;
}
tspi->def_command_reg  = SLINK_M_S;
-- 
2.17.1

[PATCH] spi: sprd: call pm_runtime_put if pm_runtime_get_sync fails

[Navid Emamdoost]

Call to pm_runtime_get_sync increments counter even in case of
failure leading to incorrect ref count.
Call pm_runtime_put_noidle if pm_runtime_get_sync fails.

Signed-off-by: Navid Emamdoost 
---
 drivers/spi/spi-sprd.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/spi/spi-sprd.c b/drivers/spi/spi-sprd.c
index 6678f1cbc566..860032af4b98 100644
--- a/drivers/spi/spi-sprd.c
+++ b/drivers/spi/spi-sprd.c
@@ -1018,6 +1018,7 @@ static int sprd_spi_remove(struct platform_device *pdev)
ret = pm_runtime_get_sync(ss->dev);
if (ret < 0) {
dev_err(ss->dev, "failed to resume SPI controller\n");
+   pm_runtime_put_noidle(>dev);
return ret;
}
 
-- 
2.17.1

[PATCH] spi: tegra114: missing put on pm_runtime_get_sync failure

[Navid Emamdoost]

the call to pm_runtime_get_sync increments the counter even 
in case of failure leading to incorrect ref count.
Call pm_runtime_put if pm_runtime_get_sync fails.

Signed-off-by: Navid Emamdoost 
---
 drivers/spi/spi-tegra114.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/spi/spi-tegra114.c b/drivers/spi/spi-tegra114.c
index 83edabdb41ad..dccd2ac1a975 100644
--- a/drivers/spi/spi-tegra114.c
+++ b/drivers/spi/spi-tegra114.c
@@ -974,6 +974,7 @@ static int tegra_spi_setup(struct spi_device *spi)
dev_err(tspi->dev, "pm runtime failed, e = %d\n", ret);
if (cdata)
tegra_spi_cleanup(spi);
+   pm_runtime_put(tspi->dev);
return ret;
}
 
@@ -1398,6 +1399,7 @@ static int tegra_spi_probe(struct platform_device *pdev)
ret = pm_runtime_get_sync(>dev);
if (ret < 0) {
dev_err(>dev, "pm runtime get failed, e = %d\n", ret);
+   pm_runtime_put(>dev);
goto exit_pm_disable;
}
 
@@ -1479,6 +1481,7 @@ static int tegra_spi_resume(struct device *dev)
ret = pm_runtime_get_sync(dev);
if (ret < 0) {
dev_err(dev, "pm runtime failed, e = %d\n", ret);
+   pm_runtime_put(dev);
return ret;
}
tegra_spi_writel(tspi, tspi->command1_reg, SPI_COMMAND1);
-- 
2.17.1

[PATCH] spi: tegra20-sflash: call pm_runtime_put in case of pm_runtime_get failure

[Navid Emamdoost]

The counter is incremented via pm_runtime_get even in failure case.
To correct the counter call pm_runtime_put in case of failure, too.

Signed-off-by: Navid Emamdoost 
---
 drivers/spi/spi-tegra20-sflash.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/spi/spi-tegra20-sflash.c b/drivers/spi/spi-tegra20-sflash.c
index 514429379206..33c34f9c2021 100644
--- a/drivers/spi/spi-tegra20-sflash.c
+++ b/drivers/spi/spi-tegra20-sflash.c
@@ -552,6 +552,7 @@ static int tegra_sflash_resume(struct device *dev)
 
ret = pm_runtime_get_sync(dev);
if (ret < 0) {
+   pm_runtime_put(dev);
dev_err(dev, "pm runtime failed, e = %d\n", ret);
return ret;
}
-- 
2.17.1

[PATCH] spi: spi-ti-qspi: call pm_runtime_put on pm_runtime_get failure

[Navid Emamdoost]

The counter is incremented via pm_runtime_get even in failure case.
To correct the counter call pm_runtime_put in case of failure, too.

Signed-off-by: Navid Emamdoost 
---
 drivers/spi/spi-ti-qspi.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/spi/spi-ti-qspi.c b/drivers/spi/spi-ti-qspi.c
index 366a3e5cca6b..dfb811c5ef76 100644
--- a/drivers/spi/spi-ti-qspi.c
+++ b/drivers/spi/spi-ti-qspi.c
@@ -174,6 +174,7 @@ static int ti_qspi_setup(struct spi_device *spi)
 
ret = pm_runtime_get_sync(qspi->dev);
if (ret < 0) {
+   pm_runtime_put_autosuspend(qspi->dev);
dev_err(qspi->dev, "pm_runtime_get_sync() failed\n");
return ret;
}
-- 
2.17.1

[PATCH] pwm: img: call pm_runtime_put in pm_runtime_get_sync failed case

[Navid Emamdoost]

Even in failed case of pm_runtime_get_sync, the usage_count
is incremented. In order to keep the usage_count with correct
value call appropriate put.

Signed-off-by: Navid Emamdoost 
---
 drivers/pwm/pwm-img.c | 8 ++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/pwm/pwm-img.c b/drivers/pwm/pwm-img.c
index c9e57bd109fb..599a0f66a384 100644
--- a/drivers/pwm/pwm-img.c
+++ b/drivers/pwm/pwm-img.c
@@ -129,8 +129,10 @@ static int img_pwm_config(struct pwm_chip *chip, struct 
pwm_device *pwm,
duty = DIV_ROUND_UP(timebase * duty_ns, period_ns);
 
ret = pm_runtime_get_sync(chip->dev);
-   if (ret < 0)
+   if (ret < 0) {
+   pm_runtime_put_autosuspend(chip->dev);
return ret;
+   }
 
val = img_pwm_readl(pwm_chip, PWM_CTRL_CFG);
val &= ~(PWM_CTRL_CFG_DIV_MASK << PWM_CTRL_CFG_DIV_SHIFT(pwm->hwpwm));
@@ -331,8 +333,10 @@ static int img_pwm_remove(struct platform_device *pdev)
int ret;
 
ret = pm_runtime_get_sync(>dev);
-   if (ret < 0)
+   if (ret < 0) {
+   pm_runtime_put(>dev);
return ret;
+   }
 
for (i = 0; i < pwm_chip->chip.npwm; i++) {
val = img_pwm_readl(pwm_chip, PWM_CTRL_CFG);
-- 
2.17.1

[PATCH] power: supply: bq24190_charger.c: call pm_runtime_put in pm_runtime_get_sync failed case

[Navid Emamdoost]

From: Navid 

Even in failed case of pm_runtime_get_sync, the usage_count
is incremented. In order to keep the usage_count with correct
value call pm_runtime_put_autosuspend.

Signed-off-by: Navid 
---
 drivers/power/supply/bq24190_charger.c | 26 ++
 1 file changed, 18 insertions(+), 8 deletions(-)

diff --git a/drivers/power/supply/bq24190_charger.c 
b/drivers/power/supply/bq24190_charger.c
index 453d6332d43a..502ccdac2443 100644
--- a/drivers/power/supply/bq24190_charger.c
+++ b/drivers/power/supply/bq24190_charger.c
@@ -448,8 +448,10 @@ static ssize_t bq24190_sysfs_show(struct device *dev,
return -EINVAL;
 
ret = pm_runtime_get_sync(bdi->dev);
-   if (ret < 0)
-   return ret;
+   if (ret < 0) {
+   count = ret;
+   goto out;
+   }
 
ret = bq24190_read_mask(bdi, info->reg, info->mask, info->shift, );
if (ret)
@@ -458,6 +460,7 @@ static ssize_t bq24190_sysfs_show(struct device *dev,
count = scnprintf(buf, PAGE_SIZE, "%hhx\n", v);
 
pm_runtime_mark_last_busy(bdi->dev);
+out:
pm_runtime_put_autosuspend(bdi->dev);
 
return count;
@@ -481,14 +484,17 @@ static ssize_t bq24190_sysfs_store(struct device *dev,
return ret;
 
ret = pm_runtime_get_sync(bdi->dev);
-   if (ret < 0)
-   return ret;
+   if (ret < 0) {
+   count = ret;
+   goto out;
+   }
 
ret = bq24190_write_mask(bdi, info->reg, info->mask, info->shift, v);
if (ret)
count = ret;
 
pm_runtime_mark_last_busy(bdi->dev);
+out:
pm_runtime_put_autosuspend(bdi->dev);
 
return count;
@@ -1076,7 +1082,7 @@ static int bq24190_charger_get_property(struct 
power_supply *psy,
 
ret = pm_runtime_get_sync(bdi->dev);
if (ret < 0)
-   return ret;
+   goto out;
 
switch (psp) {
case POWER_SUPPLY_PROP_CHARGE_TYPE:
@@ -1132,6 +1138,7 @@ static int bq24190_charger_get_property(struct 
power_supply *psy,
}
 
pm_runtime_mark_last_busy(bdi->dev);
+out:
pm_runtime_put_autosuspend(bdi->dev);
 
return ret;
@@ -1148,7 +1155,7 @@ static int bq24190_charger_set_property(struct 
power_supply *psy,
 
ret = pm_runtime_get_sync(bdi->dev);
if (ret < 0)
-   return ret;
+   goto out;
 
switch (psp) {
case POWER_SUPPLY_PROP_ONLINE:
@@ -1174,6 +1181,7 @@ static int bq24190_charger_set_property(struct 
power_supply *psy,
}
 
pm_runtime_mark_last_busy(bdi->dev);
+out:
pm_runtime_put_autosuspend(bdi->dev);
 
return ret;
@@ -1409,7 +1417,7 @@ static int bq24190_battery_get_property(struct 
power_supply *psy,
 
ret = pm_runtime_get_sync(bdi->dev);
if (ret < 0)
-   return ret;
+   goto out;
 
switch (psp) {
case POWER_SUPPLY_PROP_STATUS:
@@ -1438,6 +1446,7 @@ static int bq24190_battery_get_property(struct 
power_supply *psy,
}
 
pm_runtime_mark_last_busy(bdi->dev);
+out:
pm_runtime_put_autosuspend(bdi->dev);
 
return ret;
@@ -1455,7 +1464,7 @@ static int bq24190_battery_set_property(struct 
power_supply *psy,
 
ret = pm_runtime_get_sync(bdi->dev);
if (ret < 0)
-   return ret;
+   goto out;
 
switch (psp) {
case POWER_SUPPLY_PROP_ONLINE:
@@ -1469,6 +1478,7 @@ static int bq24190_battery_set_property(struct 
power_supply *psy,
}
 
pm_runtime_mark_last_busy(bdi->dev);
+out:
pm_runtime_put_autosuspend(bdi->dev);
 
return ret;
-- 
2.17.1

Re: [PATCH] ath9k: release allocated buffer if timed out

[Navid Emamdoost]

Hi Brian,

On Tue, May 12, 2020 at 11:57 AM Brian Norris  wrote:
>
> On Fri, Sep 6, 2019 at 11:59 AM Navid Emamdoost
>  wrote:
> >
> > In ath9k_wmi_cmd, the allocated network buffer needs to be released
> > if timeout happens. Otherwise memory will be leaked.
> >
> > Signed-off-by: Navid Emamdoost 
>
> I wonder, did you actually test your patches? I ask, because it seems
> that all your patches are of the same mechanical variety (produced by
> some sort of research project?), and if I look around a bit, I see
I found this via static analysis and as a result, did had the inputs
to test it with (like the way fuzzing works).
It may be beneficial if you could point me to any testing
infrastructure that you use or are aware of for future cases.

> several mistakes and regressions noted on your other patches. And
> recently, I see someone reporting a 5.4 kernel regression, which looks
> a lot like it was caused by this patch:
>
> https://bugzilla.kernel.org/show_bug.cgi?id=207703#c1
>
> I'll propose a revert, if there's no evidence this was actually tested
> or otherwise confirmed to fix a real bug.
>
> Brian



-- 
Navid.

Potential Race Condition in tls_hw_hash() and alike

[Navid Emamdoost]

Hi,

I was wondering if a race condition in net/tls/tls_main.c may lead to
a UAF or not?

The scenario can be like this:
1) device is initialized and registered via chtls_register_dev()
2) while tls_hw_hash() is executed in one thread, the device gets
detached (CPU2), and another thread tries to acquire the pointer
(CPU3):

CPU1:  tls_hw_hash()
CPU2: chtls_uld_state_change()
  CPU3: can be tls_hw_hash() or tls_hw_unhash()
//
spin_lock_bh(_spinlock);
list_for_each_entry(dev, _list, dev_list) {
if (dev->hash) {
kref_get(>kref);  //kref == 2
spin_unlock_bh(_spinlock);
 kref_put(>tlsdev.kref, cdev->tlsdev.release); //kref == 1
err |= dev->hash(dev, sk);


spin_lock_bh(_spinlock);
kref_put(>kref, dev->release);   //kref
== 0, release
 kref_get(>kref);  //BUG: kref 0 to 1!




Basically, the problem comes from the fact that kref_put is not lock protected.
Do you agree that such a race condition may happen? If yes, then is
moving kref_put inside the lock a practical solution?

Thank you,
--
Navid.

Re: [PATCH] clocksource/drivers: Fix memory leak in ttc_setup_clockevent

[Navid Emamdoost]

Thanks for the feedback, I updated this patch and sent v2.
Also, I submitted a patch to fix the error handling path in
ttc_setup_clocksource(). Here is the link to it:
https://lore.kernel.org/patchwork/patch/1143242/

On Tue, Oct 22, 2019 at 3:51 AM Michal Simek  wrote:
>
> On 22. 10. 19 10:26, Markus Elfring wrote:
> >> In the impelementation of ttc_setup_clockevent() the allocated memory
> >> for ttcce should be released if clk_notifier_register() fails.
> >
> > * Please avoid the copying of typos from previous change descriptions.
> >
> > * Under which circumstances will an “imperative mood” matter for you here?
> >   
> > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?id=7d194c2100ad2a6dded545887d02754948ca5241#n151
> >
> >
> >> +++ b/drivers/clocksource/timer-cadence-ttc.c
> >> @@ -424,6 +424,7 @@ static int __init ttc_setup_clockevent(struct clk *clk,
> >>  >ttc.clk_rate_change_nb);
> >>  if (err) {
> >>  pr_warn("Unable to register clock notifier.\n");
> >> +kfree(ttcce);
> >>  return err;
> >>  }
> >
> > This addition looks correct.
> > But I would prefer to move such exception handling code to the end of
> > this function implementation so that duplicate source code will be reduced.
> > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/coding-style.rst?id=7d194c2100ad2a6dded545887d02754948ca5241#n450
>
> Just a note. Maybe you should also consider to fix this error path in
> ttc_setup_clocksource() when notifier also can fail that there is no
> need to continue with code execution.
>
> Thanks,
> Michal



-- 
Navid.

[PATCH] clocksource/drivers: Fix error handling in ttc_setup_clocksource

[Navid Emamdoost]

In the implementation of ttc_setup_clocksource() when
clk_notifier_register() fails the execution should go to error handling.
Additionally, to avoid memory leak the allocated memory for ttccs should
be released, too. So, goto error handling to release the memory and
return.

Fixes: e932900a3279 ("arm: zynq: Use standard timer binding")
Signed-off-by: Navid Emamdoost 
---
 drivers/clocksource/timer-cadence-ttc.c | 20 +++-
 1 file changed, 11 insertions(+), 9 deletions(-)

diff --git a/drivers/clocksource/timer-cadence-ttc.c 
b/drivers/clocksource/timer-cadence-ttc.c
index 88fe2e9ba9a3..035e16bc17d3 100644
--- a/drivers/clocksource/timer-cadence-ttc.c
+++ b/drivers/clocksource/timer-cadence-ttc.c
@@ -328,10 +328,8 @@ static int __init ttc_setup_clocksource(struct clk *clk, 
void __iomem *base,
ttccs->ttc.clk = clk;
 
err = clk_prepare_enable(ttccs->ttc.clk);
-   if (err) {
-   kfree(ttccs);
-   return err;
-   }
+   if (err)
+   goto release_ttccs;
 
ttccs->ttc.freq = clk_get_rate(ttccs->ttc.clk);
 
@@ -341,8 +339,10 @@ static int __init ttc_setup_clocksource(struct clk *clk, 
void __iomem *base,
 
err = clk_notifier_register(ttccs->ttc.clk,
>ttc.clk_rate_change_nb);
-   if (err)
+   if (err) {
pr_warn("Unable to register clock notifier.\n");
+   goto release_ttccs;
+   }
 
ttccs->ttc.base_addr = base;
ttccs->cs.name = "ttc_clocksource";
@@ -363,16 +363,18 @@ static int __init ttc_setup_clocksource(struct clk *clk, 
void __iomem *base,
 ttccs->ttc.base_addr + TTC_CNT_CNTRL_OFFSET);
 
err = clocksource_register_hz(>cs, ttccs->ttc.freq / PRESCALE);
-   if (err) {
-   kfree(ttccs);
-   return err;
-   }
+   if (err)
+   goto release_ttccs;
 
ttc_sched_clock_val_reg = base + TTC_COUNT_VAL_OFFSET;
sched_clock_register(ttc_sched_clock_read, timer_width,
 ttccs->ttc.freq / PRESCALE);
 
return 0;
+
+release_ttccs:
+   kfree(ttccs);
+   return err;
 }
 
 static int ttc_rate_change_clockevent_cb(struct notifier_block *nb,
-- 
2.17.1

[PATCH v2] clocksource/drivers: Fix memory leak in ttc_setup_clockevent

[Navid Emamdoost]

In the implementation of ttc_setup_clockevent() release the allocated
memory for ttcce if clk_notifier_register() fails.

Fixes: 70504f311d4b ("clocksource/drivers/cadence_ttc: Convert init function to 
return error")
Signed-off-by: Navid Emamdoost 
---
Changes in v2:
- Added goto label for error handling
- Update description and fix typo

 drivers/clocksource/timer-cadence-ttc.c | 19 ++-
 1 file changed, 10 insertions(+), 9 deletions(-)

diff --git a/drivers/clocksource/timer-cadence-ttc.c 
b/drivers/clocksource/timer-cadence-ttc.c
index 88fe2e9ba9a3..0caacbc67102 100644
--- a/drivers/clocksource/timer-cadence-ttc.c
+++ b/drivers/clocksource/timer-cadence-ttc.c
@@ -411,10 +411,8 @@ static int __init ttc_setup_clockevent(struct clk *clk,
ttcce->ttc.clk = clk;
 
err = clk_prepare_enable(ttcce->ttc.clk);
-   if (err) {
-   kfree(ttcce);
-   return err;
-   }
+   if (err)
+   goto release_ttcce;
 
ttcce->ttc.clk_rate_change_nb.notifier_call =
ttc_rate_change_clockevent_cb;
@@ -424,7 +422,7 @@ static int __init ttc_setup_clockevent(struct clk *clk,
>ttc.clk_rate_change_nb);
if (err) {
pr_warn("Unable to register clock notifier.\n");
-   return err;
+   goto release_ttcce;
}
 
ttcce->ttc.freq = clk_get_rate(ttcce->ttc.clk);
@@ -453,15 +451,18 @@ static int __init ttc_setup_clockevent(struct clk *clk,
 
err = request_irq(irq, ttc_clock_event_interrupt,
  IRQF_TIMER, ttcce->ce.name, ttcce);
-   if (err) {
-   kfree(ttcce);
-   return err;
-   }
+   if (err)
+   goto release_ttcce;
 
clockevents_config_and_register(>ce,
ttcce->ttc.freq / PRESCALE, 1, 0xfffe);
 
return 0;
+
+release_ttcce:
+
+   kfree(ttcce);
+   return err;
 }
 
 /**
-- 
2.17.1

[PATCH] clocksource/drivers/davinci: Fix memory leak in davinci_timer_register

[Navid Emamdoost]

In the impelementation of davinci_timer_register() the allocated memory
for clockevent should be released if request_irq() fails.

Fixes: 721154f972aa ("clocksource/drivers/davinci: Add support for clockevents")
Signed-off-by: Navid Emamdoost 
---
 drivers/clocksource/timer-davinci.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/clocksource/timer-davinci.c 
b/drivers/clocksource/timer-davinci.c
index 62745c962049..910d4d2f0d64 100644
--- a/drivers/clocksource/timer-davinci.c
+++ b/drivers/clocksource/timer-davinci.c
@@ -299,6 +299,7 @@ int __init davinci_timer_register(struct clk *clk,
 "clockevent/tim12", clockevent);
if (rv) {
pr_err("Unable to request the clockevent interrupt");
+   kfree(clockevent);
return rv;
}
 
-- 
2.17.1

[PATCH] clocksource/drivers: Fix memory leak in ttc_setup_clockevent

[Navid Emamdoost]

In the impelementation of ttc_setup_clockevent() the allocated memory
for ttcce should be released if clk_notifier_register() fails.

Fixes: 70504f311d4b ("clocksource/drivers/cadence_ttc: Convert init function to 
return error")
Signed-off-by: Navid Emamdoost 
---
 drivers/clocksource/timer-cadence-ttc.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/clocksource/timer-cadence-ttc.c 
b/drivers/clocksource/timer-cadence-ttc.c
index 88fe2e9ba9a3..b40fc6581389 100644
--- a/drivers/clocksource/timer-cadence-ttc.c
+++ b/drivers/clocksource/timer-cadence-ttc.c
@@ -424,6 +424,7 @@ static int __init ttc_setup_clockevent(struct clk *clk,
>ttc.clk_rate_change_nb);
if (err) {
pr_warn("Unable to register clock notifier.\n");
+   kfree(ttcce);
return err;
}
 
-- 
2.17.1

[PATCH] locking/atomics: Fix memory leak in bcm2835_timer_init

[Navid Emamdoost]

In the impelementation of bcm2835_timer_init() the allocated memory for
timer should be released if setup_irq() fails.

Fixes: 84c39b8b7d46 ("clocksource/drivers/bcm2835_timer: Unmap region obtained 
by of_iomap")
Signed-off-by: Navid Emamdoost 
---
 drivers/clocksource/bcm2835_timer.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/clocksource/bcm2835_timer.c 
b/drivers/clocksource/bcm2835_timer.c
index 2b196cbfadb6..7b27cc53ce9c 100644
--- a/drivers/clocksource/bcm2835_timer.c
+++ b/drivers/clocksource/bcm2835_timer.c
@@ -121,6 +121,7 @@ static int __init bcm2835_timer_init(struct device_node 
*node)
ret = setup_irq(irq, >act);
if (ret) {
pr_err("Can't set up timer IRQ\n");
+   kfree(timer);
goto err_iounmap;
}
 
-- 
2.17.1

[PATCH] ipmi: Fix memory leak in __ipmi_bmc_register

[Navid Emamdoost]

In the impelementation of __ipmi_bmc_register() the allocated memory for
bmc should be released in case ida_simple_get() fails.

Fixes: 68e7e50f195f ("ipmi: Don't use BMC product/dev ids in the BMC name")
Signed-off-by: Navid Emamdoost 
---
 drivers/char/ipmi/ipmi_msghandler.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/char/ipmi/ipmi_msghandler.c 
b/drivers/char/ipmi/ipmi_msghandler.c
index 2aab80e19ae0..e4928ed46396 100644
--- a/drivers/char/ipmi/ipmi_msghandler.c
+++ b/drivers/char/ipmi/ipmi_msghandler.c
@@ -3031,8 +3031,11 @@ static int __ipmi_bmc_register(struct ipmi_smi *intf,
bmc->pdev.name = "ipmi_bmc";
 
rv = ida_simple_get(_bmc_ida, 0, 0, GFP_KERNEL);
-   if (rv < 0)
+   if (rv < 0) {
+   kfree(bmc);
goto out;
+   }
+
bmc->pdev.dev.driver = 
bmc->pdev.id = rv;
bmc->pdev.dev.release = release_bmc_device;
-- 
2.17.1

[PATCH] drm/amdgpu: Fix memory leak in amdgpu_fence_emit

[Navid Emamdoost]

In the impelementation of amdgpu_fence_emit() if dma_fence_wait() fails
and returns an errno, before returning the allocated memory for fence
should be released.

Fixes: 3d2aca8c8620 ("drm/amdgpu: fix old fence check in amdgpu_fence_emit")
Signed-off-by: Navid Emamdoost 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c
index 23085b352cf2..2f59c9270a7e 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c
@@ -166,8 +166,10 @@ int amdgpu_fence_emit(struct amdgpu_ring *ring, struct 
dma_fence **f,
if (old) {
r = dma_fence_wait(old, false);
dma_fence_put(old);
-   if (r)
+   if (r) {
+   dma_fence_put(fence);
return r;
+   }
}
}
 
-- 
2.17.1

Re: [PATCH v2] apparmor: Fix use-after-free in aa_audit_rule_init

[Navid Emamdoost]

On Mon, Oct 21, 2019 at 10:45 AM Tyler Hicks  wrote:
>
> On 2019-10-21 10:23:47, Navid Emamdoost wrote:
> > In the implementation of aa_audit_rule_init(), when aa_label_parse()
> > fails the allocated memory for rule is released using
> > aa_audit_rule_free(). But after this release, the return statement
> > tries to access the label field of the rule which results in
> > use-after-free. Before releasing the rule, copy errNo and return it
> > after release.
> >
> > Fixes: 52e8c38001d8 ("apparmor: Fix memory leak of rule on error exit path")
>
> Ugh! I'm not sure what I was thinking when I authored that patch. :/
>
> > Signed-off-by: Navid Emamdoost 
> > ---
> > Changes in v2:
> >   -- Fix typo in description
> >   -- move err definition inside the if statement.
> >
> >  security/apparmor/audit.c | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/security/apparmor/audit.c b/security/apparmor/audit.c
> > index 5a98661a8b46..334065302fb6 100644
> > --- a/security/apparmor/audit.c
> > +++ b/security/apparmor/audit.c
> > @@ -197,8 +197,9 @@ int aa_audit_rule_init(u32 field, u32 op, char 
> > *rulestr, void **vrule)
> >   rule->label = aa_label_parse(_ns->unconfined->label, rulestr,
> >GFP_KERNEL, true, false);
> >   if (IS_ERR(rule->label)) {
> > + int err = rule->label;
>
> Since rule->label is a pointer, I'd like to see this:
>
>  int err = PTR_ERR(rule->label);
>
> >   aa_audit_rule_free(rule);
> > - return PTR_ERR(rule->label);
> > + return PTR_ERR(err);
>
> This line would change to:
>
>  return err;
>
Tyler, I made the changes and sent v3.

>
> Tyler
>
> >   }
> >
> >   *vrule = rule;
> > --
> > 2.17.1
> >



-- 
Navid.

[PATCH v3] apparmor: Fix use-after-free in aa_audit_rule_init

[Navid Emamdoost]

In the implementation of aa_audit_rule_init(), when aa_label_parse()
fails the allocated memory for rule is released using
aa_audit_rule_free(). But after this release, the return statement
tries to access the label field of the rule which results in
use-after-free. Before releasing the rule, copy errNo and return it
after release.

Fixes: 52e8c38001d8 ("apparmor: Fix memory leak of rule on error exit path")
Signed-off-by: Navid Emamdoost 
---
Changes in v3:
-- applied Tyler Hicks recommendation on err initialization.

 security/apparmor/audit.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/security/apparmor/audit.c b/security/apparmor/audit.c
index 5a98661a8b46..597732503815 100644
--- a/security/apparmor/audit.c
+++ b/security/apparmor/audit.c
@@ -197,8 +197,9 @@ int aa_audit_rule_init(u32 field, u32 op, char *rulestr, 
void **vrule)
rule->label = aa_label_parse(_ns->unconfined->label, rulestr,
 GFP_KERNEL, true, false);
if (IS_ERR(rule->label)) {
+   int err = PTR_ERR(rule->label);
aa_audit_rule_free(rule);
-   return PTR_ERR(rule->label);
+   return err;
}
 
*vrule = rule;
-- 
2.17.1

Re: [PATCH] apparmor: Fix use-after-free in aa_audit_rule_init

[Navid Emamdoost]

On Sun, Oct 20, 2019 at 1:51 PM John Johansen
 wrote:
>
> On 10/20/19 7:16 AM, Markus Elfring wrote:
> >> … But after this release the the return statement
> >> tries to access the label field of the rule which results in
> >> use-after-free. Before releaseing the rule, copy errNo and return it
> >> after releasing rule.
> >
> Navid thanks for finding this, and Markus thanks for the review
>
> > Please avoid a duplicate word and a typo in this change description.
> > My preference would be a v2 version of the patch with the small clean-ups
> that Markus has pointed out.

John and Markus, I updated and submitted v2.

>
> If I don't see a v2 this week I can pull this one in and do the revisions
> myself adding a little fix-up note.
>
> >
> > …
> >> +++ b/security/apparmor/audit.c
> > …
> >> @@ -197,8 +198,9 @@ int aa_audit_rule_init(u32 field, u32 op, char 
> >> *rulestr, void **vrule)
> >>  rule->label = aa_label_parse(_ns->unconfined->label, rulestr,
> >>   GFP_KERNEL, true, false);
> >>  if (IS_ERR(rule->label)) {
> >> +err = rule->label;
> >
> > How do you think about to define the added local variable in this if branch 
> > directly?
> >
> > + int err = rule->label;
> >
>
> yes, since err isn't defined or in use else where this would be preferable
>
> >>  aa_audit_rule_free(rule);
> >> -return PTR_ERR(rule->label);
> >> +return PTR_ERR(err);
> >>  }
> >>
> >>  *vrule = rule;
> >
> >
> > Regards,
> > Markus
> >
>


-- 
Thanks,
Navid.

[PATCH v2] apparmor: Fix use-after-free in aa_audit_rule_init

[Navid Emamdoost]

In the implementation of aa_audit_rule_init(), when aa_label_parse()
fails the allocated memory for rule is released using
aa_audit_rule_free(). But after this release, the return statement
tries to access the label field of the rule which results in
use-after-free. Before releasing the rule, copy errNo and return it
after release.

Fixes: 52e8c38001d8 ("apparmor: Fix memory leak of rule on error exit path")
Signed-off-by: Navid Emamdoost 
---
Changes in v2:
-- Fix typo in description
-- move err definition inside the if statement.

 security/apparmor/audit.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/security/apparmor/audit.c b/security/apparmor/audit.c
index 5a98661a8b46..334065302fb6 100644
--- a/security/apparmor/audit.c
+++ b/security/apparmor/audit.c
@@ -197,8 +197,9 @@ int aa_audit_rule_init(u32 field, u32 op, char *rulestr, 
void **vrule)
rule->label = aa_label_parse(_ns->unconfined->label, rulestr,
 GFP_KERNEL, true, false);
if (IS_ERR(rule->label)) {
+   int err = rule->label;
aa_audit_rule_free(rule);
-   return PTR_ERR(rule->label);
+   return PTR_ERR(err);
}
 
*vrule = rule;
-- 
2.17.1

[PATCH] apparmor: Fix use-after-free in aa_audit_rule_init

[Navid Emamdoost]

In the implementation of aa_audit_rule_init(), when aa_label_parse()
fails the allocated memory for rule is released using
aa_audit_rule_free(). But after this release the the return statement
tries to access the label field of the rule which results in
use-after-free. Before releaseing the rule, copy errNo and return it
after releasing rule.

Fixes: 52e8c38001d8 ("apparmor: Fix memory leak of rule on error exit path")
Signed-off-by: Navid Emamdoost 
---
 security/apparmor/audit.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/security/apparmor/audit.c b/security/apparmor/audit.c
index 5a98661a8b46..48c15fb0aafe 100644
--- a/security/apparmor/audit.c
+++ b/security/apparmor/audit.c
@@ -178,6 +178,7 @@ void aa_audit_rule_free(void *vrule)
 int aa_audit_rule_init(u32 field, u32 op, char *rulestr, void **vrule)
 {
struct aa_audit_rule *rule;
+   int err;
 
switch (field) {
case AUDIT_SUBJ_ROLE:
@@ -197,8 +198,9 @@ int aa_audit_rule_init(u32 field, u32 op, char *rulestr, 
void **vrule)
rule->label = aa_label_parse(_ns->unconfined->label, rulestr,
 GFP_KERNEL, true, false);
if (IS_ERR(rule->label)) {
+   err = rule->label;
aa_audit_rule_free(rule);
-   return PTR_ERR(rule->label);
+   return PTR_ERR(err);
}
 
*vrule = rule;
-- 
2.17.1

Re: [PATCH 1/2] drm/imx: Fix error handling for a kmemdup() call in imx_pd_bind()

[Navid Emamdoost]

On Sat, Oct 12, 2019 at 4:07 AM Markus Elfring  wrote:
>
> From: Markus Elfring 
> Date: Sat, 12 Oct 2019 10:30:21 +0200
>
> The return value from a call of the function “kmemdup” was not checked
> in this function implementation. Thus add the corresponding error handling.
>
> Fixes: 19022aaae677dfa171a719e9d1ff04823ce65a65 ("staging: drm/imx: Add 
> parallel display support")
> Signed-off-by: Markus Elfring 
> ---
>  drivers/gpu/drm/imx/parallel-display.c | 7 ++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/imx/parallel-display.c 
> b/drivers/gpu/drm/imx/parallel-display.c
> index 35518e5de356..39c4798f56b6 100644
> --- a/drivers/gpu/drm/imx/parallel-display.c
> +++ b/drivers/gpu/drm/imx/parallel-display.c
> @@ -210,8 +210,13 @@ static int imx_pd_bind(struct device *dev, struct device 
> *master, void *data)
> return -ENOMEM;
>
> edidp = of_get_property(np, "edid", >edid_len);
> -   if (edidp)
> +   if (edidp) {
> imxpd->edid = kmemdup(edidp, imxpd->edid_len, GFP_KERNEL);
> +   if (!imxpd->edid) {
> +   devm_kfree(dev, imxpd);

You should not try to free imxpd here as it is a resource-managed
allocation via devm_kzalloc(). It means memory allocated with this
function is
 automatically freed on driver detach. So, this patch introduces a double-free.

> +   return -ENOMEM;
> +   }
> +   }
>
> ret = of_property_read_string(np, "interface-pix-fmt", );
> if (!ret) {
> --
> 2.23.0
>


-- 
Navid.

[PATCH] clk: bcm2835: Fix memory leak in bcm2835_register_pll

[Navid Emamdoost]

In the implementation of bcm2835_register_pll(), the allocated memory
for pll should be released if devm_clk_hw_register() fails.

Fixes: b19f009d4510 ("clk: bcm2835: Migrate to clk_hw based registration and OF 
APIs")
Signed-off-by: Navid Emamdoost 
---
 drivers/clk/bcm/clk-bcm2835.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/clk/bcm/clk-bcm2835.c b/drivers/clk/bcm/clk-bcm2835.c
index 802e488fd3c3..99549642110a 100644
--- a/drivers/clk/bcm/clk-bcm2835.c
+++ b/drivers/clk/bcm/clk-bcm2835.c
@@ -1320,8 +1320,10 @@ static struct clk_hw *bcm2835_register_pll(struct 
bcm2835_cprman *cprman,
pll->hw.init = 
 
ret = devm_clk_hw_register(cprman->dev, >hw);
-   if (ret)
+   if (ret) {
+   kfree(pll);
return NULL;
+   }
return >hw;
 }
 
-- 
2.17.1

[PATCH] cpufreq/cpufreq_governor: Fix memory leak in cpufreq_dbs_governor_init

[Navid Emamdoost]

In the implementation of cpufreq_dbs_governor_init(), dbs_data is
allocated and later is assigned to governor_data. But before that
assignment, if gov->init() fails this allocation is not released.
dbs_data should be released in case if gov->init() failure.

Fixes: 714a2d9c8792 ("cpufreq: governor: split cpufreq_governor_dbs()")
Signed-off-by: Navid Emamdoost 
---
 drivers/cpufreq/cpufreq_governor.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/cpufreq/cpufreq_governor.c 
b/drivers/cpufreq/cpufreq_governor.c
index 4bb054d0cb43..deb099d36266 100644
--- a/drivers/cpufreq/cpufreq_governor.c
+++ b/drivers/cpufreq/cpufreq_governor.c
@@ -428,8 +428,10 @@ int cpufreq_dbs_governor_init(struct cpufreq_policy 
*policy)
gov_attr_set_init(_data->attr_set, _dbs->list);
 
ret = gov->init(dbs_data);
-   if (ret)
+   if (ret) {
+   kfree(dbs_data);
goto free_policy_dbs_info;
+   }
 
/*
 * The sampling interval should not be less than the transition latency
-- 
2.17.1

[PATCH v2] media: usb: fix memory leak in af9005_identify_state

[Navid Emamdoost]

In af9005_identify_state when returning -EIO the allocated buffer should
be released. Replace the "return -EIO" with assignment into ret and move
deb_info() under a check.

Fixes: af4e067e1dcf ("V4L/DVB (5625): Add support for the AF9005 demodulator 
from Afatech")
Signed-off-by: Navid Emamdoost 
---
Changes in v2:
-- Reused the error handling to release buf
-- Added Fixes tag
---
 drivers/media/usb/dvb-usb/af9005.c | 7 ---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/media/usb/dvb-usb/af9005.c 
b/drivers/media/usb/dvb-usb/af9005.c
index 02697d86e8c1..bc6e7659c37b 100644
--- a/drivers/media/usb/dvb-usb/af9005.c
+++ b/drivers/media/usb/dvb-usb/af9005.c
@@ -975,9 +975,10 @@ static int af9005_identify_state(struct usb_device *udev,
*cold = 1;
else if (reply == 0x02)
*cold = 0;
-   else
-   return -EIO;
-   deb_info("Identify state cold = %d\n", *cold);
+   else
+   ret = -EIO;
+   if (!ret)
+   deb_info("Identify state cold = %d\n", *cold);
 
 err:
kfree(buf);
-- 
2.17.1

Re: [PATCH] media: usb: fix memory leak in af9005_identify_state

[Navid Emamdoost]

Hi Hans,

On Wed, Oct 9, 2019 at 7:42 AM Hans Verkuil  wrote:
>
> On 9/14/19 1:55 AM, Navid Emamdoost wrote:
> > In af9005_identify_state when returning -EIO the allocated buffer should
> > be released.
> >
> > Signed-off-by: Navid Emamdoost 
> > ---
> >  drivers/media/usb/dvb-usb/af9005.c | 4 +++-
> >  1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/media/usb/dvb-usb/af9005.c 
> > b/drivers/media/usb/dvb-usb/af9005.c
> > index 02697d86e8c1..aee500beaab6 100644
> > --- a/drivers/media/usb/dvb-usb/af9005.c
> > +++ b/drivers/media/usb/dvb-usb/af9005.c
> > @@ -975,8 +975,10 @@ static int af9005_identify_state(struct usb_device 
> > *udev,
> >   *cold = 1;
> >   else if (reply == 0x02)
> >   *cold = 0;
> > - else
> > + else {
> > + kfree(buf);
> >   return -EIO;
> > + }
>
> Why not just set ret = -EIO; here?
>
This makes sense. I will send a v2.

> You only need to add a
>
> if (!ret)
> >   deb_info("Identify state cold = %d\n", *cold);
>
> before this line.
>
> So this becomes:
>
> else
> ret = -EIO;
> if (!ret)
> deb_info("Identify state cold = %d\n", *cold);
>
> Regards,
>
> Hans
>
> >
> >  err:
> >
>


-- 
Navid.

Re: [PATCH] rtlwifi: fix memory leak in rtl_usb_probe

[Navid Emamdoost]

Oh! It's duplicate, thanks for catching that.

On Sat, Oct 5, 2019 at 11:08 AM Markus Elfring  wrote:
>
> > In rtl_usb_probe, a new hw is allocated via ieee80211_alloc_hw(). This
> > allocation should be released in case of allocation failure for
> > rtlpriv->usb_data.
> >
> > Fixes: a7959c1394d4 ("rtlwifi: Preallocate USB read buffers and eliminate 
> > kalloc in read routine")
>
> Which event did trigger the sending of this patch variant
> after a similar change was integrated already?
> https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/?id=3f93616951138a598d930dcaec40f2bfd9ce43bb
> https://lore.kernel.org/lkml/20191001092047.71e8460...@smtp.codeaurora.org/
> https://lore.kernel.org/patchwork/comment/1331936/
>
> Regards,
> Markus



-- 
Navid.

[PATCH] net: qlogic: Fix memory leak in ql_alloc_large_buffers

[Navid Emamdoost]

In ql_alloc_large_buffers, a new skb is allocated via netdev_alloc_skb.
This skb should be released if pci_dma_mapping_error fails.

Fixes: 0f8ab89e825f ("qla3xxx: Check return code from pci_map_single() in 
ql_release_to_lrg_buf_free_list(), ql_populate_free_queue(), 
ql_alloc_large_buffers(), and ql3xxx_send()")
Signed-off-by: Navid Emamdoost 
---
 drivers/net/ethernet/qlogic/qla3xxx.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/ethernet/qlogic/qla3xxx.c 
b/drivers/net/ethernet/qlogic/qla3xxx.c
index 457444894d80..b4b8ba00ee01 100644
--- a/drivers/net/ethernet/qlogic/qla3xxx.c
+++ b/drivers/net/ethernet/qlogic/qla3xxx.c
@@ -2787,6 +2787,7 @@ static int ql_alloc_large_buffers(struct ql3_adapter 
*qdev)
netdev_err(qdev->ndev,
   "PCI mapping failed with error: 
%d\n",
   err);
+   dev_kfree_skb_irq(skb);
ql_free_large_buffers(qdev);
return -ENOMEM;
}
-- 
2.17.1

[PATCH] mwifiex: pcie: Fix memory leak in mwifiex_pcie_init_evt_ring

[Navid Emamdoost]

In mwifiex_pcie_init_evt_ring, a new skb is allocated which should be
released if mwifiex_map_pci_memory() fails. The release for skb and
card->evtbd_ring_vbase is added.

Fixes: 0732484b47b5 ("mwifiex: separate ring initialization and ring creation 
routines")
Signed-off-by: Navid Emamdoost 
---
 drivers/net/wireless/marvell/mwifiex/pcie.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/net/wireless/marvell/mwifiex/pcie.c 
b/drivers/net/wireless/marvell/mwifiex/pcie.c
index eff06d59e9df..096334e941a1 100644
--- a/drivers/net/wireless/marvell/mwifiex/pcie.c
+++ b/drivers/net/wireless/marvell/mwifiex/pcie.c
@@ -687,8 +687,11 @@ static int mwifiex_pcie_init_evt_ring(struct 
mwifiex_adapter *adapter)
skb_put(skb, MAX_EVENT_SIZE);
 
if (mwifiex_map_pci_memory(adapter, skb, MAX_EVENT_SIZE,
-  PCI_DMA_FROMDEVICE))
+  PCI_DMA_FROMDEVICE)) {
+   kfree_skb(skb);
+   kfree(card->evtbd_ring_vbase);
return -1;
+   }
 
buf_pa = MWIFIEX_SKB_DMA_ADDR(skb);
 
-- 
2.17.1

[PATCH] mwifiex: pcie: Fix memory leak in mwifiex_pcie_alloc_cmdrsp_buf

[Navid Emamdoost]

In mwifiex_pcie_alloc_cmdrsp_buf, a new skb is allocated which should be
released if mwifiex_map_pci_memory() fails. The release is added.

Fixes: fc3314609047 ("mwifiex: use pci_alloc/free_consistent APIs for PCIe")
Signed-off-by: Navid Emamdoost 
---
 drivers/net/wireless/marvell/mwifiex/pcie.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/net/wireless/marvell/mwifiex/pcie.c 
b/drivers/net/wireless/marvell/mwifiex/pcie.c
index eff06d59e9df..1578eaa071bd 100644
--- a/drivers/net/wireless/marvell/mwifiex/pcie.c
+++ b/drivers/net/wireless/marvell/mwifiex/pcie.c
@@ -1029,8 +1029,10 @@ static int mwifiex_pcie_alloc_cmdrsp_buf(struct 
mwifiex_adapter *adapter)
}
skb_put(skb, MWIFIEX_UPLD_SIZE);
if (mwifiex_map_pci_memory(adapter, skb, MWIFIEX_UPLD_SIZE,
-  PCI_DMA_FROMDEVICE))
+  PCI_DMA_FROMDEVICE)) {
+   kfree_skb(skb);
return -1;
+   }
 
card->cmdrsp_buf = skb;
 
-- 
2.17.1

[PATCH] staging: vt6655: Fix memory leak in vt6655_probe

[Navid Emamdoost]

In vt6655_probe, if vnt_init() fails the cleanup code needs to be called
like other error handling cases. The call to device_free_info() is
added.

Fixes: 67013f2c0e58 ("staging: vt6655: mac80211 conversion add main mac80211 
functions")
Signed-off-by: Navid Emamdoost 
---
 drivers/staging/vt6655/device_main.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/vt6655/device_main.c 
b/drivers/staging/vt6655/device_main.c
index c6bb4aaf9bd0..082302944c37 100644
--- a/drivers/staging/vt6655/device_main.c
+++ b/drivers/staging/vt6655/device_main.c
@@ -1748,8 +1748,10 @@ vt6655_probe(struct pci_dev *pcid, const struct 
pci_device_id *ent)
 
priv->hw->max_signal = 100;
 
-   if (vnt_init(priv))
+   if (vnt_init(priv)) {
+   device_free_info(priv);
return -ENODEV;
+   }
 
device_print_info(priv);
pci_set_drvdata(pcid, priv);
-- 
2.17.1