From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:48181) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Ycwp4-00073e-BD for qemu-devel@nongnu.org; Tue, 31 Mar 2015 10:07:22 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Ycwoz-0005Ja-Pu for qemu-devel@nongnu.org; Tue, 31 Mar 2015 10:07:18 -0400 Date: Tue, 31 Mar 2015 15:07:10 +0100 From: Stefan Hajnoczi Message-ID: <20150331140710.GB27156@stefanha-thinkpad.redhat.com> References: <1426298416-884-1-git-send-email-zhaoshenglong@huawei.com> <20150330132941.GK25181@stefanha-thinkpad.redhat.com> <87pp7qlgzu.fsf@linux.vnet.ibm.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="qcHopEYAB45HaUaB" Content-Disposition: inline In-Reply-To: <87pp7qlgzu.fsf@linux.vnet.ibm.com> Subject: Re: [Qemu-devel] [PATCH v2] fsdev/virtfs-proxy-helper: Fix possible overflow List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Aneesh Kumar K.V" Cc: peter.maydell@linaro.org, hangaohuai@huawei.com, qemu-trivial@nongnu.org, mjt@tls.msk.ru, qemu-devel@nongnu.org, peter.huangpeng@huawei.com, shannon.zhao@linaro.org, Shannon Zhao , pbonzini@redhat.com --qcHopEYAB45HaUaB Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Mon, Mar 30, 2015 at 07:33:33PM +0530, Aneesh Kumar K.V wrote: > Stefan Hajnoczi writes: >=20 > > On Sat, Mar 14, 2015 at 10:00:16AM +0800, Shannon Zhao wrote: > >> It's detected by coverity. As max of sockaddr_un.sun_path is > >> sizeof(helper.sun_path), should check the length of source > >> and use strncpy instead of strcpy. > >>=20 > >> Signed-off-by: Shannon Zhao > >> Signed-off-by: Shannon Zhao > >> --- > >> v1->v2: Still use strcpy [Paolo] > >> --- > >> fsdev/virtfs-proxy-helper.c | 1 + > >> 1 file changed, 1 insertion(+) > >>=20 > >> diff --git a/fsdev/virtfs-proxy-helper.c b/fsdev/virtfs-proxy-helper.c > >> index bf2e5f3..13fe032 100644 > >> --- a/fsdev/virtfs-proxy-helper.c > >> +++ b/fsdev/virtfs-proxy-helper.c > >> @@ -738,6 +738,7 @@ static int proxy_socket(const char *path, uid_t ui= d, gid_t gid) > >> return -1; > >> } > >> =20 > >> + g_assert(strlen(path) < sizeof(proxy.sun_path)); > >> sock =3D socket(AF_UNIX, SOCK_STREAM, 0); > > > > path is user input. While the assertion check silences Coverity, it is > > not suitable for input validation. Users expect a graceful exit with an > > error message, not an assertion failure if the given path is too long. > > > > I will send a patch. >=20 > That is the proxy helper. The assert will cause an exit() which is good, > isn't it ? I did update the qemu side of the patch to do a graceful exit assert(3) calls abort(3), which sends SIGABRT. The signal causes a core dump by default. That's not a proper way to exit on invalid input. Plus, if the code is ever compiled with the NDEBUG macro set, the assert would be removed and the check bypassed. This is why assert() isn't suitable for input validation. Stefan --qcHopEYAB45HaUaB Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQEcBAEBAgAGBQJVGqoOAAoJEJykq7OBq3PIjc8H/2oaIyZFCm6blOp6ovnixjtx gH//HTrjlQPxzT74HopFrVnmdqVqN2eKLFCyvCVQkipeW85jzTf057KIEapHY6Rf j8xFXG7owNMENOlbx/dgq/k+82AmEhPO1lPkVC8cXKQBTgSoDMEoqRtRsPLEbqiI 3nXcko/jUGxHDlR+BXk4BjsoHll3SXh+Tz4aqtdb/rUmWQ64q9GBapU3QuFSNnoy 2IVfI+advRNmGfKICx8GJAJQJ6b+4Js5+S5R4nOiDu8YqSNj57zJCB6WZiHgIk2y 3Q/A2powl8RZL6kgB82fAQyb3AoE7nqgBFfK0H3Y00tJQvjtzJudZ9N5eO98yrs= =S2DB -----END PGP SIGNATURE----- --qcHopEYAB45HaUaB--