Hi,
Coverty complains that the while loop at the end of m_adj could
dereference m if it is NULL. The for loop before checks that m is
not NULL, so coverty thinks it may be NULL. See CID 501458.
In fact the for (;m;) check is not necessary. We have calculated
the length of the mbuf chain before and stored it in count. The
only way we could leave the final for loop is by the m_len check
and the break.
So I have changed:
- Remove the for (;m;) check from the final for loop, it is not
necessary.
- Move the m = mp close to all the loops where we traverse the mbuf
chain.
- Use mp to access the m_pkthdr consistently.
- Move the next assignemnt from for (;;m = m->m_next) to the
end of the loop to make it consistent to the previous for (;;)
where we calculate the total lenght.
ok?
bluhm
Index: kern/uipc_mbuf.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/kern/uipc_mbuf.c,v
retrieving revision 1.248
diff -u -p -r1.248 uipc_mbuf.c
--- kern/uipc_mbuf.c 27 May 2017 16:41:10 -0000 1.248
+++ kern/uipc_mbuf.c 7 Sep 2017 20:02:01 -0000
@@ -804,12 +804,13 @@ m_adj(struct mbuf *mp, int req_len)
struct mbuf *m;
int count;
- if ((m = mp) == NULL)
+ if (mp == NULL)
return;
if (len >= 0) {
/*
* Trim from head.
*/
+ m = mp;
while (m != NULL && len > 0) {
if (m->m_len <= len) {
len -= m->m_len;
@@ -833,6 +834,7 @@ m_adj(struct mbuf *mp, int req_len)
*/
len = -len;
count = 0;
+ m = mp;
for (;;) {
count += m->m_len;
if (m->m_next == NULL)
@@ -853,15 +855,16 @@ m_adj(struct mbuf *mp, int req_len)
* Find the mbuf with last data, adjust its length,
* and toss data from remaining mbufs on chain.
*/
+ if (mp->m_flags & M_PKTHDR)
+ mp->m_pkthdr.len = count;
m = mp;
- if (m->m_flags & M_PKTHDR)
- m->m_pkthdr.len = count;
- for (; m; m = m->m_next) {
+ for (;;) {
if (m->m_len >= count) {
m->m_len = count;
break;
}
count -= m->m_len;
+ m = m->m_next;
}
while ((m = m->m_next) != NULL)
m->m_len = 0;
