Date: Sun, 09 Oct 2011 15:53:44 +0400 From: "Alexander V. Chernikov" <melifaro@FreeBSD.org> To: Gleb Smirnoff <glebius@FreeBSD.org> Cc: svn-src-head@FreeBSD.org, "Andrey V. Elsukov" <ae@FreeBSD.org>, svn-src-all@FreeBSD.org, src-committers@FreeBSD.org Subject: Re: svn commit: r225586 - in head/sys: modules/netgraph/ipfw netgraph Message-ID: <4E918B48.5080408@FreeBSD.org> In-Reply-To: <20111009063420.GZ94905@FreeBSD.org> References: <201109151228.p8FCSHVY073618@svn.freebsd.org> <20111009063420.GZ94905@FreeBSD.org>
next in thread | previous in thread | raw e-mail | index | archive | help
This is an OpenPGP/MIME signed message (RFC 2440 and 3156) --------------enig5083CEE90FB8C4014FC03A03 Content-Type: multipart/mixed; boundary="------------080608030806000102020101" This is a multi-part message in MIME format. --------------080608030806000102020101 Content-Type: text/plain; charset=KOI8-R Content-Transfer-Encoding: quoted-printable Gleb Smirnoff wrote: > Alexander, Andrey, >=20 > see a couple of comments below please. =2E.. > A> + if (m->m_len < sizeof(struct ip) && > A> + (m =3D m_pullup(m, sizeof(struct ip))) =3D=3D NULL) > A> + return (EINVAL); >=20 > In most cases we return ENOBUFS in case if m_pullup() failure. Lesser a= mount > of code uses ENOMEM and EINVAL. I personally hate EINVAL, since it is u= sually > used as one-for-all error, and thus is meaningless for user. Understood. So can we use more descriptive ENOENT in code below? tag =3D m_tag_locate(m, MTAG_IPFW_RULE, 0, NULL); if (tag =3D=3D NULL) { NG_FREE_M(m); return (EINVAL); /* XXX: find smth better */ }; >=20 > A> + ip =3D mtod(m, struct ip *); > A> + =2E... > A> + default: > A> return (EINVAL); >=20 > Shouldn't you free the mbuf before returning? Ups. Please see an attached patch --------------080608030806000102020101 Content-Type: text/plain; name="ng_ipfw.diff" Content-Transfer-Encoding: quoted-printable Content-Disposition: inline; filename="ng_ipfw.diff" Index: sys/netgraph/ng_ipfw.c =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D --- sys/netgraph/ng_ipfw.c (revision 226165) +++ sys/netgraph/ng_ipfw.c (working copy) @@ -242,7 +242,7 @@ ng_ipfw_rcvdata(hook_p hook, item_p item) =20 if (m->m_len < sizeof(struct ip) && (m =3D m_pullup(m, sizeof(struct ip))) =3D=3D NULL) - return (EINVAL); + return (ENOBUFS); =20 ip =3D mtod(m, struct ip *); =20 @@ -252,18 +252,14 @@ ng_ipfw_rcvdata(hook_p hook, item_p item) #ifdef INET case IPVERSION: ip_input(m); - break; + return (0); #endif #ifdef INET6 case IPV6_VERSION >> 4: ip6_input(m); - break; + return (0); #endif - default: - NG_FREE_M(m); - return (EINVAL); } - return (0); } else { switch (ip->ip_v) { #ifdef INET @@ -277,10 +273,12 @@ ng_ipfw_rcvdata(hook_p hook, item_p item) return (ip6_output(m, NULL, NULL, 0, NULL, NULL, NULL)); #endif - default: - return (EINVAL); } } + + /* unknown IP protocol version */ + NG_FREE_M(m); + return (EPROTONOSUPPORT); } =20 static int --------------080608030806000102020101-- --------------enig5083CEE90FB8C4014FC03A03 Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2.0.14 (FreeBSD) Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/ iEYEARECAAYFAk6Ri0sACgkQwcJ4iSZ1q2nWGgCfX/EJZpmQCskKQxphoCpQ3TMn 2U4An1Tmbj4lAvEhY77Vp1LULJpRKfOo =8LMM -----END PGP SIGNATURE----- --------------enig5083CEE90FB8C4014FC03A03--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?4E918B48.5080408>