[Bug 299325] LinuxKPI 802.11: linuxkpi_ieee80211_rx() leaks a node reference on its error paths

https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=299325

            Bug ID: 299325
           Summary: LinuxKPI 802.11: linuxkpi_ieee80211_rx() leaks a node
                    reference on its error paths
           Product: Base System
           Version: CURRENT
          Hardware: amd64
                OS: Any
            Status: New
          Severity: Affects Only Me
          Priority: ---
         Component: kern
          Assignee: bugs@FreeBSD.org
          Reporter: fbsdbugs@westryn.net

Created attachment 275615
  --> https://bugs.freebsd.org/bugzilla/attachment.cgi?id=275615&action=edit
release node reference on error

While working on AP mode for ath10k on top of LinuxKPI I was reading through
linuxkpi_ieee80211_rx() and noticed that the node reference it takes is never
dropped if the frame doesn't make it onto the deferred RX queue.

The function gets a referenced node early on, either from the driver's sta
(ieee80211_ref_node(lsta->ni), line 8417 in main) or from
ieee80211_find_rxnode() (line 8422).  On success that reference rides along
with the mbuf and lkpi_80211_lhw_rxq_rx_one() releases it after input.  But
there are four ways out after the reference is taken that jump to err: and
free the mbuf without releasing it:

  - line 8434: ieee80211_add_rx_params() fails
  - line 8530: m_tag_alloc() fails (LKPI_80211_USE_MTAG only)
  - line 8545: the rx queue is already stopped
  - line 8558: mbufq_enqueue() fails because the queue is full

err: only frees the skb, so each of these leaks the node, and with it the
driver's lsta.

I found this by reading the code, not by hitting it.  All four paths are
uncommon (allocation failure, a full 2048-frame queue, or frames arriving while
the hardware is being stopped), so I'd expect it to show up, if ever, as a node
that never goes away after a stop or under heavy load rather than anything
dramatic.

## Fix

The attached patch initializes ni to NULL, clears it once the frame has been
queued (the queue owns the reference from then on), and releases it at err:
if it's still set.  It doesn't touch the success path.

While I was there I also changed the debug message on the enqueue-failure
path.  Under D80211_TRACE_RX it's printed with ic_printf(ni->ni_ic, ...), but
ni is NULL for any frame from an unknown sender, so it uses the local ic
instead.

I've built and booted a kernel with this applied, but I haven't been able to
exercise the error paths themselves.

-- 
You are receiving this mail because:
You are the assignee for the bug.