On Mon, Aug 15, 2022 at 04:20:57PM +0300, Vitaliy Makkoveev wrote:
> On Mon, Aug 15, 2022 at 02:36:34PM +0200, Alexander Bluhm wrote:
> > On Mon, Aug 15, 2022 at 02:29:02PM +0300, Vitaliy Makkoveev wrote:
> > > Nothing special, just get assign `inp' and `tp' from passed socket. I
> > > want to do this before (*pru_usrreq)() split to avoid huge code
> > > duplication.
> > >
> > > We have "tp might get 0 when using socket splicing" commentary. I
> > > checked where we set `inp_ppcb' to NULL and found, tcp_close() is the
> > > only place, and the following in_pcbdetach() sets `so_pcb' to NULL. So
> > > this commentary looks wrong, because we can't have TCP sockets without
> > > TCP pcb attached.
> >
> > I added the comment and check after a panic in production.
> >
> > ----------------------------
> > revision 1.109
> > date: 2012/01/03 21:50:12; author: bluhm; state: Exp; lines: +6 -2;
> > When used with socket splicing, tcp_usrreq() might get called with
> > a socket that has an inp but tp is NULL. The call stack for that
> > is tcp_input() tcp_close() soisdisconnected() sorwakeup() somove()
> > tcp_usrreq(PRU_RCVD). To avoid a NULL dereference, just return in
> > that case.
> > ok henning@
> > ----------------------------
> >
> > tcp_input() calls tcp_close() after a reset packet.
>
May be I misunderstood something, but the socket should be destroyed
after tcp_close() call:
void
in_pcbdetach(struct inpcb *inp)
{
struct socket *so = inp->inp_socket;
struct inpcbtable *table = inp->inp_table;
NET_ASSERT_LOCKED();
so->so_pcb = NULL;
/*
* As long as the NET_LOCK() is the default lock for Internet
* sockets, do not release it to not introduce new sleeping
* points.
*/
sofree(so, 1);
/* ... */
}
So it looks like the use-after-free in the somove() thread.
