From owner-svn-src-all@FreeBSD.ORG Sun Oct 9 11:59:19 2011 Return-Path: Delivered-To: svn-src-all@FreeBSD.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 5585D106566B; Sun, 9 Oct 2011 11:59:19 +0000 (UTC) (envelope-from melifaro@FreeBSD.org) Received: from mail.ipfw.ru (unknown [IPv6:2a01:4f8:120:6141::2]) by mx1.freebsd.org (Postfix) with ESMTP id 0E8348FC0A; Sun, 9 Oct 2011 11:59:19 +0000 (UTC) Received: from secured.by.ipfw.ru ([81.200.11.182] helo=ws.su29.net) by mail.ipfw.ru with esmtpsa (TLSv1:AES256-SHA:256) (Exim 4.76 (FreeBSD)) (envelope-from ) id 1RCs2G-000GUm-ID; Sun, 09 Oct 2011 15:59:16 +0400 Message-ID: <4E918B48.5080408@FreeBSD.org> Date: Sun, 09 Oct 2011 15:53:44 +0400 From: "Alexander V. Chernikov" User-Agent: Thunderbird 2.0.0.24 (X11/20100515) MIME-Version: 1.0 To: Gleb Smirnoff References: <201109151228.p8FCSHVY073618@svn.freebsd.org> <20111009063420.GZ94905@FreeBSD.org> In-Reply-To: <20111009063420.GZ94905@FreeBSD.org> X-Enigmail-Version: 0.96.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="------------enig5083CEE90FB8C4014FC03A03" Cc: svn-src-head@FreeBSD.org, "Andrey V. Elsukov" , svn-src-all@FreeBSD.org, src-committers@FreeBSD.org Subject: Re: svn commit: r225586 - in head/sys: modules/netgraph/ipfw netgraph X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 09 Oct 2011 11:59:19 -0000 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--