diff mbox series

[ovs-dev,v2,1/2] dpif-netdev: fix memory leak in dpcls subtable set command

Message ID 20210812155735.3113912-1-harry.van.haaren@intel.com
State Accepted
Headers show
Series [ovs-dev,v2,1/2] dpif-netdev: fix memory leak in dpcls subtable set command | expand

Checks

Context Check Description
ovsrobot/apply-robot success apply and check: success
ovsrobot/github-robot-_Build_and_Test success github build: passed

Commit Message

Van Haaren, Harry Aug. 12, 2021, 3:57 p.m. UTC
This patch fixes a memory leak when the command
"dpif-netdev/subtable-lookup-prio-set" is run, the pmd_list
required to iterate the PMD threads was not being freed.
This issue was identified by a static analysis tool.

Fixes: 3d018c3e ("dpif-netdev: Add subtable lookup prio set command.")

Signed-off-by: Harry van Haaren <harry.van.haaren@intel.com>

---

Maintainers, please consider applying this patch to 2.14 and 2.15,
which are expected to have the same issue as fixed here.
---
 lib/dpif-netdev.c | 1 +
 1 file changed, 1 insertion(+)

Comments

Kevin Traynor Aug. 12, 2021, 4:20 p.m. UTC | #1
On 12/08/2021 16:57, Harry van Haaren wrote:
> This patch fixes a memory leak when the command
> "dpif-netdev/subtable-lookup-prio-set" is run, the pmd_list
> required to iterate the PMD threads was not being freed.
> This issue was identified by a static analysis tool.
> 
> Fixes: 3d018c3e ("dpif-netdev: Add subtable lookup prio set command.")
> 
> Signed-off-by: Harry van Haaren <harry.van.haaren@intel.com>
> 

LGTM,

Acked-by: Kevin Traynor <ktraynor@redhat.com>

> ---
> 
> Maintainers, please consider applying this patch to 2.14 and 2.15,
> which are expected to have the same issue as fixed here.
> ---
>  lib/dpif-netdev.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
> index 03f460c7d1..9e0d5c3103 100644
> --- a/lib/dpif-netdev.c
> +++ b/lib/dpif-netdev.c
> @@ -994,6 +994,7 @@ dpif_netdev_subtable_lookup_set(struct unixctl_conn *conn, int argc OVS_UNUSED,
>  
>          /* release port mutex before netdev mutex. */
>          ovs_mutex_unlock(&dp->port_mutex);
> +        free(pmd_list);
>      }
>      ovs_mutex_unlock(&dp_netdev_mutex);
>  
>
Stokes, Ian Aug. 16, 2021, 3:17 p.m. UTC | #2
> On 12/08/2021 16:57, Harry van Haaren wrote:
> > This patch fixes a memory leak when the command
> > "dpif-netdev/subtable-lookup-prio-set" is run, the pmd_list
> > required to iterate the PMD threads was not being freed.
> > This issue was identified by a static analysis tool.
> >
> > Fixes: 3d018c3e ("dpif-netdev: Add subtable lookup prio set command.")
> >
> > Signed-off-by: Harry van Haaren <harry.van.haaren@intel.com>
> >
> 
> LGTM,
> 
> Acked-by: Kevin Traynor <ktraynor@redhat.com>

Thanks for the review Kevin,

@Ilya, are you happy with this patch series? I can apply and backport down to 2.14 if you are ok with it?

Regards
Ian

> 
> > ---
> >
> > Maintainers, please consider applying this patch to 2.14 and 2.15,
> > which are expected to have the same issue as fixed here.
> > ---
> >  lib/dpif-netdev.c | 1 +
> >  1 file changed, 1 insertion(+)
> >
> > diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
> > index 03f460c7d1..9e0d5c3103 100644
> > --- a/lib/dpif-netdev.c
> > +++ b/lib/dpif-netdev.c
> > @@ -994,6 +994,7 @@ dpif_netdev_subtable_lookup_set(struct unixctl_conn
> *conn, int argc OVS_UNUSED,
> >
> >          /* release port mutex before netdev mutex. */
> >          ovs_mutex_unlock(&dp->port_mutex);
> > +        free(pmd_list);
> >      }
> >      ovs_mutex_unlock(&dp_netdev_mutex);
> >
> >
> 
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Ilya Maximets Aug. 16, 2021, 3:23 p.m. UTC | #3
On 8/16/21 5:17 PM, Stokes, Ian wrote:
>> On 12/08/2021 16:57, Harry van Haaren wrote:
>>> This patch fixes a memory leak when the command
>>> "dpif-netdev/subtable-lookup-prio-set" is run, the pmd_list
>>> required to iterate the PMD threads was not being freed.
>>> This issue was identified by a static analysis tool.
>>>
>>> Fixes: 3d018c3e ("dpif-netdev: Add subtable lookup prio set command.")
>>>
>>> Signed-off-by: Harry van Haaren <harry.van.haaren@intel.com>
>>>
>>
>> LGTM,
>>
>> Acked-by: Kevin Traynor <ktraynor@redhat.com>
> 
> Thanks for the review Kevin,
> 
> @Ilya, are you happy with this patch series? I can apply and backport down to 2.14 if you are ok with it?

Both patches looks OK to me.

Best regards, Ilya Maximets.

> 
> Regards
> Ian
> 
>>
>>> ---
>>>
>>> Maintainers, please consider applying this patch to 2.14 and 2.15,
>>> which are expected to have the same issue as fixed here.
>>> ---
>>>  lib/dpif-netdev.c | 1 +
>>>  1 file changed, 1 insertion(+)
>>>
>>> diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
>>> index 03f460c7d1..9e0d5c3103 100644
>>> --- a/lib/dpif-netdev.c
>>> +++ b/lib/dpif-netdev.c
>>> @@ -994,6 +994,7 @@ dpif_netdev_subtable_lookup_set(struct unixctl_conn
>> *conn, int argc OVS_UNUSED,
>>>
>>>          /* release port mutex before netdev mutex. */
>>>          ovs_mutex_unlock(&dp->port_mutex);
>>> +        free(pmd_list);
>>>      }
>>>      ovs_mutex_unlock(&dp_netdev_mutex);
>>>
>>>
>>
>> _______________________________________________
>> dev mailing list
>> dev@openvswitch.org
>> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Stokes, Ian Aug. 16, 2021, 3:44 p.m. UTC | #4
> On 8/16/21 5:17 PM, Stokes, Ian wrote:
> >> On 12/08/2021 16:57, Harry van Haaren wrote:
> >>> This patch fixes a memory leak when the command
> >>> "dpif-netdev/subtable-lookup-prio-set" is run, the pmd_list
> >>> required to iterate the PMD threads was not being freed.
> >>> This issue was identified by a static analysis tool.
> >>>
> >>> Fixes: 3d018c3e ("dpif-netdev: Add subtable lookup prio set command.")
> >>>
> >>> Signed-off-by: Harry van Haaren <harry.van.haaren@intel.com>
> >>>
> >>
> >> LGTM,
> >>
> >> Acked-by: Kevin Traynor <ktraynor@redhat.com>
> >
> > Thanks for the review Kevin,
> >
> > @Ilya, are you happy with this patch series? I can apply and backport down to
> 2.14 if you are ok with it?
> 
> Both patches looks OK to me.
> 
> Best regards, Ilya Maximets.

Perfect, thanks. Applied both patches to master and 2.16, and backported this patch as far as 2.14.

BR
Ian
> 
> >
> > Regards
> > Ian
> >
> >>
> >>> ---
> >>>
> >>> Maintainers, please consider applying this patch to 2.14 and 2.15,
> >>> which are expected to have the same issue as fixed here.
> >>> ---
> >>>  lib/dpif-netdev.c | 1 +
> >>>  1 file changed, 1 insertion(+)
> >>>
> >>> diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
> >>> index 03f460c7d1..9e0d5c3103 100644
> >>> --- a/lib/dpif-netdev.c
> >>> +++ b/lib/dpif-netdev.c
> >>> @@ -994,6 +994,7 @@ dpif_netdev_subtable_lookup_set(struct
> unixctl_conn
> >> *conn, int argc OVS_UNUSED,
> >>>
> >>>          /* release port mutex before netdev mutex. */
> >>>          ovs_mutex_unlock(&dp->port_mutex);
> >>> +        free(pmd_list);
> >>>      }
> >>>      ovs_mutex_unlock(&dp_netdev_mutex);
> >>>
> >>>
> >>
> >> _______________________________________________
> >> dev mailing list
> >> dev@openvswitch.org
> >> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
diff mbox series

Patch

diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index 03f460c7d1..9e0d5c3103 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -994,6 +994,7 @@  dpif_netdev_subtable_lookup_set(struct unixctl_conn *conn, int argc OVS_UNUSED,
 
         /* release port mutex before netdev mutex. */
         ovs_mutex_unlock(&dp->port_mutex);
+        free(pmd_list);
     }
     ovs_mutex_unlock(&dp_netdev_mutex);