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?

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.

> +             } 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)


Reply via email to