Hi Claudio,
On Fri, Jan 14, 2022 at 11:02:18AM +0100, Claudio Jeker wrote:
> One small comment below.
>
> > diff --git a/sys/kern/uipc_mbuf.c b/sys/kern/uipc_mbuf.c
> > index 5e4cb5ba88..21ae5059b0 100644
> > --- a/sys/kern/uipc_mbuf.c
> > +++ b/sys/kern/uipc_mbuf.c
> > [ ... ]
> > @@ -983,14 +982,18 @@ m_pullup(struct mbuf *m0, int 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;
> > + }
>
> This change is not really needed. The for (;;) loop below that does the
> copy will handle an empty initial mbuf just fine and m_free() it.
> I would not change this since I think it makes the code more complex for
> little gain.
True. Here's an updated version.
ok?
If so, any volunteers to commit this?
regards Christian
commit de01ce44f59450c9ddb0c2362bbc93eafb8cfd0a
Author: Christian Ehrhardt <[email protected]>
Date: Tue Jan 11 10:31:46 2022 +0100
m_pullup: Properly handle read-only clusters
If the first mbuf of a chain in m_pullup is a cluster, check if
the cluster is read-only (shared or an external buffer). If so
don't touch it an create an new mbuf for the pullup data.
diff --git a/sys/kern/uipc_mbuf.c b/sys/kern/uipc_mbuf.c
index 5e4cb5ba88..45bf2b2cc2 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,10 +970,11 @@ 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;
+ } 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;
@@ -983,7 +982,7 @@ m_pullup(struct mbuf *m0, int 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)
smime.p7s
Description: S/MIME cryptographic signature
