On Thu, Jul 30, 2020 at 09:13:46PM +0900, YASUOKA Masahiko wrote:
> Hi,
>
> sys/net/if_ethersubr.c:
> 372 void
> 373 ether_input(struct ifnet *ifp, struct mbuf *m)
> (snip)
> 519 #if NPPPOE > 0 || defined(PIPEX)
> 520 case ETHERTYPE_PPPOEDISC:
> 521 case ETHERTYPE_PPPOE:
> 522 if (m->m_flags & (M_MCAST | M_BCAST))
> 523 goto dropanyway;
> 524 #ifdef PIPEX
> 525 if (pipex_enable) {
> 526 struct pipex_session *session;
> 527
> 528 if ((session = pipex_pppoe_lookup_session(m)) !=
> NULL) {
> 529 pipex_pppoe_input(m, session);
> 530 return;
> 531 }
> 532 }
> 533 #endif
>
> previously a packet which branchces to #529 is enqueued.
>
> If the diff removes the queue, then the pipex input routine is
> executed by the NIC's interrupt handler.
>
> The queues had been made to avoid that kind of situations.
It's not enqueued in pppoe case. According pipex_pppoe_input() code we
call pipex_common_input() with `useq' argument set to '0', so we don't
enqueue mbuf(9) but pass it to pipex_ppp_input() which will pass it to
ipv{4,6}_input().
---- cut begin ----
pipex_pppoe_input(struct mbuf *m0, struct pipex_session *session)
{
int hlen;
struct pipex_pppoe_header pppoe;
NET_ASSERT_LOCKED();
/* already checked at pipex_pppoe_lookup_session */
KASSERT(m0->m_pkthdr.len >= (sizeof(struct ether_header) +
sizeof(pppoe)));
m_copydata(m0, sizeof(struct ether_header),
sizeof(struct pipex_pppoe_header), (caddr_t)&pppoe);
hlen = sizeof(struct ether_header) + sizeof(struct pipex_pppoe_header);
if ((m0 = pipex_common_input(session, m0, hlen, ntohs(pppoe.length), 0))
== NULL)
return (NULL);
m_freem(m0);
session->stat.ierrors++;
return (NULL);
}
pipex_common_input(struct pipex_session *session, struct mbuf *m0, int hlen,
int plen, int useq)
{
/* skip */
if (!useq) {
pipex_ppp_input(m0, session, 0);
return (NULL);
}
/* input ppp packets to kernel session */
if (pipex_ppp_enqueue(m0, session, &pipexinq) != 0)
goto dropped;
else
return (NULL);
---- cut end ----
We enqueue pppac(4) related mbufs, except incoming pppoe. We enqueue
pppx(4) related incoming mbufs except pppoe. We don't enqueue pppx(4)
outgoing mbufs, we don't enqueue pppoe incoming mbufs.
>
> Also I don't see a relation of the use-after-free problem and killing
> queues. Can't we fix the problem unless we kill the queues?
>
Yes we can. Reference counters allow us to keep orphan sessions in these
queues without use after free issue.
I will wait your commentaries current enqueuing before to do something.
