From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from thejh.net ([37.221.195.125]:46466 "EHLO thejh.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755439AbcIRSdr (ORCPT ); Sun, 18 Sep 2016 14:33:47 -0400 Date: Sun, 18 Sep 2016 20:33:42 +0200 From: Jann Horn To: Ben Hutchings Cc: Alexander Viro , Roland McGrath , Oleg Nesterov , John Johansen , James Morris , "Serge E. Hallyn" , Paul Moore , Stephen Smalley , Eric Paris , Casey Schaufler , Kees Cook , Andrew Morton , Janis Danisevskis , Seth Forshee , "Eric . Biederman" , Thomas Gleixner , Benjamin LaHaise , linux-fsdevel@vger.kernel.org, linux-security-module@vger.kernel.org, security@kernel.org Subject: Re: [PATCH 4/9] futex: don't leak robust_list pointer Message-ID: <20160918183342.GB17170@pc.thejh.net> References: <1474211117-16674-1-git-send-email-jann@thejh.net> <1474211117-16674-5-git-send-email-jann@thejh.net> <20160918182846.GR10601@decadent.org.uk> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="nVMJ2NtxeReIH9PS" Content-Disposition: inline In-Reply-To: <20160918182846.GR10601@decadent.org.uk> Sender: linux-fsdevel-owner@vger.kernel.org List-ID: --nVMJ2NtxeReIH9PS Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Sun, Sep 18, 2016 at 07:28:46PM +0100, Ben Hutchings wrote: > On Sun, Sep 18, 2016 at 05:05:12PM +0200, Jann Horn wrote: > > This prevents an attacker from determining the robust_list or > > compat_robust_list userspace pointer of a process created by executing > > a setuid binary. Such an attack could be performed by racing > > get_robust_list() with a setuid execution. The impact of this issue is = that > > an attacker could theoretically bypass ASLR when attacking setuid binar= ies. > >=20 > > Signed-off-by: Jann Horn > > --- > > kernel/futex.c | 31 +++++++++++++++++++++---------- > > kernel/futex_compat.c | 31 +++++++++++++++++++++---------- > > 2 files changed, 42 insertions(+), 20 deletions(-) > >=20 > > diff --git a/kernel/futex.c b/kernel/futex.c > > index 46cb3a3..002f056 100644 > > --- a/kernel/futex.c > > +++ b/kernel/futex.c > > @@ -3007,31 +3007,42 @@ SYSCALL_DEFINE3(get_robust_list, int, pid, > > if (!futex_cmpxchg_enabled) > > return -ENOSYS; > > =20 > > - rcu_read_lock(); > > - > > - ret =3D -ESRCH; > > - if (!pid) > > + if (!pid) { > > p =3D current; > > - else { > > + get_task_struct(p); > > + } else { > > + rcu_read_lock(); > > p =3D find_task_by_vpid(pid); > > - if (!p) > > - goto err_unlock; > > + /* pin the task to permit dropping the RCU read lock before > > + * acquiring the mutex > > + */ > > + get_task_struct(p); >=20 > get_task_struct() requires a non-null pointer so you can't move the > null check below it. Oh, right, thanks. Will fix that in v2. --nVMJ2NtxeReIH9PS Content-Type: application/pgp-signature; name="signature.asc" Content-Description: Digital signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQIcBAEBAgAGBQJX3t4GAAoJED4KNFJOeCOouNMQANccDxLQCF3MKH9Cxg+JPmq4 LzOTbBNiq4wTcuK5HDhzYAeCuyixqyjPBa/ZJYPiz1rOoAPqRc+HiY79Y/xaw0rd 4p34vO59g34ZLbo+h7QXGpGj8Py6HvAi04/3s+LKWgLl8mNmJ2mwBTEozNZwK5wU 8/jUmpoEYwDYx8CEucH9ygZMWc3CX2j5crvbA05sJ3gtAcub9S1dRh5QJo66nGFl ezGGQzyZxhyII7NAqFM7h+zN6VmnfJ82j86FEXLLtiG8AyrQycQ9oJ580O9+4kbK UcjIKY3Oap4QOWyNUHPMX/vzh5I6QPRdnIl3hGPS9eJyvUOtc038+7Cf8lykZmRC E9K7yPmP8VxQRzOquqoP3EwE+TC1qbRlQ2IhnVsLjkmjKtCwTucnNsmkSYloPwpS AuSHpvkts9ILrqP8rzjNCziJ+L/5HHtjaPxlcgAr05J5zqj5kJ1+p6lcQHDcD7er GLx6HA/6SiRpp6c4nTdg5kZJ7I1WLtxqPuHiL7UpulbnTZkGesHfE3ckpySPnCIq ED+/MlpIbTPGiMrabxFD/byTsH7kJ4SzViHqJsiRpNIy2+qYYq/U5vTaA+UoZ9Ao rgrJZhDGQHayGYLdLoOkFsIYK9D6myZ+j+8kixXseEaWmBOxabSrkdZOoRVHr9OW shD3Fug1VO+xfpu98MaP =nASN -----END PGP SIGNATURE----- --nVMJ2NtxeReIH9PS--