thanks for the fast replies. I believe you helped me very much to look
in the right direction.
On Sat, 24 Oct 2009 14:19:58 +0200 Claudio Jeker wrote:
> On Sat, Oct 24, 2009 at 01:56:51PM +0200, Christopher Zimmermann wrote:
> > panic: pool_do_get(mbpl: free list modified
>
> Free list modified means that you hit a use after free or even worse
> overflowed the previous mbuf and wrote data into the the mbuf you try to
> allocate now.
>
> More that it is writing data into memory it should not touch.
> Make sure you map the mbuf correctly into the DMA ring and remove it
> correctly from the ring before passing it on to ether_input.
Puh, all this dma stuff gives me even more headache than all this mbuf
stuff. The manpage sounds reasonable, but when reading the code in
drivers it looks all different. Is there no openBSD kernel hacking HOWTO
or some more documentation than only the manpages?
>
> > What is also very funny is that this panic is very accurately
> > reproducible. When doing a scp (big packets ?!) to produce some traffic
> > it almost always panics exactly when handling the 80th interupt for rx
> > packets.
> >
> > When doing ping (small packets ?!) it happens in the 256th packet,
> > therefore when the rx-ring is wrapping. But this may be yet another bug?
>
> This sounds like you map the mbuf as cluster without attachin a cluster to
> it and so the dma length is wrong. FreeBSD has a special function to
> return mbufs + clusters that is not implemented in OpenBSD. We on the
> other hand have MCLGETI which does the same and way more.
> I guess you need to check your newbuf function.
Yes, I read something about "mbuf cluster" is the comments. As I
understand the manpages this means the mbufs are chained together using
the m_hdr.mh_next pointer !? But as far as I understand the code in
sis_newbuf() it uses the MH_dat.m_ext. Isn't this mutually exclusive?
Actually I have no clue about those mbufs. What use is a chained cluster
to my NIC which needs a contiguous memory area, doesn't it?
> It would help to see the code in question. I bet it is just a two or three
> line change to make it work but finding can be painful (been there many
> times).
>
/*
* Initialize an RX descriptor and attach an MBUF cluster.
*/
static int
sis_newbuf(sc, i, m)
struct sis_softc *sc;
u_int32_t i;
struct mbuf *m;
{
struct sis19x_list_data *ld = &sc->sis_ldata;
struct sis19x_chain_data *cd = &sc->sis_cdata;
int error, alloc;
if (m == NULL) {
m = m_gethdr(M_DONTWAIT, MT_DATA);
if (m == NULL) {
SIS_OUT("unable to get new MBUF");
return (ENOBUFS);
}
cd->sis_rx_mbuf[i] = m;
alloc = 1;
} else {
if(cd->sis_rx_mbuf[i] != m)
SIS_OUT("m != sis_rx_mbuf[i]");
m->m_data = m->m_ext.ext_buf;
alloc = 0;
}
m->m_len = m->m_pkthdr.len = MCLBYTES;
if (alloc) {
/* So this is the call which turns the mbuf to a pointer
* usable by the NIC ? */
error = bus_dmamap_load_mbuf(sc->sis_tag, cd->sis_rx_map[i],
m, BUS_DMA_NOWAIT);
if (error) {
SIS_OUT("unable to map and load the MBUF");
m_freem(m);
return (ENOBUFS);
}
}
/* This is used both to initialize the newly created RX
* descriptor as well as for re-initializing it for reuse. */
ld->sis_rx_ring[i].sis_sts_size = 0;
ld->sis_rx_ring[i].sis_cmdsts =
htole32(OWNbit | INTbit | IPbit | TCPbit | UDPbit);
/* And here it is put into the descriptor ? */
ld->sis_rx_ring[i].sis_ptr =
htole32(cd->sis_rx_map[i]->dm_segs[0].ds_addr);
ld->sis_rx_ring[i].sis_flags =
htole32(cd->sis_rx_map[i]->dm_segs[0].ds_len);
KASSERT(cd->sis_rx_map[i]->dm_nsegs == 1,
("wrong number of segments, should be 1"));
bus_dmamap_sync(sc->sis_tag, cd->sis_rx_map[i], 0,
cd->sis_rx_map[i]->dm_mapsize,
BUS_DMASYNC_PREREAD);
return (0);
}
/*
* A frame has been uploaded: pass the resulting mbuf chain up to
* the higher level protocols.
*/
static void
sis_rxeof(sc)
struct sis_softc *sc;
{
struct mbuf *m, *m0;
struct ifnet *ifp = sc->sis_ifp;
struct sis19x_list_data *ld = &sc->sis_ldata;
struct sis19x_chain_data *cd = &sc->sis_cdata;
struct sis19x_desc *cur_rx;
u_int32_t i, rxstat, total_len = 0;
for (i = cd->sis_rx_prod; !SIS_OWNDESC(&ld->sis_rx_ring[i]);
SIS_INC(i, SIS_RX_RING_CNT)) {
bus_dmamap_sync(sc->sis_tag, cd->sis_rx_map[i], 0,
cd->sis_rx_map[i]->dm_mapsize,
BUS_DMASYNC_POSTREAD);
cur_rx = &ld->sis_rx_ring[i];
rxstat = SIS_RXSTATUS(cur_rx);
total_len = SIS_RXSIZE(cur_rx);
m = cd->sis_rx_mbuf[i];
if (m == NULL)
SIS_OUT("mbuf is NULL");
/*
* If an error occurs, update stats, clear the
* status word and leave the mbuf cluster in place:
* it should simply get re-used next time this descriptor
* comes up in the ring.
*/
if (rxstat & RX_ERR_BITS) {
SIS_OUT("ERROR_BITS=%#x", rxstat);
ifp->if_ierrors++;
/* TODO: better error differentiation */
sis_newbuf(sc, i, m);
continue;
}
/* No errors; receive the packet. */
cd->sis_rx_mbuf[i] = NULL; /* XXX neccessary? */
printf("<%u:%d>", i, total_len);
/* this was originally disabled in the freeBSD driver. Enabling it
* didn't help. First m_devget paniced. Now m_gethdr panics. */
#if 1 //def __NO_STRICT_ALIGNMENT
if (sis_newbuf(sc, i, NULL) == 0) {
m->m_pkthdr.len = m->m_len = total_len;
} else
#endif
{
/* XXX: At this point bad kernel panics happen. Trace:
* pool_do_get
* pool_get
* m_gethdr
* m_devget
*/
printf("%ud", i);
m0 = m_devget(mtod(m, char *), total_len,
ETHER_ALIGN, ifp, NULL);
printf("D");
sis_newbuf(sc, i, m);
if (m0 == NULL) {
SIS_OUT("unable to copy MBUF");
ifp->if_ierrors++;
continue;
}
m = m0;
}
ifp->if_ipackets++;
m->m_pkthdr.rcvif = ifp;
#if NBPFILTER > 0
if (ifp->if_bpf)
bpf_mtap(ifp->if_bpf, m, BPF_DIRECTION_IN);
#endif
/* pass it on. */
ether_input_mbuf(ifp, m);
}
cd->sis_rx_prod = i;
}
[demime 1.01d removed an attachment of type application/pgp-signature which had
a name of signature.asc]
