Jan Klemkow <j.klem...@wemelug.de> wrote:

> On Wed, Jan 12, 2022 at 05:54:07PM +0100, Mark Kettenis wrote:
> > > Date: Wed, 12 Jan 2022 17:45:57 +0100
> > > From: Jan Klemkow <j.klem...@wemelug.de>
> > > 
> > > On Wed, Jan 12, 2022 at 05:36:01PM +0100, Mark Kettenis wrote:
> > > > > Date: Wed, 12 Jan 2022 17:02:03 +0100
> > > > > From: Jan Klemkow <j.klem...@wemelug.de>
> > > > > 
> > > > > Hi,
> > > > > 
> > > > > This diff enables TCP and UDP checksum offloading in ix(4) for IPv6.
> > > > > 
> > > > > IPv6 extension headers aren't a problem in this case.
> > > > > in6_proto_cksum_out() in netinet6/ip6_output.c disables checksum
> > > > > offloading if ip6_nxt is not TCP or UDP.  Thus, we can just use this
> > > > > field.
> > > > > 
> > > > > Tested with:
> > > > > ix0 at pci5 dev 0 function 0 "Intel 82599" rev 0x01, msix, 8 queues, 
> > > > > address 00:1b:21:94:4c:48
> > > > > 
> > > > > OK?
> > > > 
> > > > Isn't this the same disaster as the ixl(4) diff you sent earlier?  We
> > > > have sparc64 machines with onboard ix(4)...
> > > 
> > > Yes, but we don't parse the TCP header here.  As bluhm@ figured out:
> > > The access to ip_hl does not generate an alignment problem on sparc64.
> > > Because, the bits of ip_hl are on the other of the byte, as bits of
> > > th_off.
> > > 
> > > This diff just touches the IPv6 case, where we don't have this kind of
> > > problem, anyway.
> > 
> > But you're still using m_getptr(), casting the result to a struct and
> > then look at a member of the struct, which may access data beyond the
> > end of the mbuf.
> 
> We use the same pattern in re(4), vio(4) and the IPv4 case in ix(4).
> And we check for this case with the KASSERT(), except re(4).
> 
> For me, it looks as this assumption is safe.

Not sure I understand.  Looking at vio

                        mip = m_getptr(m, ehdrlen, &ipoff);
                        KASSERT(mip != NULL && mip->m_len - ipoff >= 
sizeof(*ip));

Where does the network stack (when creating mbufs) satisfy this condition --
that the struct is linear, and present in the head mbuf?

Or do you think a kernel panic at runtime is ok?

Reply via email to