On Tue, Feb 06, 2018 at 11:04:51AM +1300, Richard Procter wrote:
> > @@ -657,12 +667,13 @@ ah_input(struct mbuf *m, struct tdb *tdb
> > m_copyback(m, skip + rplen, ahx->authsize, ipseczeroes, M_NOWAIT);
> >
> > /* "Massage" the packet headers for crypto processing. */
> > - if ((btsx = ah_massage_headers(&m, tdb->tdb_dst.sa.sa_family,
> > - skip, ahx->type, 0)) != 0) {
> > + error = ah_massage_headers(&m, tdb->tdb_dst.sa.sa_family, skip,
> > + ahx->type, 0);
> > + if (error) {
> > /* mbuf will be free'd by callee. */
>
> This pre-existing comment muddled me. ah_massage_headers() has already
> freed it on error.
Although the code is correct, I also had to read the comment twice
when I first saw it. I think the tense is wrong. The callee is
ah_massage_headers, and there free has been called.
Does this clarify it?
bluhm
Index: netinet/ip_ah.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/ip_ah.c,v
retrieving revision 1.135
diff -u -p -r1.135 ip_ah.c
--- netinet/ip_ah.c 6 Feb 2018 14:54:22 -0000 1.135
+++ netinet/ip_ah.c 6 Feb 2018 15:14:29 -0000
@@ -670,7 +670,7 @@ ah_input(struct mbuf *m, struct tdb *tdb
error = ah_massage_headers(&m, tdb->tdb_dst.sa.sa_family, skip,
ahx->type, 0);
if (error) {
- /* mbuf will be free'd by callee. */
+ /* mbuf has been be free'd by callee. */
free(tc, M_XDATA, 0);
crypto_freereq(crp);
return error;
@@ -1158,7 +1158,7 @@ ah_output(struct mbuf *m, struct tdb *td
error = ah_massage_headers(&m, tdb->tdb_dst.sa.sa_family, skip,
ahx->type, 1);
if (error) {
- /* mbuf will be free'd by callee. */
+ /* mbuf has been be free'd by callee. */
free(tc, M_XDATA, 0);
crypto_freereq(crp);
return error;
