Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1757204AbcJ3Qrp (ORCPT ); Sun, 30 Oct 2016 12:47:45 -0400 Received: from thejh.net ([37.221.195.125]:54027 "EHLO thejh.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1757135AbcJ3Qrk (ORCPT ); Sun, 30 Oct 2016 12:47:40 -0400 Date: Sun, 30 Oct 2016 17:47:35 +0100 From: Jann Horn To: Andy Lutomirski Cc: Arnd Bergmann , Greg Kroah-Hartman , Sudip Mukherjee , "linux-kernel@vger.kernel.org" , linux-parport@lists.infradead.org Subject: Re: [PATCH] ppdev: fix double-free of pp->pdev->name Message-ID: <20161030164735.GA2558@pc.thejh.net> References: <1477842248-2234-1-git-send-email-jann@thejh.net> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="M9NhX3UHpAaciwkO" Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.23 (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 3461 Lines: 84 --M9NhX3UHpAaciwkO Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Sun, Oct 30, 2016 at 09:29:10AM -0700, Andy Lutomirski wrote: > On Sun, Oct 30, 2016 at 8:44 AM, Jann Horn wrote: > > free_pardevice() is called by parport_unregister_device() and already f= rees > > pp->pdev->name, don't try to do it again. > > > > This bug causes kernel crashes. > > > > I found and verified this with KASAN and some added pr_emerg()s: > > > > [ 60.316568] pp_release: pp->pdev->name =3D=3D ffff88039cb264c0 > > [ 60.316692] free_pardevice: freeing par_dev->name at ffff88039cb264c0 > > [ 60.316706] pp_release: kfree(ffff88039cb264c0) > > [ 60.316714] =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 > > [ 60.316722] BUG: Double free or freeing an invalid pointer > > [ 60.316731] Unexpected shadow byte: 0xFB > > [ 60.316801] Object at ffff88039cb264c0, in cache kmalloc-32 size: 32 > > [ 60.316813] Allocated: > > [ 60.316824] PID =3D 1695 > > [ 60.316869] Freed: > > [ 60.316880] PID =3D 1695 > > [ 60.316935] =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 > > > > CCing Andy Lutomirski because I think this is what broke vmapped stacks > > for me - after applying this patch, vmapped stacks worked for me. > > Previously, I got oopses (and lockups) caused by area->pages[0] being > > 0x400000000 in __vunmap(), with area->pages being allocated in the kmal= loc > > area. >=20 > That's an odd symptom. I assume that what's happening is that the > pages array is being freed early by the extra kfree in here and then > getting corrupted. Well, as far as I can tell, there are two ways to reach that. Obvious first way, but a pretty tight race: Task A: free(name) Task B: allocate area->pages in same place Task A: second free(name), releasing area->pages Second way (if the SLAB allocator, which I'm using, is used): Task A: free(name), appends the object to ac->entry in ___cache_free() Task A: second free(name), appends the object to ac->entry again Task B: ____cache_alloc() returns object from array cache Task C: ____cache_alloc() returns same object again So then the same memory would be used by two separate objects? --M9NhX3UHpAaciwkO Content-Type: application/pgp-signature; name="signature.asc" Content-Description: Digital signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQIcBAEBAgAGBQJYFiQnAAoJED4KNFJOeCOowpYQAIwZovNzwJ4kKUq0tYcWcLhz Ksz5qqJIT0VWLqzXtOMOLaJi+CB2AYrHXf8W7aIww1DHN5fzVZJsOVDSSAmG1lI5 VOo2IyIKRQNiWv9fHgvBBwvn6w8YBRS2FYuphPtQjFPWwzdoDpngurnU8tpF5S6l BgpsB8eVxBEdd+YbjPB8jrJPrzAKJPTf7u3XQyVR/eUkPZCCwGyhw4ritM0k8hHx uyNPxCa9f1Jc7maSI2nwU6ljUHewR0HQ+8CKMNpCdB9gQBjwhBzxAxlF8zcnWqnF l/oAzoIsMpaOCRw9QPV68F7SmbzZpFYb1K2SDGn/2Wjqjd1snTQytwEkmgJ3Zw7T LsTI2BhK61rkDutipMVICK/GIemyjG80V3Q3MS3bo5rzjNcnY7z3hKhaqMK4Vpy5 yP2EHlyK+t55jsr7rbPAG8oPh4WhEpT4bxaf690DgN/x1w5CDdHUZFQIqNk7SD+v KJipB9MAGkQdvWEoxMHHKZ/ZzUGbiAd/faIJGyCZDTxOYIdAJ0i98Tq4HFfr0Rw4 cgl8MWlGVe6g+MXybVcuuToS75VkgoG9vPYT+cbHQmCA9GRVmiiWE220xL6lvMRd lLmRNAbNuaP6Vs31cOK5MASbnjJDR05nvjyq4Oo9xSYka8bwtpFqspgXAnJxuJ69 xd1JKdMMlYpEJVXdbo1b =VS+d -----END PGP SIGNATURE----- --M9NhX3UHpAaciwkO--