Re: athn(4) hostap: mem leak

[Benjamin Baier]

On Sat, 1 Dec 2018 15:48:13 -0200
Martin Pieuchot <m...@openbsd.org> wrote:

> On 30/11/18(Fri) 13:49, Benjamin Baier wrote:
> > Hi
> > 
> > There is a leak of *arg in 
> > dev/usb/if_athn_usb.c:athn_usb_newauth() line 1263
> > since Rev. 1.49
> > Because athn_usb_do_async() memcpy's the argument anyway.
> > 
> > Found with llvm/scan-build.
> > 
> > Instead of adding free(arg) I opted to make this function
> > more like the other ones which call athn_usb_do_async.
> > 
> > Only compile tested... looking for tests.  
> 
> You should also remove the free(arg...) in athn_usb_newauth_cb().
Indeed, new patch attached.

Index: if_athn_usb.c
===================================================================
RCS file: /cvs/src/sys/dev/usb/if_athn_usb.c,v
retrieving revision 1.51
diff -u -p -r1.51 if_athn_usb.c
--- if_athn_usb.c       6 Sep 2018 11:50:54 -0000       1.51
+++ if_athn_usb.c       2 Dec 2018 09:09:29 -0000
@@ -1202,8 +1202,6 @@ athn_usb_newauth_cb(struct athn_usb_soft
        struct athn_node *an = (struct athn_node *)ni;
        int s, error = 0;
 
-       free(arg, M_DEVBUF, sizeof(*arg));
-
        if (ic->ic_state != IEEE80211_S_RUN)
                return;
 
@@ -1231,7 +1229,7 @@ athn_usb_newauth(struct ieee80211com *ic
        struct ifnet *ifp = &ic->ic_if;
        struct athn_node *an = (struct athn_node *)ni;
        int nsta;
-       struct athn_usb_newauth_cb_arg *arg;
+       struct athn_usb_newauth_cb_arg arg;
 
        if (ic->ic_opmode != IEEE80211_M_HOSTAP)
                return 0;
@@ -1254,12 +1252,9 @@ athn_usb_newauth(struct ieee80211com *ic
         * In a process context, try to add this node to the
         * firmware table and confirm the AUTH request.
         */
-       arg = malloc(sizeof(*arg), M_DEVBUF, M_NOWAIT);
-       if (arg == NULL)
-               return ENOMEM;
-       arg->ni = ieee80211_ref_node(ni);
-       arg->seq = seq;
-       athn_usb_do_async(usc, athn_usb_newauth_cb, arg, sizeof(*arg));
+       arg.ni = ieee80211_ref_node(ni);
+       arg.seq = seq;
+       athn_usb_do_async(usc, athn_usb_newauth_cb, &arg, sizeof(arg));
        return EBUSY;
 #else
        return 0;