From owner-freebsd-x11@FreeBSD.ORG Mon Feb 3 14:25:27 2014 Return-Path: Delivered-To: freebsd-x11@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) (using TLSv1 with cipher ADH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTPS id 0F9A66BC; Mon, 3 Feb 2014 14:25:27 +0000 (UTC) Received: from master.debian.org (master.debian.org [IPv6:2001:41b8:202:deb:216:36ff:fe40:4001]) (using TLSv1.2 with cipher DHE-RSA-AES128-SHA (128/128 bits)) (No client certificate requested) by mx1.freebsd.org (Postfix) with ESMTPS id C34C91D38; Mon, 3 Feb 2014 14:25:26 +0000 (UTC) Received: from localhost ([::1]) by master.debian.org with esmtp (Exim 4.80) (envelope-from ) id 1WAKSi-0001T7-P0; Mon, 03 Feb 2014 14:25:25 +0000 Message-ID: <52EFA6D3.3000309@freebsd.org> Date: Mon, 03 Feb 2014 14:25:23 +0000 From: Robert Millan User-Agent: Mozilla/5.0 (X11; FreeBSD amd64; rv:24.0) Gecko/20100101 Thunderbird/24.2.0 MIME-Version: 1.0 To: Baptiste Daroussin Subject: [PATCH] Fix double-free conditions in X devd backend References: <52EC4254.5040602@freebsd.org> <20140201231625.GM54904@ithaqua.etoilebsd.net> In-Reply-To: <20140201231625.GM54904@ithaqua.etoilebsd.net> X-Enigmail-Version: 1.6 Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="AFiediR9vOUCjL5oW7qqopFsbwAqWXaN9" Cc: freebsd-x11@freebsd.org X-BeenThere: freebsd-x11@freebsd.org X-Mailman-Version: 2.1.17 Precedence: list List-Id: X11 on FreeBSD -- maintaining and support List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 03 Feb 2014 14:25:27 -0000 This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --AFiediR9vOUCjL5oW7qqopFsbwAqWXaN9 Content-Type: multipart/mixed; boundary="------------010009080402080502010900" This is a multi-part message in MIME format. --------------010009080402080502010900 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable On 01/02/2014 23:16, Baptiste Daroussin wrote: > On Sat, Feb 01, 2014 at 01:39:48AM +0100, Robert Millan wrote: >> >> Hi Baptiste, >> >> Is the devd backend you wrote for X still maintained? If so, I've fixe= d a >> few problems (including a 100% reproducible heap corruption!). Shall I= send >> patches your way? >> >=20 > Yes it is please send the patches to the x11@ mailing list CC me . Okay, here's the first one which fixes three conditions that could lead t= o double-free: - xstrdup(path) before passing it to input_option_new() a second time. Th= is avoids the potential for double-free when the callee deallocates them. - Fix another double-free condition: socket_getline() is expected by its = caller to set **out as a pointer to an allocated block whenever it returns a non-negative value. Therefore do not free() buf when its strlen() is ze= ro. - The routine in wakeup_handler() ends with a "free(line)" so the `line' variable must not be tampered with. This issue is 100% reproducible and= in my system results in an X server crash each time a mouse/keyboard is= plugged/unplugged! --=20 Robert Millan --------------010009080402080502010900 Content-Type: text/x-patch; name="devd_double_free.diff" Content-Transfer-Encoding: quoted-printable Content-Disposition: attachment; filename="devd_double_free.diff" =3D=3D=3D modified file 'devd.c' --- devd.c 2014-02-03 14:05:46 +0000 +++ devd.c 2014-02-03 14:11:30 +0000 @@ -263,10 +271,10 @@ device_added(char *line) } #if XORG_VERSION_CURRENT > 10800000 attrs.usb_id =3D NULL; - options =3D input_option_new(options, "path", path); + options =3D input_option_new(options, "path", xstrdup(path)); options =3D input_option_new(options, "device", path); #else - add_option(&options, "path", path); + add_option(&options, "path", xstrdup(path)); add_option(&options, "device", path); #endif =20 @@ -390,7 +398,7 @@ socket_getline(int fd, char **out) } =20 buf[sz] =3D '\0'; - if (sz > 0) + if (sz >=3D 0) *out =3D buf; else free(buf); @@ -412,10 +420,10 @@ wakeup_handler(pointer data, int err, po =20 switch(*line) { case DEVD_EVENT_ADD: - device_added(line++); + device_added(line+1); break; case DEVD_EVENT_REMOVE: - device_removed(line++); + device_removed(line+1); break; default: break; --------------010009080402080502010900-- --AFiediR9vOUCjL5oW7qqopFsbwAqWXaN9 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.22 (FreeBSD) Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iQIcBAEBCAAGBQJS76bTAAoJELd1onhloKnOlfwQAIOSP8hBaYDtket7Cwzin+Db XeGYmnMquowf2rYeLajoutYdoTNXQedy1XBT8kuPYbiNjQWj/cB5Vcb7hROYKu1/ 6NtI7YVGqEuRGZxLqtx2R0NY0M+97e3tZrNrUms55g3EzOTHejYZWsgiwhr81NwD v3xEKVjUUnro7F42eVzYwhJRiMGSyM4hZDxFofogY/46IJORD8EonWo1FBM9Y+bn dq7dMKHIZcXqry+9Vfp4iPamoKV8090oH6kP67BCI/K1mY7RAaTmWOGvX6utNUZb Yzm0PL3rQMu92KSWazM9dNu4UlHCeFGl+0TNoqdQ+S+iQfgNXEAs1THRnZbs6ss4 33S/fXbZf/hG02fItAFB5cYxplzbrOKJeNLBoy0UrJaAQ5nI2anAN8MMmekgsRRK /L8FD1l/XsSG8bNXkDh9WUWB221KbMrDfaGfW8p4rWefFEbTAQyWZmcmRPVhT1Vp ZdqaaXlvj0Zey61Rcit/+Fs1sR3p3ndt0qgzBYBM1A9eEJfuZaooR6uWKnLYH/ob D29IQMpUwME5IvTWaXGrrO1mGPr7Ag/d2bmeR4uboRvBbuZOe+BMUWmoRKou9Eu3 xX3JC471VYyR/lATC8+RO8xUs3/bu3SkGZrXMey0o4l1moXbMZkHEbQPng8FeVf0 LwP8FfSldIuTMM+YpEMk =us4C -----END PGP SIGNATURE----- --AFiediR9vOUCjL5oW7qqopFsbwAqWXaN9--