Re: [PATCH v2 3/3] pppoatm: protect against freeing of vcc

[David Woodhouse]

On Thu, 2012-11-29 at 12:17 -0500, chas williams - CONTRACTOR wrote:
> most atm hardware that i am familiar with, wont deliver vpi/vci data
> unless you are actually trying to receive it.  however, this hardware
> is generally doing its reassembly in hardware and delivering aal5
> pdu's and needs to manage its memory resources carefully.  you might be
> trying to reassemble 1000 pdu's from different vpi/vci's.

In almost *all* cases, there is only one VCC. So much so, in fact, that
I only just realised its firmware seems to crash if you send it a
PKT_POPEN for a new VPI/VCI pair while it's already got one open. But
yes, I think it does actually work in the same way.

We do see the 'packet received for unknown VCC' complaint, after we
reboot the host without resetting the card. And as shown in the commit I
just referenced, we rely on the !ATM_VF_READY check in order to prevent
a use-after-free when the tasklet is sending packets up a VCC that's
just been closed.

> > So bearing that in mind: from the moment ATM_VF_READY gets cleared, as
> > far as the ATM core is concerned, we will no longer receive packets on
> > the given VCC. If we receive any, we'll just complain about receiving
> > packets for an unknown VCI/VPI.
> > 
> > For the TX case ... yes, we need to be sure we aren't continuing to send
> > packets after our close() routine completes. We *used* to, but the
> > resulting ->pop() calls were causing problems, and that's why we're
> > looking at this code path closer. The currently proposed patches (except
> > one suggestion from Krzyztof that we both shouted down) would fix that.
> 
> again part of this is poor synchronization.  the detach protocol (i.e.
> push of a NULL skb) should be flushing any pending transmits and
> shutting down whatever in the protocol is doing any sending and
> receiving.  however, i release this might be difficult to do since the
> detach protocol is invoked in such a strange way.

You mean that (e.g.) pppoatm_push(vcc, NULL) should be waiting for any
pending TX skb (that has already been passed off to the driver) to
*complete*? How would it even do that?

-- 
dwmw2

Attachment: smime.p7s
Description: S/MIME cryptographic signature