Index | Thread | Search

From:
Claudio Jeker <cjeker@diehard.n-r-g.com>
Subject:
Re: mbuf pulldown replace memmove with memcpy
To:
Alexander Bluhm <bluhm@openbsd.org>
Cc:
tech@openbsd.org
Date:
Thu, 29 Aug 2024 16:54:19 +0200

Download raw body.

Thread
On Thu, Aug 29, 2024 at 04:21:01PM +0200, Alexander Bluhm wrote:
> Hi,
> 
> m_pulldown() copies memory between different mbufs.  So they cannot
> overlap and memcpy() should be enough.
> 
> ok?

Indeed. The first memcpy is save because n is not a read-only buffer and
so n and n->m_next have do be different mbufs or clusters.
The 2nd memcpy goes to freshly allocated buffer o. So that is safe too.

OK claudio@
 
> bluhm
> 
> Index: kern/uipc_mbuf2.c
> ===================================================================
> RCS file: /data/mirror/openbsd/cvs/src/sys/kern/uipc_mbuf2.c,v
> diff -u -p -r1.46 uipc_mbuf2.c
> --- kern/uipc_mbuf2.c	29 Aug 2024 10:44:40 -0000	1.46
> +++ kern/uipc_mbuf2.c	29 Aug 2024 14:04:56 -0000
> @@ -171,7 +171,7 @@ m_pulldown(struct mbuf *m, int off, int 
>  		n->m_next->m_data -= hlen;
>  		n->m_next->m_len += hlen;
>  		counters_inc(mbstat, MBSTAT_PULLDOWN_COPY);
> -		memmove(mtod(n->m_next, caddr_t), mtod(n, caddr_t) + off, hlen);
> +		memcpy(mtod(n->m_next, caddr_t), mtod(n, caddr_t) + off, hlen);
>  		n->m_len -= hlen;
>  		n = n->m_next;
>  		off = 0;
> @@ -201,7 +201,7 @@ m_pulldown(struct mbuf *m, int off, int 
>  	}
>  	/* get hlen from <n, off> into <o, 0> */
>  	o->m_len = hlen;
> -	memmove(mtod(o, caddr_t), mtod(n, caddr_t) + off, hlen);
> +	memcpy(mtod(o, caddr_t), mtod(n, caddr_t) + off, hlen);
>  	n->m_len -= hlen;
>  	/* get tlen from <n->m_next, 0> into <o, hlen> */
>  	m_copydata(n->m_next, 0, tlen, mtod(o, caddr_t) + o->m_len);
> 

-- 
:wq Claudio