LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Gen Zhang <blackgod016574@gmail.com>
To: Kalle Valo <kvalo@codeaurora.org>
Cc: davem@davemloft.net, linux-wireless@vger.kernel.org,
	netdev@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] wlcore: spi: Fix a memory leaking bug in wl1271_probe()
Date: Tue, 28 May 2019 20:14:52 +0800	[thread overview]
Message-ID: <20190528121452.GA23464@zhanggen-UX430UQ> (raw)
In-Reply-To: <20190528113922.E2B1060312@smtp.codeaurora.org>

On Tue, May 28, 2019 at 11:39:22AM +0000, Kalle Valo wrote:
> Gen Zhang <blackgod016574@gmail.com> wrote:
> 
> > In wl1271_probe(), 'glue->core' is allocated by platform_device_alloc(),
> > when this allocation fails, ENOMEM is returned. However, 'pdev_data'
> > and 'glue' are allocated by devm_kzalloc() before 'glue->core'. When
> > platform_device_alloc() returns NULL, we should also free 'pdev_data'
> > and 'glue' before wl1271_probe() ends to prevent leaking memory.
> > 
> > Similarly, we shoulf free 'pdev_data' when 'glue' is NULL. And we should
> > free 'pdev_data' and 'glue' when 'glue->reg' is error and when 'ret' is
> > error.
> > 
> > Further, we should free 'glue->core', 'pdev_data' and 'glue' when this 
> > function normally ends to prevent leaking memory.
> > 
> > Signed-off-by: Gen Zhang <blackgod016574@gmail.com>
> 
> Same questions as with similar SDIO patch:
> 
> https://patchwork.kernel.org/patch/10959049/
> 
> Patch set to Changes Requested.
> 
> -- 
> https://patchwork.kernel.org/patch/10959053/
> 
> https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
> 
Thanks for your reply, Kalle. I had debate with Jon about this patch. 
You could kindly refer to lkml: https://lkml.org/lkml/2019/5/23/1547. 
And I don't think a practical conclusion is made there.

Further, I e-mailed Greg K-H about when should we use devm_kmalloc().

On Tue, May 28, 2019 at 08:32:57AM +0800, Gen Zhang wrote:
> devm_kmalloc() is used to allocate memory for a driver dev. Comments
> above the definition and doc 
> (https://www.kernel.org/doc/Documentation/driver-model/devres.txt) all
> imply that allocated the memory is automatically freed on driver attach,
> no matter allocation fail or not. However, I examined the code, and
> there are many sites that devm_kfree() is used to free devm_kmalloc().
> e.g. hisi_sas_debugfs_init() in drivers/scsi/hisi_sas/hisi_sas_main.c.
> So I am totally confused about this issue. Can anybody give me some
> guidance? When should we use devm_kfree()?
He replied: If you "know" you need to free the memory now, 
call devm_kfree(). If you want to wait for it to be cleaned up latter, 
like normal, then do not call it.

So could please look in to this issue?

Thanks
Gen

  parent reply	other threads:[~2019-05-28 12:15 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-05-24  3:02 Gen Zhang
2019-05-28 11:39 ` Kalle Valo
     [not found] ` <20190528113922.E2B1060312@smtp.codeaurora.org>
2019-05-28 12:14   ` Gen Zhang [this message]
2019-05-28 12:33     ` Kalle Valo
2019-05-28 12:38       ` Gen Zhang

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190528121452.GA23464@zhanggen-UX430UQ \
    --to=blackgod016574@gmail.com \
    --cc=davem@davemloft.net \
    --cc=kvalo@codeaurora.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-wireless@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --subject='Re: [PATCH] wlcore: spi: Fix a memory leaking bug in wl1271_probe()' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).