Re: kern/54419: mbuf leak when deleting route

[Ryota Ozaki <ozaki.ryota%gmail.com@localhost>]

On Mon, Jul 29, 2019 at 5:35 PM <sc.dying%gmail.com@localhost> wrote:
>
> >Number:         54419
> >Category:       kern
> >Synopsis:       mbuf leak when deleting route
> >Confidential:   no
> >Severity:       serious
> >Priority:       high
> >Responsible:    kern-bug-people
> >State:          open
> >Class:          sw-bug
> >Submitter-Id:   net
> >Arrival-Date:   Mon Jul 29 08:35:00 +0000 2019
> >Originator:     sc.dying%gmail.com@localhost
> >Release:        NetBSD 8.99.51
> >Organization:
> >Environment:
> System: NetBSD  8.99.51 NetBSD 8.99.51 (GENERIC) #0: Sun Jul 28 17:41:21 UTC 2019  mkrepro%mkrepro.NetBSD.org@localhost:/usr/src/sys/arch/amd64/compile/GENERIC amd64 x86_64
> >Description:
>
> An mbuf goes somewhere when deleting a route on -current/amd64 and -8.1/amd64.
>
> When you delete a route using route command, it is deleted successfully, but
> mbuf usage of netstat -m gains 1.
> See How-To-Repeat for detail.
>
> I saw same problem on NetBSD-8.1/amd64, too.
>
> >How-To-Repeat:
>
> After boot single, I typed following commands.
>
> # uname -ap
> NetBSD  8.99.51 NetBSD 8.99.51 (GENERIC) #0: Sun Jul 28 17:41:21 UTC 2019  mkrepro%mkrepro.NetBSD.org@localhost:/usr/src/sys/arch/amd64/compile/GENERIC amd64 x86_64
> # ifconfig lo0 127.0.0.1
> # route add 192.168.0.1 127.0.0.1
> add host 192.168.0.1: gateway 127.0.0.1
> # netstat -m
> 0 mbufs in use:
> 0 calls to protocol drain routines
> # route delete 192.168.0.1
> delete host 192.168.0.1
> # netstat -m
> 1 mbufs in use:
>         1 mbufs allocated to data
> 0 calls to protocol drain routines
> # sh -c 'i=0; while [ $i -lt 10000 ]; do route -q add 192.168.0.1 127.0.0.1; route -q delete 192.168.0.1; i=$((i+1)); done'
> # netstat -m
> 10001 mbufs in use:
>         10001 mbufs allocated to data
> 0 calls to protocol drain routines
> #

The leak occurs if there is no listener of routing sockets.  That's the
reason the single user mode is needed and we don't observe the leak
normally.

>
> >Fix:
>         Yes, please.

The below patch fixes the leak.

I guess it's a typo.  Should we disallow normal users of m_freem to
use m_free accidentally by say renaming it to _m_free or something?

Thanks,
  ozaki-r

diff --git a/sys/net/raw_usrreq.c b/sys/net/raw_usrreq.c
index 24185c17939..19d96dcb82b 100644
--- a/sys/net/raw_usrreq.c
+++ b/sys/net/raw_usrreq.c
@@ -110,7 +110,7 @@ raw_input(struct mbuf *m0, struct sockproto
*proto, struct sockaddr *src,
        }
        if (last != NULL) {
                if (sbappendaddr(&last->so_rcv, src, m, NULL) == 0) {
-                       m_free(m);
+                       m_freem(m);
                        soroverflow(last);
                } else
                        sorwakeup(last);