Seems sensible. I've asked Rick Macklem to verify.

If you look at krpc_call(), it does an m_freem(mhead) for most error cases.
For some of these, mhead->m_next = m, so this patch is not safe and
can result in multiple frees, for an error return after this point in the function.

I think krpc_call() can be fixed by adding this one line after #279:
279: mhead->m_next = *data;
add-> *data = NULL;

With this one line change, I think the above patch is safe.

However, I think it might be preferable to have all the m_freem() calls in krpc_call() instead for the error return
cases that preceed "mhead->m_next = *data, since it already frees the mbuf chain passed in for subsequent
cases? (Basically the first 4 error return cases, 2 which do return and two which do "goto out".)

rick

Sorry for the delay, this had dropped off my radar until sbruno prodded me about it. Thanks for taking the time to review, looking over the goto outs in krpc_call() reveals the possible double-frees as you've mentioned.

I believe this represents what you were speaking to, simply freeing before the return on the first 2 error cases.

Sorry, but this patch would be worse than a leak. "mhead" is not assigned at
this point in the code (unless there is some other patch already assumed).
mhead is set NULL at line#216, after the first m_freem(mhead) this patch adds.
The code must m_freem(*data); for these cases.

Also, there are three places where there is a "goto out" before mhead->m_next
is set to "*data". These cases (line#s 235, 246 and 268) also need a "m_freem(*data)"
in front of them.

Btw, I think this code is only executed during mounting of an NFS root fs, so a
leak is probably harmless.

rick