Re: git: e1882428dcbb - main - ifnet/mbuf: provide KPI to serialize/restore m->m_pkthdr.rcvif

On Thu, Jan 27, 2022 at 09:11:01AM -0800, Gleb Smirnoff wrote:
T>   Mateusz,
T> 
T> On Thu, Jan 27, 2022 at 05:59:03PM +0100, Mateusz Guzik wrote:
T> M> --- trap 0xc, rip = 0xffffffff80616d98, rsp = 0xfffffe06462889e0, rbp
T> M> = 0xfffffe06462889e0 ---
T> M> m_rcvif_serialize() at m_rcvif_serialize+0x8/frame 0xfffffe06462889e0
T> M> netisr_queue_src() at netisr_queue_src+0x15c/frame 0xfffffe0646288a30
T> M> route_output() at route_output+0x890/frame 0xfffffe0646288c70
T> M> sosend_generic() at sosend_generic+0x456/frame 0xfffffe0646288d10
T> M> soo_write() at soo_write+0x43/frame 0xfffffe0646288d40
T> M> dofilewrite() at dofilewrite+0x81/frame 0xfffffe0646288d90
T> M> sys_write() at sys_write+0xbc/frame 0xfffffe0646288e00
T> M> amd64_syscall() at amd64_syscall+0x107/frame 0xfffffe0646288f30
T> M> fast_syscall_common() at fast_syscall_common+0xf8/frame 0xfffffe0646288f30
T> M> --- syscall (4, FreeBSD ELF64, sys_write), rip = 0x3b000ba6703a, rsp =
T> 
T> The breaking commit is 6871de9363e559fef6765f0e49acc47f77544999,
T> not exactly this one.
T> 
T> This means route_output() tries to queue on netisr an mbuf without
T> rcvif set. And by means of route protocol it is legitimate.
T> 
T> The question is how did that work before?
T> 
T> If we look at netisr.c before my change, we see that on dequeuing we
T> would require the rcvif for VIMAGE case:
T> 
T> netisr_process_workstream_proto():
T> 
T> 		VNET_ASSERT(m->m_pkthdr.rcvif != NULL,
T> 		    ("%s:%d rcvif == NULL: m=%p", __func__, __LINE__, m));
T> 		CURVNET_SET(m->m_pkthdr.rcvif->if_vnet);
T> 		netisr_proto[proto].np_handler(m);
T> 
T> This won't panic without VIMAGE however.
T> 
T> It also is dereferenced in netisr_drain_proto_vnet(), but apparently
T> this function is also disabled without VIMAGE.

Oh, here is the answer:

static void
rt_dispatch(struct mbuf *m, sa_family_t saf)
{
        struct m_tag *tag;

        /*              
         * Preserve the family from the sockaddr, if any, in an m_tag for
         * use when injecting the mbuf into the routing socket buffer from
         * the netisr.
         */
        if (saf != AF_UNSPEC) {
                tag = m_tag_get(PACKET_TAG_RTSOCKFAM, sizeof(unsigned short),
                    M_NOWAIT);
                if (tag == NULL) {
                        m_freem(m);
                        return;
                }
                *(unsigned short *)(tag + 1) = saf;
                m_tag_prepend(m, tag);
        }
#ifdef VIMAGE
        if (V_loif)
                m->m_pkthdr.rcvif = V_loif;
        else {
                m_freem(m);
                return;
        }
#endif
        netisr_queue(NETISR_ROUTE, m);  /* mbuf is free'd on failure. */
}

Give me 15 more minutes, I will either revert the change or make a hot fix.

-- 
Gleb Smirnoff