On Mon, 26 Sep 2016, Bruce Evans wrote:
On Mon, 26 Sep 2016, Hiren Panchasara wrote:
Author: hiren
Date: Mon Sep 26 10:13:58 2016
New Revision: 306337
URL: https://svnweb.freebsd.org/changeset/base/306337
Log:
In sendit(), if mp->msg_control is present, then in sockargs() we are
allocating
I didn't actually write mangling of the quotes like that.
mbuf to store mp->msg_control. Later in kern_sendit(), call to
getsock_cap(),
will check validity of file pointer passed, if this fails EBADF is
returned but
mbuf allocated in sockargs() is not freed. Fix this possible leak.
Hmm. In my reply I thought it was a cleanup after a simpler function that
was missing. The bad API makes it kern_sendit()'s responsibility to
clean up in all cases.
I don't like the layering, but the mere existence of kern_sendit() means
that you can't fix it by changing one of its callers. It exists so that
it can have multiple callers. it has 4 other callers, and 2 of these
(linux and freebsd32 compat) pass it a non-null 'control'
@@ -737,6 +737,8 @@ sendit(struct thread *td, int s, struct
bad:
free(to, M_SONAME);
+ if (control)
+ m_freem(control);
return (error);
}
The "bad" label is an old style bug. This is the general return path,
not an error handling path. This gives many collateral obfuscations
and pessimizations.
Now it seems to give a large bug with the help of the obfuscations:
when mp->msg_control != NULL, in the usual subcase where there is no
error, kern_sendit() must have already done m_freem(control) else the
usual subcase would have leaked. The code falls through to "bad"
and does m_freem(control) again.
It is possible that a double m_freem() does nothing the second time
because it sees a null chain, but it doesn't seem to support that.
[... my cleanups don't fix the problem]
Bruce
_______________________________________________
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
