When mdiobus_register() fails, priv->mdio allocated
by mdiobus_alloc() has not been freed, which leads
to memleak.
Signed-off-by: Dinghao Liu <[email protected]>
---
drivers/net/ethernet/ethoc.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/ethoc.c b/drivers/net/ethernet/ethoc.c
index 0981fe9652e5..3d9b0b161e24 100644
--- a/drivers/net/ethernet/ethoc.c
+++ b/drivers/net/ethernet/ethoc.c
@@ -1211,7 +1211,7 @@ static int ethoc_probe(struct platform_device *pdev)
ret = mdiobus_register(priv->mdio);
if (ret) {
dev_err(&netdev->dev, "failed to register MDIO bus\n");
- goto free2;
+ goto free3;
}
ret = ethoc_mdio_probe(netdev);
@@ -1243,6 +1243,7 @@ static int ethoc_probe(struct platform_device *pdev)
netif_napi_del(&priv->napi);
error:
mdiobus_unregister(priv->mdio);
+free3:
mdiobus_free(priv->mdio);
free2:
clk_disable_unprepare(priv->clk);
--
2.17.1
On Wed, Dec 23, 2020 at 07:06:12PM +0800, Dinghao Liu wrote:
> When mdiobus_register() fails, priv->mdio allocated
> by mdiobus_alloc() has not been freed, which leads
> to memleak.
>
> Signed-off-by: Dinghao Liu <[email protected]>
Fixes: bfa49cfc5262 ("net/ethoc: fix null dereference on error exit path")
Reviewed-by: Andrew Lunn <[email protected]>
Andrew
On Wed, 23 Dec 2020 16:33:04 +0100 Andrew Lunn wrote:
> On Wed, Dec 23, 2020 at 07:06:12PM +0800, Dinghao Liu wrote:
> > When mdiobus_register() fails, priv->mdio allocated
> > by mdiobus_alloc() has not been freed, which leads
> > to memleak.
> >
> > Signed-off-by: Dinghao Liu <[email protected]>
>
> Fixes: bfa49cfc5262 ("net/ethoc: fix null dereference on error exit path")
>
> Reviewed-by: Andrew Lunn <[email protected]>
Ooof, I applied without looking at your email and I added:
Fixes: e7f4dc3536a4 ("mdio: Move allocation of interrupts into core")
I believe this is the correct Fixes tag. Before the exit patch was
freeing both MDIO and the IRQ depending on the fact that kfree(NULL)
is fine. Am I wrong? Not that we can fix it now :)
On Wed, Dec 23, 2020 at 12:32:18PM -0800, Jakub Kicinski wrote:
> On Wed, 23 Dec 2020 16:33:04 +0100 Andrew Lunn wrote:
> > On Wed, Dec 23, 2020 at 07:06:12PM +0800, Dinghao Liu wrote:
> > > When mdiobus_register() fails, priv->mdio allocated
> > > by mdiobus_alloc() has not been freed, which leads
> > > to memleak.
> > >
> > > Signed-off-by: Dinghao Liu <[email protected]>
> >
> > Fixes: bfa49cfc5262 ("net/ethoc: fix null dereference on error exit path")
> >
> > Reviewed-by: Andrew Lunn <[email protected]>
>
> Ooof, I applied without looking at your email and I added:
>
> Fixes: e7f4dc3536a4 ("mdio: Move allocation of interrupts into core")
[Goes and looks deeper]
Yes, commit e7f4dc3536a4 looks like it introduced the original
problem. bfa49cfc5262 just moved to code around a bit.
Does patchwork not automagically add Fixes: lines from full up emails?
That seems like a reasonable automation.
Andrew
On Wed, 23 Dec 2020 22:00:44 +0100 Andrew Lunn wrote:
> On Wed, Dec 23, 2020 at 12:32:18PM -0800, Jakub Kicinski wrote:
> > On Wed, 23 Dec 2020 16:33:04 +0100 Andrew Lunn wrote:
> > > On Wed, Dec 23, 2020 at 07:06:12PM +0800, Dinghao Liu wrote:
> > > > When mdiobus_register() fails, priv->mdio allocated
> > > > by mdiobus_alloc() has not been freed, which leads
> > > > to memleak.
> > > >
> > > > Signed-off-by: Dinghao Liu <[email protected]>
> > >
> > > Fixes: bfa49cfc5262 ("net/ethoc: fix null dereference on error exit path")
> > >
> > > Reviewed-by: Andrew Lunn <[email protected]>
> >
> > Ooof, I applied without looking at your email and I added:
> >
> > Fixes: e7f4dc3536a4 ("mdio: Move allocation of interrupts into core")
>
> [Goes and looks deeper]
>
> Yes, commit e7f4dc3536a4 looks like it introduced the original
> problem. bfa49cfc5262 just moved to code around a bit.
>
> Does patchwork not automagically add Fixes: lines from full up emails?
> That seems like a reasonable automation.
Looks like it's been a TODO for 3 years now:
https://github.com/getpatchwork/patchwork/issues/151
:(
On 12/23/2020 1:11 PM, Jakub Kicinski wrote:
> On Wed, 23 Dec 2020 22:00:44 +0100 Andrew Lunn wrote:
>> On Wed, Dec 23, 2020 at 12:32:18PM -0800, Jakub Kicinski wrote:
>>> On Wed, 23 Dec 2020 16:33:04 +0100 Andrew Lunn wrote:
>>>> On Wed, Dec 23, 2020 at 07:06:12PM +0800, Dinghao Liu wrote:
>>>>> When mdiobus_register() fails, priv->mdio allocated
>>>>> by mdiobus_alloc() has not been freed, which leads
>>>>> to memleak.
>>>>>
>>>>> Signed-off-by: Dinghao Liu <[email protected]>
>>>>
>>>> Fixes: bfa49cfc5262 ("net/ethoc: fix null dereference on error exit path")
>>>>
>>>> Reviewed-by: Andrew Lunn <[email protected]>
>>>
>>> Ooof, I applied without looking at your email and I added:
>>>
>>> Fixes: e7f4dc3536a4 ("mdio: Move allocation of interrupts into core")
>>
>> [Goes and looks deeper]
>>
>> Yes, commit e7f4dc3536a4 looks like it introduced the original
>> problem. bfa49cfc5262 just moved to code around a bit.
>>
>> Does patchwork not automagically add Fixes: lines from full up emails?
>> That seems like a reasonable automation.
>
> Looks like it's been a TODO for 3 years now:
>
> https://github.com/getpatchwork/patchwork/issues/151
It was proposed before, but rejected. You can have your local patchwork
admin take care of that for you though and add custom tags:
https://lists.ozlabs.org/pipermail/patchwork/2017-January/003910.html
--
Florian
On Wed, 23 Dec 2020 14:17:29 -0800 Florian Fainelli wrote:
> On 12/23/2020 1:11 PM, Jakub Kicinski wrote:
> > On Wed, 23 Dec 2020 22:00:44 +0100 Andrew Lunn wrote:
> >> On Wed, Dec 23, 2020 at 12:32:18PM -0800, Jakub Kicinski wrote:
> >>> On Wed, 23 Dec 2020 16:33:04 +0100 Andrew Lunn wrote:
> >>>> On Wed, Dec 23, 2020 at 07:06:12PM +0800, Dinghao Liu wrote:
> >>>>> When mdiobus_register() fails, priv->mdio allocated
> >>>>> by mdiobus_alloc() has not been freed, which leads
> >>>>> to memleak.
> >>>>>
> >>>>> Signed-off-by: Dinghao Liu <[email protected]>
> >>>>
> >>>> Fixes: bfa49cfc5262 ("net/ethoc: fix null dereference on error exit path")
> >>>>
> >>>> Reviewed-by: Andrew Lunn <[email protected]>
> >>>
> >>> Ooof, I applied without looking at your email and I added:
> >>>
> >>> Fixes: e7f4dc3536a4 ("mdio: Move allocation of interrupts into core")
> >>
> >> [Goes and looks deeper]
> >>
> >> Yes, commit e7f4dc3536a4 looks like it introduced the original
> >> problem. bfa49cfc5262 just moved to code around a bit.
> >>
> >> Does patchwork not automagically add Fixes: lines from full up emails?
> >> That seems like a reasonable automation.
> >
> > Looks like it's been a TODO for 3 years now:
> >
> > https://github.com/getpatchwork/patchwork/issues/151
>
> It was proposed before, but rejected. You can have your local patchwork
> admin take care of that for you though and add custom tags:
>
> https://lists.ozlabs.org/pipermail/patchwork/2017-January/003910.html
Konstantin, would you be willing to mod the kernel.org instance of
patchwork to populate Fixes tags in the generated mboxes?
On Wed, Dec 23, 2020 at 05:41:46PM -0800, Jakub Kicinski wrote:
> > >> Does patchwork not automagically add Fixes: lines from full up emails?
> > >> That seems like a reasonable automation.
> > >
> > > Looks like it's been a TODO for 3 years now:
> > >
> > > https://github.com/getpatchwork/patchwork/issues/151
> >
> > It was proposed before, but rejected. You can have your local patchwork
> > admin take care of that for you though and add custom tags:
> >
> > https://lists.ozlabs.org/pipermail/patchwork/2017-January/003910.html
>
> Konstantin, would you be willing to mod the kernel.org instance of
> patchwork to populate Fixes tags in the generated mboxes?
I'd really rather not -- we try not to diverge from project upstream if at all
possible, as this dramatically complicates upgrades.
-K
On 12/24/2020 10:06 AM, Konstantin Ryabitsev wrote:
> On Wed, Dec 23, 2020 at 05:41:46PM -0800, Jakub Kicinski wrote:
>>>>> Does patchwork not automagically add Fixes: lines from full up emails?
>>>>> That seems like a reasonable automation.
>>>>
>>>> Looks like it's been a TODO for 3 years now:
>>>>
>>>> https://github.com/getpatchwork/patchwork/issues/151
>>>
>>> It was proposed before, but rejected. You can have your local patchwork
>>> admin take care of that for you though and add custom tags:
>>>
>>> https://lists.ozlabs.org/pipermail/patchwork/2017-January/003910.html
>>
>> Konstantin, would you be willing to mod the kernel.org instance of
>> patchwork to populate Fixes tags in the generated mboxes?
>
> I'd really rather not -- we try not to diverge from project upstream if at all
> possible, as this dramatically complicates upgrades.
Well that is really unfortunate then because the Linux developer
community settled on using the Fixes: tag for years now and having
patchwork automatically append those tags would greatly help maintainers.
--
Florian
On Mon, Dec 28, 2020 at 01:05:26PM -0800, Florian Fainelli wrote:
> On 12/28/2020 12:23 PM, Konstantin Ryabitsev wrote:
> > On Thu, Dec 24, 2020 at 01:57:40PM -0800, Florian Fainelli wrote:
> >>>> Konstantin, would you be willing to mod the kernel.org instance of
> >>>> patchwork to populate Fixes tags in the generated mboxes?
> >>>
> >>> I'd really rather not -- we try not to diverge from project upstream if at all
> >>> possible, as this dramatically complicates upgrades.
> >>
> >> Well that is really unfortunate then because the Linux developer
> >> community settled on using the Fixes: tag for years now and having
> >> patchwork automatically append those tags would greatly help maintainers.
> >
> > I agree -- but this is something that needs to be implemented upstream.
> > Picking up a one-off patch just for patchwork.kernel.org is not the right way
> > to go about this.
>
> You should be able to tune this from the patchwork administrative
> interface and add new tags there, would not that be acceptable?
Oh, oops, I got confused by the mention of a rejected upstream patch -- I
didn't realize that this is already possible with a configuration setting.
Sure, I added a match for ^Fixes: -- let me know if it's not doing the right
thing.
-K
On 12/28/2020 12:23 PM, Konstantin Ryabitsev wrote:
> On Thu, Dec 24, 2020 at 01:57:40PM -0800, Florian Fainelli wrote:
>>>> Konstantin, would you be willing to mod the kernel.org instance of
>>>> patchwork to populate Fixes tags in the generated mboxes?
>>>
>>> I'd really rather not -- we try not to diverge from project upstream if at all
>>> possible, as this dramatically complicates upgrades.
>>
>> Well that is really unfortunate then because the Linux developer
>> community settled on using the Fixes: tag for years now and having
>> patchwork automatically append those tags would greatly help maintainers.
>
> I agree -- but this is something that needs to be implemented upstream.
> Picking up a one-off patch just for patchwork.kernel.org is not the right way
> to go about this.
You should be able to tune this from the patchwork administrative
interface and add new tags there, would not that be acceptable?
--
Florian
On Thu, Dec 24, 2020 at 01:57:40PM -0800, Florian Fainelli wrote:
> >> Konstantin, would you be willing to mod the kernel.org instance of
> >> patchwork to populate Fixes tags in the generated mboxes?
> >
> > I'd really rather not -- we try not to diverge from project upstream if at all
> > possible, as this dramatically complicates upgrades.
>
> Well that is really unfortunate then because the Linux developer
> community settled on using the Fixes: tag for years now and having
> patchwork automatically append those tags would greatly help maintainers.
I agree -- but this is something that needs to be implemented upstream.
Picking up a one-off patch just for patchwork.kernel.org is not the right way
to go about this.
-K
On Mon, 28 Dec 2020 16:14:17 -0500 Konstantin Ryabitsev wrote:
> On Mon, Dec 28, 2020 at 01:05:26PM -0800, Florian Fainelli wrote:
> > On 12/28/2020 12:23 PM, Konstantin Ryabitsev wrote:
> > > On Thu, Dec 24, 2020 at 01:57:40PM -0800, Florian Fainelli wrote:
> > >>>> Konstantin, would you be willing to mod the kernel.org instance of
> > >>>> patchwork to populate Fixes tags in the generated mboxes?
> > >>>
> > >>> I'd really rather not -- we try not to diverge from project upstream if at all
> > >>> possible, as this dramatically complicates upgrades.
> > >>
> > >> Well that is really unfortunate then because the Linux developer
> > >> community settled on using the Fixes: tag for years now and having
> > >> patchwork automatically append those tags would greatly help maintainers.
> > >
> > > I agree -- but this is something that needs to be implemented upstream.
> > > Picking up a one-off patch just for patchwork.kernel.org is not the right way
> > > to go about this.
> >
> > You should be able to tune this from the patchwork administrative
> > interface and add new tags there, would not that be acceptable?
>
> Oh, oops, I got confused by the mention of a rejected upstream patch -- I
> didn't realize that this is already possible with a configuration setting.
>
> Sure, I added a match for ^Fixes: -- let me know if it's not doing the right
> thing.
I used this one for a test:
https://patchwork.kernel.org/project/netdevbpf/patch/1609312994-121032-1-git-send-email-abaci-bugfix@linux.alibaba.com/
I'm not getting the Fixes tag when I download the mbox.
> On Mon, 28 Dec 2020 16:14:17 -0500 Konstantin Ryabitsev wrote:
> > On Mon, Dec 28, 2020 at 01:05:26PM -0800, Florian Fainelli wrote:
> > > On 12/28/2020 12:23 PM, Konstantin Ryabitsev wrote:
> > > > On Thu, Dec 24, 2020 at 01:57:40PM -0800, Florian Fainelli wrote:
> > > >>>> Konstantin, would you be willing to mod the kernel.org instance of
> > > >>>> patchwork to populate Fixes tags in the generated mboxes?
> > > >>>
> > > >>> I'd really rather not -- we try not to diverge from project upstream if at all
> > > >>> possible, as this dramatically complicates upgrades.
> > > >>
> > > >> Well that is really unfortunate then because the Linux developer
> > > >> community settled on using the Fixes: tag for years now and having
> > > >> patchwork automatically append those tags would greatly help maintainers.
> > > >
> > > > I agree -- but this is something that needs to be implemented upstream.
> > > > Picking up a one-off patch just for patchwork.kernel.org is not the right way
> > > > to go about this.
> > >
> > > You should be able to tune this from the patchwork administrative
> > > interface and add new tags there, would not that be acceptable?
> >
> > Oh, oops, I got confused by the mention of a rejected upstream patch -- I
> > didn't realize that this is already possible with a configuration setting.
> >
> > Sure, I added a match for ^Fixes: -- let me know if it's not doing the right
> > thing.
>
> I used this one for a test:
>
> https://patchwork.kernel.org/project/netdevbpf/patch/1609312994-121032-1-git-send-email-abaci-bugfix@linux.alibaba.com/
>
> I'm not getting the Fixes tag when I download the mbox.
It seems that automatically generating Fixes tags is a hard work.
Both patches and bugs could be complex. Sometimes even human cannot
determine which commit introduced a target bug.
Is this an already implemented functionality?
Regards,
Dinghao
On Wed, 6 Jan 2021 18:56:23 +0800 (GMT+08:00) [email protected]
wrote:
> > I used this one for a test:
> >
> > https://patchwork.kernel.org/project/netdevbpf/patch/1609312994-121032-1-git-send-email-abaci-bugfix@linux.alibaba.com/
> >
> > I'm not getting the Fixes tag when I download the mbox.
>
> It seems that automatically generating Fixes tags is a hard work.
> Both patches and bugs could be complex. Sometimes even human cannot
> determine which commit introduced a target bug.
>
> Is this an already implemented functionality?
I'm not sure I understand. Indeed finding the right commit to use in
a Fixes tag is not always easy, and definitely not easy to automate.
Human validation is always required.
If we could easily automate finding the commit which introduced a
problem we wouldn't need to add the explicit tag, backporters could
just run such script locally.. That's why it's best if the author
does the digging and provides the right tag.
The conversation with Konstantin and Florian was about automatically
picking up Fixes tags from the mailing list by the patchwork software,
when such tags are posted in reply to the original posting, just like
review tags. But the tags are still generated by humans.
> On Wed, 6 Jan 2021 18:56:23 +0800 (GMT+08:00) [email protected]
> wrote:
> > > I used this one for a test:
> > >
> > > https://patchwork.kernel.org/project/netdevbpf/patch/1609312994-121032-1-git-send-email-abaci-bugfix@linux.alibaba.com/
> > >
> > > I'm not getting the Fixes tag when I download the mbox.
> >
> > It seems that automatically generating Fixes tags is a hard work.
> > Both patches and bugs could be complex. Sometimes even human cannot
> > determine which commit introduced a target bug.
> >
> > Is this an already implemented functionality?
>
> I'm not sure I understand. Indeed finding the right commit to use in
> a Fixes tag is not always easy, and definitely not easy to automate.
> Human validation is always required.
>
> If we could easily automate finding the commit which introduced a
> problem we wouldn't need to add the explicit tag, backporters could
> just run such script locally.. That's why it's best if the author
> does the digging and provides the right tag.
>
> The conversation with Konstantin and Florian was about automatically
> picking up Fixes tags from the mailing list by the patchwork software,
> when such tags are posted in reply to the original posting, just like
> review tags. But the tags are still generated by humans.
It's clear to me, thanks.
Regards,
Dinghao