[PATCH] realtek: fix memory leak in netevent handler

[Sander Vanheule]

Hi Jan,

On Wed, 2023-02-08 at 22:53 +0100, Jan Hoffmann wrote:
> The net_event_work struct is allocated, but only freed in a single case.
> Move the allocation to the branch where it is actually needed, and free
> it after the work has been done.
> 
> Fixes: 03e1d93e0779 ("realtek: add driver support for routing offload")
> Signed-off-by: Jan Hoffmann <jan at 3e8.eu>
> ---
> I noticed this issue due to increasing memory usage on a HPE 1920-16G.
> While I haven't done any long-term testing with this patch yet, using
> kmemleak on a HPE 1920-8G no longer shows any leaks in the realtek
> drivers.

Thanks for noticing and taking care of fixing it!

> 
>  .../files-5.10/drivers/net/dsa/rtl83xx/common.c | 17 +++++++++--------
>  .../files-5.15/drivers/net/dsa/rtl83xx/common.c | 17 +++++++++--------
>  2 files changed, 18 insertions(+), 16 deletions(-)
> 
> diff --git a/target/linux/realtek/files-5.10/drivers/net/dsa/rtl83xx/common.c
> b/target/linux/realtek/files-5.10/drivers/net/dsa/rtl83xx/common.c
> index 15e6ed092696..d2d677230010 100644
> --- a/target/linux/realtek/files-5.10/drivers/net/dsa/rtl83xx/common.c
> +++ b/target/linux/realtek/files-5.10/drivers/net/dsa/rtl83xx/common.c
> @@ -1286,6 +1286,8 @@ static void rtl83xx_net_event_work_do(struct work_struct *work)
>         struct rtl838x_switch_priv *priv = net_work->priv;
>  
>         rtl83xx_l3_nexthop_update(priv, net_work->gw_addr, net_work->mac);
> +
> +       kfree(net_work);
>  }
>  
>  static int rtl83xx_netevent_event(struct notifier_block *this,
> @@ -1299,13 +1301,6 @@ static int rtl83xx_netevent_event(struct notifier_block *this,
>  
>         priv = container_of(this, struct rtl838x_switch_priv, ne_nb);
>  
> -       net_work = kzalloc(sizeof(*net_work), GFP_ATOMIC);
> -       if (!net_work)
> -               return NOTIFY_BAD;
> -
> -       INIT_WORK(&net_work->work, rtl83xx_net_event_work_do);
> -       net_work->priv = priv;
> -
>         switch (event) {
>         case NETEVENT_NEIGH_UPDATE:
>                 if (n->tbl != &arp_tbl)
> @@ -1314,10 +1309,16 @@ static int rtl83xx_netevent_event(struct notifier_block *this,
>                 port = rtl83xx_port_dev_lower_find(dev, priv);
>                 if (port < 0 || !(n->nud_state & NUD_VALID)) {
>                         pr_debug("%s: Neigbour invalid, not updating\n", __func__);
> -                       kfree(net_work);
>                         return NOTIFY_DONE;
>                 }
>  
> +               net_work = kzalloc(sizeof(*net_work), GFP_ATOMIC);
> +               if (!net_work)
> +                       return NOTIFY_BAD;
> +
> +               INIT_WORK(&net_work->work, rtl83xx_net_event_work_do);
> +               net_work->priv = priv;
> +
>                 net_work->mac = ether_addr_to_u64(n->ha);
>                 net_work->gw_addr = *(__be32 *) n->primary_key;

rtl83xx_netevent_event() has a variable err which isn't ever assigned, so I think there's
a bit of dead code at the end. Otherwise I think this change is good, and err could be
removed in another patch.

Best,
Sander