Re: [PATCH v2] usb: phy: phy-tahvo: fix memory leak in tahvo_usb_probe()

Re: [PATCH v2] usb: phy: phy-tahvo: fix memory leak in tahvo_usb_probe()

[Greg KH]

On Tue, Apr 25, 2023 at 10:58:51AM +0800, ktestrobot@126.com wrote:
> Hi, Li Yang
> This email is automatically replied by KTestRobot(Beta). Please do not reply to this email.

But I will!

> If you have any questions or suggestions about KTestRobot, please contact ZhongYong <U201911841@hust.edu.cn>

First question, why are you responding from an email that is not allowed
to be responded to and forced to have us manually add an address?
That's not very nice or helpful for us who have to see these messages,
please fix that.

> --- Changed Paths ---
> drivers/usb/phy/phy-tahvo.c
> --- Log Message ---
> Smatch reports:
> drivers/usb/phy/phy-tahvo.c: tahvo_usb_probe()
> warn: missing unwind goto?
> 
> After geting irq, if ret < 0, it will return without error handling to
> free memory.
> Just add error handling to fix this problem.
> 
> --- Test Result ---
> *** CheckPatch	PASS ***
> *** CheckSmatch	PASS ***
> *** ApplyToLinuxNext	PASS ***

Why is only "does the patch apply" matter for this bot?  What is it
supposed to be doing here?  Is it going to run on every single patch to
this mailing list, and if so, how can we turn it off as "it applied!" is
a very very low bar to be testing something for, and we already have a
bot that does this.

thanks,

greg k-h

Re: [PATCH v2] usb: phy: phy-tahvo: fix memory leak in tahvo_usb_probe()

[Dongliang Mu]

On 2023/4/25 13:15, Greg KH wrote:
> On Tue, Apr 25, 2023 at 10:58:51AM +0800, ktestrobot@126.com wrote:
>> Hi, Li Yang
>> This email is automatically replied by KTestRobot(Beta). Please do not reply to this email.
> But I will!
>
>> If you have any questions or suggestions about KTestRobot, please contact ZhongYong <U201911841@hust.edu.cn>
> First question, why are you responding from an email that is not allowed
> to be responded to and forced to have us manually add an address?
> That's not very nice or helpful for us who have to see these messages,
> please fix that.

Hi greg,

thanks for your feedback.

This robot is only used for our internal patch review. It should not 
send any email out to Linux kernel mailing list.

Why does this situation (this testing robot mistakenly sent emails to 
the kernel mailing list) occur?

Because Zhong Yong who is responsible for this testing robot does not 
classify patches which are sent to kernel mailing list and which are 
still in the internal review.

Sorry about this false alarm.

>
>> --- Changed Paths ---
>> drivers/usb/phy/phy-tahvo.c
>> --- Log Message ---
>> Smatch reports:
>> drivers/usb/phy/phy-tahvo.c: tahvo_usb_probe()
>> warn: missing unwind goto?
>>
>> After geting irq, if ret < 0, it will return without error handling to
>> free memory.
>> Just add error handling to fix this problem.
>>
>> --- Test Result ---
>> *** CheckPatch	PASS ***
>> *** CheckSmatch	PASS ***
>> *** ApplyToLinuxNext	PASS ***
> Why is only "does the patch apply" matter for this bot?  What is it
> supposed to be doing here?  Is it going to run on every single patch to
> this mailing list,

Because we have many newbie students, from first grade to forth grade in 
our university. They sometimes make mistakes in the patch generation, 
crafting patches that cannot be applied in the kernel tree.

This robot will check all the patches sent to this internal mailing list 
and report this issue. **But our design is only to use it for internal 
review. It should not send any emails to kernel mailing list.**

> and if so, how can we turn it off as "it applied!" is
> a very very low bar to be testing something for, and we already have a
> bot that does this.

This bot is only used for our internal review mailing list, and 
**again** it will not send any emails to kernel maintainers.

It is because the student missed an important logic: filter out the 
patches that are already sent for external review.

Really sorry about this situation.

Dongliang Mu

>
> thanks,
>
> greg k-h

Re: [PATCH v2] usb: phy: phy-tahvo: fix memory leak in tahvo_usb_probe()

[Greg KH]

On Tue, Apr 25, 2023 at 01:32:35PM +0800, Dongliang Mu wrote:
> 
> On 2023/4/25 13:15, Greg KH wrote:
> > On Tue, Apr 25, 2023 at 10:58:51AM +0800, ktestrobot@126.com wrote:
> > > Hi, Li Yang
> > > This email is automatically replied by KTestRobot(Beta). Please do not reply to this email.
> > But I will!
> > 
> > > If you have any questions or suggestions about KTestRobot, please contact ZhongYong <U201911841@hust.edu.cn>
> > First question, why are you responding from an email that is not allowed
> > to be responded to and forced to have us manually add an address?
> > That's not very nice or helpful for us who have to see these messages,
> > please fix that.
> 
> Hi greg,
> 
> thanks for your feedback.
> 
> This robot is only used for our internal patch review. It should not send
> any email out to Linux kernel mailing list.

But it did :(

> Sorry about this false alarm.

It happened more than once here, so please be more careful with the bot.

thanks,

greg k-h

Re: [PATCH v2] usb: phy: phy-tahvo: fix memory leak in tahvo_usb_probe()

[Dongliang Mu]

On 2023/4/25 13:50, Greg KH wrote:
> On Tue, Apr 25, 2023 at 01:32:35PM +0800, Dongliang Mu wrote:
>> On 2023/4/25 13:15, Greg KH wrote:
>>> On Tue, Apr 25, 2023 at 10:58:51AM +0800, ktestrobot@126.com wrote:
>>>> Hi, Li Yang
>>>> This email is automatically replied by KTestRobot(Beta). Please do not reply to this email.
>>> But I will!
>>>
>>>> If you have any questions or suggestions about KTestRobot, please contact ZhongYong <U201911841@hust.edu.cn>
>>> First question, why are you responding from an email that is not allowed
>>> to be responded to and forced to have us manually add an address?
>>> That's not very nice or helpful for us who have to see these messages,
>>> please fix that.
>> Hi greg,
>>
>> thanks for your feedback.
>>
>> This robot is only used for our internal patch review. It should not send
>> any email out to Linux kernel mailing list.
> But it did :(
>
>> Sorry about this false alarm.
> It happened more than once here, so please be more careful with the bot.

Sure. All the emails sent to kernel mailing list are from the same time 
period. It results from the same development mistake. We will be more 
careful in the future.

Sorry again for our mistake.

BTW, should I reply to the affected mailing list and explain the 
situation? Or just leave it alone.

Dongliang Mu

> thanks,
>
> greg k-h

[PATCH v2] usb: phy: phy-tahvo: fix memory leak in tahvo_usb_probe()

[Li Yang]

Smatch reports:
drivers/usb/phy/phy-tahvo.c: tahvo_usb_probe()
warn: missing unwind goto?

After geting irq, if ret < 0, it will return without error handling to
free memory.
Just add error handling to fix this problem.

Fixes: 0d45a1373e66 ("usb: phy: tahvo: add IRQ check")
Signed-off-by: Li Yang <lidaxian@hust.edu.cn>
Reviewed-by: Dongliang Mu <dzm91@hust.edu.cn>
---
v1->v2: Remove unnecessary dev_err().
The issue is found by static analysis, and the patch is remains untested.
---
 drivers/usb/phy/phy-tahvo.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/phy/phy-tahvo.c b/drivers/usb/phy/phy-tahvo.c
index f2d2cc586c5b..da63d7e4d270 100644
--- a/drivers/usb/phy/phy-tahvo.c
+++ b/drivers/usb/phy/phy-tahvo.c
@@ -391,7 +391,7 @@ static int tahvo_usb_probe(struct platform_device *pdev)
 
 	tu->irq = ret = platform_get_irq(pdev, 0);
 	if (ret < 0)
-		return ret;
+		goto err_remove_phy;
 	ret = request_threaded_irq(tu->irq, NULL, tahvo_usb_vbus_interrupt,
 				   IRQF_ONESHOT,
 				   "tahvo-vbus", tu);
-- 
2.34.1