Hi,
On Wed, Jan 12, 2022 at 02:57:28PM +0100, Stefan Sperling wrote:
> On Wed, Jan 12, 2022 at 10:50:50AM +0100, Christian Ehrhardt wrote:
> > diff --git a/sys/kern/uipc_mbuf.c b/sys/kern/uipc_mbuf.c
> > index 5e4cb5ba88..d8b9e751c6 100644
> > --- a/sys/kern/uipc_mbuf.c
> > +++ b/sys/kern/uipc_mbuf.c
> > @@ -957,8 +957,6 @@ m_pullup(struct mbuf *m0, int len)
> >
> > head = M_DATABUF(m0);
> > if (m0->m_len == 0) {
> > - m0->m_data = head;
> > -
> > while (m->m_len == 0) {
> > m = m_free(m);
> > if (m == NULL)
> > @@ -972,25 +970,29 @@ m_pullup(struct mbuf *m0, int len)
> > tail = head + M_SIZE(m0);
> > head += adj;
> >
> > - if (len <= tail - head) {
> > - /* there's enough space in the first mbuf */
> > -
> > - if (len > tail - mtod(m0, caddr_t)) {
> > + if (!M_READONLY(m0) && len <= tail - head) {
> > + /* we can copy everything into the first mbuf */
> > + if (m0->m_len == 0) {
> > + m0->m_data = head;
>
> Before your patch, m_data was effectively set to head - adj in this case,
> because head += adj had not happened yet. Does this fix a bug in the old
> code, or does it introduce a bug?
Oh, you are right. If the first mbuf is empty but large enough
to hold the pulled data, we previously did not honour the alignment.
I admit that I didn't notice this semantic change but I still think
that the new version of the code is what is supposed to happen.
> The rest makes sense to me, thanks for putting so much effort into getting
> this fixed!
>
> OK by me, but in my opinion an mbuf expert (bluhm? claudio? someone else?)
> needs to take a close look at this before it goes in.
True. Thanks for looking at it.
> > + } else if (len > tail - mtod(m0, caddr_t)) {
> > /* need to memmove to make space at the end */
> > memmove(head, mtod(m0, caddr_t), m0->m_len);
> > m0->m_data = head;
> > + len -= m0->m_len;
> > }
> > -
> > - len -= m0->m_len;
> > } else {
> > - /* the first mbuf is too small so make a new one */
> > + /* the first mbuf is too small or read-only, make a new one */
> > space = adj + len;
> >
> > if (space > MAXMCLBYTES)
> > goto bad;
> >
> > - m0->m_next = m;
> > - m = m0;
> > + if (m0->m_len == 0) {
> > + m_free(m0);
> > + } else {
> > + m0->m_next = m;
> > + m = m0;
> > + }
> >
> > MGET(m0, M_DONTWAIT, m->m_type);
> > if (m0 == NULL)
regards Christian
--
genua GmbH
Domagkstrasse 7, 85551 Kirchheim, Germany
phone +49 89 991950-195, fax -999, www.genua.eu
genua is a member of the Bundesdruckerei Group.
smime.p7s
Description: S/MIME cryptographic signature
