* [PATCH] usb: dwc3: free dwc->regset on dwc3_debugfs_exit
@ 2016-04-08 9:42 changbin.du
2016-04-11 8:06 ` Felipe Balbi
0 siblings, 1 reply; 14+ messages in thread
From: changbin.du @ 2016-04-08 9:42 UTC (permalink / raw)
To: balbi; +Cc: gregkh, linux-usb, linux-kernel, Du, Changbin
From: "Du, Changbin" <changbin.du@intel.com>
dwc->regset is allocated on dwc3_debugfs_init, and should
be released on dwc3_debugfs_exit.
Signed-off-by: Du, Changbin <changbin.du@intel.com>
---
This patch is seperated from patch set:
[PATCH v2 0/3] Improvement, fix and new entry for dwc3 debugfs
Because it is not related to other 2 patches.
---
drivers/usb/dwc3/debugfs.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/usb/dwc3/debugfs.c b/drivers/usb/dwc3/debugfs.c
index 9ac37fe..9eeb444 100644
--- a/drivers/usb/dwc3/debugfs.c
+++ b/drivers/usb/dwc3/debugfs.c
@@ -687,4 +687,7 @@ void dwc3_debugfs_exit(struct dwc3 *dwc)
{
debugfs_remove_recursive(dwc->root);
dwc->root = NULL;
+
+ kfree(dwc->regset);
+ dwc->regset = NULL;
}
--
2.5.0
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH] usb: dwc3: free dwc->regset on dwc3_debugfs_exit
2016-04-08 9:42 [PATCH] usb: dwc3: free dwc->regset on dwc3_debugfs_exit changbin.du
@ 2016-04-11 8:06 ` Felipe Balbi
2016-04-11 11:09 ` Du, Changbin
0 siblings, 1 reply; 14+ messages in thread
From: Felipe Balbi @ 2016-04-11 8:06 UTC (permalink / raw)
To: changbin.du; +Cc: gregkh, linux-usb, linux-kernel, Du, Changbin
[-- Attachment #1: Type: text/plain, Size: 1027 bytes --]
changbin.du@intel.com writes:
> From: "Du, Changbin" <changbin.du@intel.com>
>
> dwc->regset is allocated on dwc3_debugfs_init, and should
> be released on dwc3_debugfs_exit.
>
> Signed-off-by: Du, Changbin <changbin.du@intel.com>
> ---
> This patch is seperated from patch set:
> [PATCH v2 0/3] Improvement, fix and new entry for dwc3 debugfs
> Because it is not related to other 2 patches.
>
> ---
> drivers/usb/dwc3/debugfs.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/drivers/usb/dwc3/debugfs.c b/drivers/usb/dwc3/debugfs.c
> index 9ac37fe..9eeb444 100644
> --- a/drivers/usb/dwc3/debugfs.c
> +++ b/drivers/usb/dwc3/debugfs.c
> @@ -687,4 +687,7 @@ void dwc3_debugfs_exit(struct dwc3 *dwc)
> {
> debugfs_remove_recursive(dwc->root);
> dwc->root = NULL;
> +
> + kfree(dwc->regset);
we also need a kfree() on dwc3_debugfs_init().
> + dwc->regset = NULL;
setting regset to NULL is unnecessary. We only call dwc3_debugfs_exit()
when removing the driver.
--
balbi
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 818 bytes --]
^ permalink raw reply [flat|nested] 14+ messages in thread
* RE: [PATCH] usb: dwc3: free dwc->regset on dwc3_debugfs_exit
2016-04-11 8:06 ` Felipe Balbi
@ 2016-04-11 11:09 ` Du, Changbin
2016-04-11 11:13 ` Felipe Balbi
0 siblings, 1 reply; 14+ messages in thread
From: Du, Changbin @ 2016-04-11 11:09 UTC (permalink / raw)
To: Felipe Balbi; +Cc: gregkh, linux-usb, linux-kernel
> > diff --git a/drivers/usb/dwc3/debugfs.c b/drivers/usb/dwc3/debugfs.c
> > index 9ac37fe..9eeb444 100644
> > --- a/drivers/usb/dwc3/debugfs.c
> > +++ b/drivers/usb/dwc3/debugfs.c
> > @@ -687,4 +687,7 @@ void dwc3_debugfs_exit(struct dwc3 *dwc)
> > {
> > debugfs_remove_recursive(dwc->root);
> > dwc->root = NULL;
> > +
> > + kfree(dwc->regset);
>
> we also need a kfree() on dwc3_debugfs_init().
This patch is based on the patch set
[PATCH v3 1/2] usb: dwc3: make dwc3_debugfs_init return value be void>
So, they do has dependency. :)
> > + dwc->regset = NULL;
>
> setting regset to NULL is unnecessary. We only call dwc3_debugfs_exit()
> when removing the driver.
>
> --
> Balbi
I'd like keep this line even it is unnecessary, because It is a good habit to
Avoid wild pointers. Just like the dwc->root = NULL.
Thanks,
Du, Changbin
^ permalink raw reply [flat|nested] 14+ messages in thread
* RE: [PATCH] usb: dwc3: free dwc->regset on dwc3_debugfs_exit
2016-04-11 11:09 ` Du, Changbin
@ 2016-04-11 11:13 ` Felipe Balbi
2016-04-11 11:38 ` Du, Changbin
0 siblings, 1 reply; 14+ messages in thread
From: Felipe Balbi @ 2016-04-11 11:13 UTC (permalink / raw)
To: Du, Changbin; +Cc: gregkh, linux-usb, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 1184 bytes --]
"Du, Changbin" <changbin.du@intel.com> writes:
>> > diff --git a/drivers/usb/dwc3/debugfs.c b/drivers/usb/dwc3/debugfs.c
>> > index 9ac37fe..9eeb444 100644
>> > --- a/drivers/usb/dwc3/debugfs.c
>> > +++ b/drivers/usb/dwc3/debugfs.c
>> > @@ -687,4 +687,7 @@ void dwc3_debugfs_exit(struct dwc3 *dwc)
>> > {
>> > debugfs_remove_recursive(dwc->root);
>> > dwc->root = NULL;
>> > +
>> > + kfree(dwc->regset);
>>
>> we also need a kfree() on dwc3_debugfs_init().
> This patch is based on the patch set
> [PATCH v3 1/2] usb: dwc3: make dwc3_debugfs_init return value be void>
> So, they do has dependency. :)
I _did_ mention on previous set that fixes and new-features shouldn't
have dependencies between them. That was the whole point of splitting
the series in two.
>> > + dwc->regset = NULL;
>>
>> setting regset to NULL is unnecessary. We only call dwc3_debugfs_exit()
>> when removing the driver.
>>
>> --
>> Balbi
> I'd like keep this line even it is unnecessary, because It is a good habit to
> Avoid wild pointers. Just like the dwc->root = NULL.
there won't be any wild pointers here, we'll free struct dwc3 *dwc itself.
--
balbi
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 818 bytes --]
^ permalink raw reply [flat|nested] 14+ messages in thread
* RE: [PATCH] usb: dwc3: free dwc->regset on dwc3_debugfs_exit
2016-04-11 11:13 ` Felipe Balbi
@ 2016-04-11 11:38 ` Du, Changbin
2016-04-11 12:09 ` Felipe Balbi
0 siblings, 1 reply; 14+ messages in thread
From: Du, Changbin @ 2016-04-11 11:38 UTC (permalink / raw)
To: Felipe Balbi; +Cc: gregkh, linux-usb, linux-kernel
>
> >> > + dwc->regset = NULL;
> >>
> >> setting regset to NULL is unnecessary. We only call dwc3_debugfs_exit()
> >> when removing the driver.
> >>
> >> --
> >> Balbi
> > I'd like keep this line even it is unnecessary, because It is a good habit to
> > Avoid wild pointers. Just like the dwc->root = NULL.
>
> there won't be any wild pointers here, we'll free struct dwc3 *dwc itself.
>
> --
> Balbi
I agree the dwc will be freed in current code. But the 'free' logical is out
of the debugfs code. They should be treat as some logical independent. Per
this point, I still think set pointer to null is not bad. For example, if dwc3 core
code invoke dwc3_debugfs_exit twice by mistake(just an example case, not
really), then no crash/impact for the second call.
Thanks,
Du, Changbin
^ permalink raw reply [flat|nested] 14+ messages in thread
* RE: [PATCH] usb: dwc3: free dwc->regset on dwc3_debugfs_exit
2016-04-11 11:38 ` Du, Changbin
@ 2016-04-11 12:09 ` Felipe Balbi
2016-04-12 2:10 ` Du, Changbin
0 siblings, 1 reply; 14+ messages in thread
From: Felipe Balbi @ 2016-04-11 12:09 UTC (permalink / raw)
To: Du, Changbin; +Cc: gregkh, linux-usb, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 1219 bytes --]
Hi,
"Du, Changbin" <changbin.du@intel.com> writes:
>>
>> >> > + dwc->regset = NULL;
>> >>
>> >> setting regset to NULL is unnecessary. We only call dwc3_debugfs_exit()
>> >> when removing the driver.
>> >>
>> >> --
>> >> Balbi
>> > I'd like keep this line even it is unnecessary, because It is a good habit to
>> > Avoid wild pointers. Just like the dwc->root = NULL.
>>
>> there won't be any wild pointers here, we'll free struct dwc3 *dwc itself.
>>
>> --
>> Balbi
> I agree the dwc will be freed in current code. But the 'free' logical is out
> of the debugfs code. They should be treat as some logical independent. Per
> this point, I still think set pointer to null is not bad. For example, if dwc3 core
> code invoke dwc3_debugfs_exit twice by mistake(just an example case, not
> really), then no crash/impact for the second call.
the second call should crash because it's clearly wrong ;-) If dwc3 ever
calls dwc3_debugfs_exit() twice, it really deserves to crash. It's
something so wrong that we want the verbosity and urgency of a kernel
oops to make sure we fix it ASAP.
If, however, we set it to null, it might be years before we notice
anything's wrong.
--
balbi
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 818 bytes --]
^ permalink raw reply [flat|nested] 14+ messages in thread
* RE: [PATCH] usb: dwc3: free dwc->regset on dwc3_debugfs_exit
2016-04-11 12:09 ` Felipe Balbi
@ 2016-04-12 2:10 ` Du, Changbin
2016-04-12 6:18 ` Felipe Balbi
0 siblings, 1 reply; 14+ messages in thread
From: Du, Changbin @ 2016-04-12 2:10 UTC (permalink / raw)
To: Felipe Balbi; +Cc: gregkh, linux-usb, linux-kernel
> Hi,
>
> "Du, Changbin" <changbin.du@intel.com> writes:
> >>
> >> >> > + dwc->regset = NULL;
> >> >>
> >> >> setting regset to NULL is unnecessary. We only call
> dwc3_debugfs_exit()
> >> >> when removing the driver.
> >> >>
> >> >> --
> >> >> Balbi
> >> > I'd like keep this line even it is unnecessary, because It is a good habit to
> >> > Avoid wild pointers. Just like the dwc->root = NULL.
> >>
> >> there won't be any wild pointers here, we'll free struct dwc3 *dwc itself.
> >>
> >> --
> >> Balbi
> > I agree the dwc will be freed in current code. But the 'free' logical is out
> > of the debugfs code. They should be treat as some logical independent.
> Per
> > this point, I still think set pointer to null is not bad. For example, if dwc3
> core
> > code invoke dwc3_debugfs_exit twice by mistake(just an example case,
> not
> > really), then no crash/impact for the second call.
>
> the second call should crash because it's clearly wrong ;-) If dwc3 ever
> calls dwc3_debugfs_exit() twice, it really deserves to crash. It's
> something so wrong that we want the verbosity and urgency of a kernel
> oops to make sure we fix it ASAP.
>
> If, however, we set it to null, it might be years before we notice
> anything's wrong.
>
> --
> Balbi
Hmm, I agree from this point. I will combine this patch with other two patches
(due to their dependency). And I'd like remove the 'dwc->root=NULL' as well,
Is it ok for you?
Thx,
Du, Changbin
^ permalink raw reply [flat|nested] 14+ messages in thread
* RE: [PATCH] usb: dwc3: free dwc->regset on dwc3_debugfs_exit
2016-04-12 2:10 ` Du, Changbin
@ 2016-04-12 6:18 ` Felipe Balbi
2016-04-12 6:52 ` Du, Changbin
2016-04-12 8:14 ` [PATCH v2] usb: dwc3: fix memory leak of dwc->regset changbin.du
0 siblings, 2 replies; 14+ messages in thread
From: Felipe Balbi @ 2016-04-12 6:18 UTC (permalink / raw)
To: Du, Changbin; +Cc: gregkh, linux-usb, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 2307 bytes --]
Hi Changbin,
"Du, Changbin" <changbin.du@intel.com> writes:
>> Hi,
>>
>> "Du, Changbin" <changbin.du@intel.com> writes:
>> >>
>> >> >> > + dwc->regset = NULL;
>> >> >>
>> >> >> setting regset to NULL is unnecessary. We only call
>> dwc3_debugfs_exit()
>> >> >> when removing the driver.
>> >> >>
>> >> >> --
>> >> >> Balbi
>> >> > I'd like keep this line even it is unnecessary, because It is a good habit to
>> >> > Avoid wild pointers. Just like the dwc->root = NULL.
>> >>
>> >> there won't be any wild pointers here, we'll free struct dwc3 *dwc itself.
>> >>
>> >> --
>> >> Balbi
>> > I agree the dwc will be freed in current code. But the 'free' logical is out
>> > of the debugfs code. They should be treat as some logical independent.
>> Per
>> > this point, I still think set pointer to null is not bad. For example, if dwc3
>> core
>> > code invoke dwc3_debugfs_exit twice by mistake(just an example case,
>> not
>> > really), then no crash/impact for the second call.
>>
>> the second call should crash because it's clearly wrong ;-) If dwc3 ever
>> calls dwc3_debugfs_exit() twice, it really deserves to crash. It's
>> something so wrong that we want the verbosity and urgency of a kernel
>> oops to make sure we fix it ASAP.
>>
>> If, however, we set it to null, it might be years before we notice
>> anything's wrong.
>>
>> --
>> Balbi
>
> Hmm, I agree from this point. I will combine this patch with other two patches
> (due to their dependency). And I'd like remove the 'dwc->root=NULL' as well,
you are creating a dependency that doesn't exist. Please stop that. You
should have two separate branches based on v4.6-rc3 (or, if you prefer,
one based on my testing/fixes and another based on my testing/next). On
one branch you have *only* $subject and you fix *all* the memory
leaks. On the other branch you have the other two patches.
Ignore the fact that we might have a conflict, that's for git (and
maintainers) to handle when they happen.
Again, don't create dependencies between fixes for the -rc cycle and
changes for the next merge window.
> Is it ok for you?
yeah, please remove root = NULL as that's completely unnecessary, but
split these patches in separate branches and fix all memory leaks.
--
balbi
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 818 bytes --]
^ permalink raw reply [flat|nested] 14+ messages in thread
* RE: [PATCH] usb: dwc3: free dwc->regset on dwc3_debugfs_exit
2016-04-12 6:18 ` Felipe Balbi
@ 2016-04-12 6:52 ` Du, Changbin
2016-04-12 8:06 ` Felipe Balbi
2016-04-12 8:14 ` [PATCH v2] usb: dwc3: fix memory leak of dwc->regset changbin.du
1 sibling, 1 reply; 14+ messages in thread
From: Du, Changbin @ 2016-04-12 6:52 UTC (permalink / raw)
To: Felipe Balbi; +Cc: gregkh, linux-usb, linux-kernel
Hi, Balbi,
> > Hmm, I agree from this point. I will combine this patch with other two
> patches
> > (due to their dependency). And I'd like remove the 'dwc->root=NULL' as
> well,
>
> you are creating a dependency that doesn't exist. Please stop that. You
> should have two separate branches based on v4.6-rc3 (or, if you prefer,
> one based on my testing/fixes and another based on my testing/next). On
> one branch you have *only* $subject and you fix *all* the memory
> leaks. On the other branch you have the other two patches.
>
> Ignore the fact that we might have a conflict, that's for git (and
> maintainers) to handle when they happen.
>
> Again, don't create dependencies between fixes for the -rc cycle and
> changes for the next merge window.
>
Thanks for dedicated explanation. I was concern about the conflict.
Now it is very clear for me to handle such situation.
> > Is it ok for you?
>
> yeah, please remove root = NULL as that's completely unnecessary, but
> split these patches in separate branches and fix all memory leaks.
>
> --
> balbi
^ permalink raw reply [flat|nested] 14+ messages in thread
* RE: [PATCH] usb: dwc3: free dwc->regset on dwc3_debugfs_exit
2016-04-12 6:52 ` Du, Changbin
@ 2016-04-12 8:06 ` Felipe Balbi
0 siblings, 0 replies; 14+ messages in thread
From: Felipe Balbi @ 2016-04-12 8:06 UTC (permalink / raw)
To: Du, Changbin; +Cc: gregkh, linux-usb, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 1065 bytes --]
Hi,
"Du, Changbin" <changbin.du@intel.com> writes:
> Hi, Balbi,
>
>> > Hmm, I agree from this point. I will combine this patch with other two
>> patches
>> > (due to their dependency). And I'd like remove the 'dwc->root=NULL' as
>> well,
>>
>> you are creating a dependency that doesn't exist. Please stop that. You
>> should have two separate branches based on v4.6-rc3 (or, if you prefer,
>> one based on my testing/fixes and another based on my testing/next). On
>> one branch you have *only* $subject and you fix *all* the memory
>> leaks. On the other branch you have the other two patches.
>>
>> Ignore the fact that we might have a conflict, that's for git (and
>> maintainers) to handle when they happen.
>>
>> Again, don't create dependencies between fixes for the -rc cycle and
>> changes for the next merge window.
>>
> Thanks for dedicated explanation. I was concern about the conflict.
yeah, no problem ;-)
> Now it is very clear for me to handle such situation.
good :-) glad we could sort it out.
--
balbi
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 818 bytes --]
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v2] usb: dwc3: fix memory leak of dwc->regset
2016-04-12 6:18 ` Felipe Balbi
2016-04-12 6:52 ` Du, Changbin
@ 2016-04-12 8:14 ` changbin.du
2016-04-12 8:26 ` Felipe Balbi
1 sibling, 1 reply; 14+ messages in thread
From: changbin.du @ 2016-04-12 8:14 UTC (permalink / raw)
To: balbi; +Cc: gregkh, linux-usb, linux-kernel, Du, Changbin
From: "Du, Changbin" <changbin.du@intel.com>
dwc->regset is allocated on dwc3_debugfs_init, and should
be released on init failure or dwc3_debugfs_exit. Btw,
The line "dwc->root = NULL" is unnecessary, so remove it.
Signed-off-by: Du, Changbin <changbin.du@intel.com>
---
v2:
Title changed;
free dwc->regset on failure path.
---
drivers/usb/dwc3/debugfs.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/dwc3/debugfs.c b/drivers/usb/dwc3/debugfs.c
index 9ac37fe..abd8889 100644
--- a/drivers/usb/dwc3/debugfs.c
+++ b/drivers/usb/dwc3/debugfs.c
@@ -678,7 +678,8 @@ int dwc3_debugfs_init(struct dwc3 *dwc)
err1:
debugfs_remove_recursive(root);
-
+ if (!dwc->regset)
+ kfree(dwc->regset);
err0:
return ret;
}
@@ -686,5 +687,5 @@ err0:
void dwc3_debugfs_exit(struct dwc3 *dwc)
{
debugfs_remove_recursive(dwc->root);
- dwc->root = NULL;
+ kfree(dwc->regset);
}
--
2.5.0
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH v3] usb: dwc3: fix memory leak of dwc->regset
2016-04-12 8:26 ` Felipe Balbi
@ 2016-04-12 8:24 ` changbin.du
2016-04-12 10:09 ` Felipe Balbi
0 siblings, 1 reply; 14+ messages in thread
From: changbin.du @ 2016-04-12 8:24 UTC (permalink / raw)
To: balbi; +Cc: gregkh, linux-usb, linux-kernel, Du, Changbin
From: "Du, Changbin" <changbin.du@intel.com>
dwc->regset is allocated on dwc3_debugfs_init, and should
be released on init failure or dwc3_debugfs_exit. Btw,
The line "dwc->root = NULL" is unnecessary, so remove it.
Signed-off-by: Du, Changbin <changbin.du@intel.com>
---
v3:
remove unnecessary if(!NULL) for free
v2:
Title changed;
free dwc->regset on failure path.
---
drivers/usb/dwc3/debugfs.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/dwc3/debugfs.c b/drivers/usb/dwc3/debugfs.c
index 9ac37fe..07d99eb 100644
--- a/drivers/usb/dwc3/debugfs.c
+++ b/drivers/usb/dwc3/debugfs.c
@@ -678,7 +678,7 @@ int dwc3_debugfs_init(struct dwc3 *dwc)
err1:
debugfs_remove_recursive(root);
-
+ kfree(dwc->regset);
err0:
return ret;
}
@@ -686,5 +686,5 @@ err0:
void dwc3_debugfs_exit(struct dwc3 *dwc)
{
debugfs_remove_recursive(dwc->root);
- dwc->root = NULL;
+ kfree(dwc->regset);
}
--
2.5.0
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH v2] usb: dwc3: fix memory leak of dwc->regset
2016-04-12 8:14 ` [PATCH v2] usb: dwc3: fix memory leak of dwc->regset changbin.du
@ 2016-04-12 8:26 ` Felipe Balbi
2016-04-12 8:24 ` [PATCH v3] " changbin.du
0 siblings, 1 reply; 14+ messages in thread
From: Felipe Balbi @ 2016-04-12 8:26 UTC (permalink / raw)
To: changbin.du; +Cc: gregkh, linux-usb, linux-kernel, Du, Changbin
[-- Attachment #1: Type: text/plain, Size: 981 bytes --]
Hi,
changbin.du@intel.com writes:
> From: "Du, Changbin" <changbin.du@intel.com>
>
> dwc->regset is allocated on dwc3_debugfs_init, and should
> be released on init failure or dwc3_debugfs_exit. Btw,
> The line "dwc->root = NULL" is unnecessary, so remove it.
>
> Signed-off-by: Du, Changbin <changbin.du@intel.com>
> ---
> v2:
> Title changed;
> free dwc->regset on failure path.
>
> ---
> drivers/usb/dwc3/debugfs.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/usb/dwc3/debugfs.c b/drivers/usb/dwc3/debugfs.c
> index 9ac37fe..abd8889 100644
> --- a/drivers/usb/dwc3/debugfs.c
> +++ b/drivers/usb/dwc3/debugfs.c
> @@ -678,7 +678,8 @@ int dwc3_debugfs_init(struct dwc3 *dwc)
>
> err1:
> debugfs_remove_recursive(root);
> -
> + if (!dwc->regset)
> + kfree(dwc->regset);
IOW:
if regset is NULL, free NULL.
This check is wrong and unnecessary ;-) kfree(NULL) is safe.
--
balbi
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 818 bytes --]
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v3] usb: dwc3: fix memory leak of dwc->regset
2016-04-12 8:24 ` [PATCH v3] " changbin.du
@ 2016-04-12 10:09 ` Felipe Balbi
0 siblings, 0 replies; 14+ messages in thread
From: Felipe Balbi @ 2016-04-12 10:09 UTC (permalink / raw)
To: changbin.du; +Cc: gregkh, linux-usb, linux-kernel, Du, Changbin
[-- Attachment #1: Type: text/plain, Size: 2336 bytes --]
Hi,
changbin.du@intel.com writes:
> From: "Du, Changbin" <changbin.du@intel.com>
>
> dwc->regset is allocated on dwc3_debugfs_init, and should
> be released on init failure or dwc3_debugfs_exit. Btw,
> The line "dwc->root = NULL" is unnecessary, so remove it.
>
> Signed-off-by: Du, Changbin <changbin.du@intel.com>
> ---
I've modifed the patch a little bit just to make sure the error labels
are easy to follow:
commit c7232c5fdcc5e1b6fc39b498b58573553ebb43da
Author: Du, Changbin <changbin.du@intel.com>
Date: Tue Apr 12 16:24:34 2016 +0800
usb: dwc3: fix memory leak of dwc->regset
dwc->regset is allocated on dwc3_debugfs_init, and should
be released on init failure or dwc3_debugfs_exit. Btw,
The line "dwc->root = NULL" is unnecessary, so remove it.
Signed-off-by: Du, Changbin <changbin.du@intel.com>
[ felipe.balbi@linux.intel.com : add another err label for the new
error condition ]
Signed-off-by: Felipe Balbi <felipe.balbi@linux.intel.com>
diff --git a/drivers/usb/dwc3/debugfs.c b/drivers/usb/dwc3/debugfs.c
index 9ac37fe1b6a7..cebf9e38b60a 100644
--- a/drivers/usb/dwc3/debugfs.c
+++ b/drivers/usb/dwc3/debugfs.c
@@ -645,7 +645,7 @@ int dwc3_debugfs_init(struct dwc3 *dwc)
file = debugfs_create_regset32("regdump", S_IRUGO, root, dwc->regset);
if (!file) {
ret = -ENOMEM;
- goto err1;
+ goto err2;
}
if (IS_ENABLED(CONFIG_USB_DWC3_DUAL_ROLE)) {
@@ -653,7 +653,7 @@ int dwc3_debugfs_init(struct dwc3 *dwc)
dwc, &dwc3_mode_fops);
if (!file) {
ret = -ENOMEM;
- goto err1;
+ goto err2;
}
}
@@ -663,19 +663,22 @@ int dwc3_debugfs_init(struct dwc3 *dwc)
dwc, &dwc3_testmode_fops);
if (!file) {
ret = -ENOMEM;
- goto err1;
+ goto err2;
}
file = debugfs_create_file("link_state", S_IRUGO | S_IWUSR, root,
dwc, &dwc3_link_state_fops);
if (!file) {
ret = -ENOMEM;
- goto err1;
+ goto err2;
}
}
return 0;
+err2:
+ kfree(dwc->regset);
+
err1:
debugfs_remove_recursive(root);
@@ -686,5 +689,5 @@ err0:
void dwc3_debugfs_exit(struct dwc3 *dwc)
{
debugfs_remove_recursive(dwc->root);
- dwc->root = NULL;
+ kfree(dwc->regset);
}
patch is longer but functionally equivalent.
--
balbi
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 818 bytes --]
^ permalink raw reply related [flat|nested] 14+ messages in thread
end of thread, other threads:[~2016-04-12 10:11 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-04-08 9:42 [PATCH] usb: dwc3: free dwc->regset on dwc3_debugfs_exit changbin.du
2016-04-11 8:06 ` Felipe Balbi
2016-04-11 11:09 ` Du, Changbin
2016-04-11 11:13 ` Felipe Balbi
2016-04-11 11:38 ` Du, Changbin
2016-04-11 12:09 ` Felipe Balbi
2016-04-12 2:10 ` Du, Changbin
2016-04-12 6:18 ` Felipe Balbi
2016-04-12 6:52 ` Du, Changbin
2016-04-12 8:06 ` Felipe Balbi
2016-04-12 8:14 ` [PATCH v2] usb: dwc3: fix memory leak of dwc->regset changbin.du
2016-04-12 8:26 ` Felipe Balbi
2016-04-12 8:24 ` [PATCH v3] " changbin.du
2016-04-12 10:09 ` Felipe Balbi
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.