Received: by 2002:a5b:505:0:0:0:0:0 with SMTP id o5csp3376124ybp; Sun, 6 Oct 2019 10:34:07 -0700 (PDT) X-Google-Smtp-Source: APXvYqyhMHhZUY03lNq9Wc3roOaK5qd5qz4BZCb7veOSKqHy1LnyZ6BSryFDBWI7f30GyPZz0l+a X-Received: by 2002:a17:906:cc9b:: with SMTP id oq27mr20514257ejb.125.1570383247594; Sun, 06 Oct 2019 10:34:07 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1570383247; cv=none; d=google.com; s=arc-20160816; b=rvb5PWnvX0ReBgq4++eC31sUYBlEndCXnxPdAqgt78TQ/HHggyAfjC0FFAaeLQgHty WeRNbmknGwMMK8agljK4qU8chTzJSRVF4J8eI1t0nTvmbEVRr/UK0vF2SIQ1df0BbGhF 64fVUAajC0KFE7IceFesAJm0seQ4HG/0B0HfeW91keHjVK4EjRuCU2tR6CD8R79n46CA +AxwlV2QnbhhCh4SQe0wvlX9ZzK89nxFFffms5wELrsIhbpIuJKR97CYnsJkVGIoIcyo PQlkKG7/DEi+uvCouTen03urlqBB7V2wwis2WoUSLW2rGEzV2SUMMqFc+MYPNM4TIgmW RU+w== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:sender:user-agent:in-reply-to :content-disposition:mime-version:references:mail-followup-to :message-id:subject:cc:to:from:date:dkim-signature; bh=MCazKCPTQaMLlYrh/DqBjJKc1W4sX5KK1Ozgp3S+iJU=; b=H8ac7yQ1sNUO13+AX8yTZnHfzQrS3kUtc7vTazN5huF2i2eAQ6HekThQzh23+Rfjuu ULH5ivgmJzb4bXS/EOgGrnW5R5ihpNLkuHicBWRphaM81LEl0J6nOG4Yf/dTJWHoWUeK z77TRw0EDVN4LOWGYINwLE4M2AmCxJNO7qN1R2WpBEwIRv4p4oJRm8DgXw+Bd5rlnZZ8 kjV/ZcTTxWdNuIHqjnc+xyGO2piFvM6RCgdVpr9/r9DhGcnlQOiBMMbJenupmk2c1QIm +7As+GzEOG7iJEclfq9gFYasmMRaaLgbLgEmfKmRRmB62iJSkvHHcjDinmbrsyMye/YS s5Iw== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@kernel.org header.s=default header.b=121vaGNX; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=kernel.org Return-Path: Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67]) by mx.google.com with ESMTP id g24si4695469ejd.401.2019.10.06.10.33.44; Sun, 06 Oct 2019 10:34:07 -0700 (PDT) Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67; Authentication-Results: mx.google.com; dkim=pass header.i=@kernel.org header.s=default header.b=121vaGNX; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1728672AbfJFRcG (ORCPT + 99 others); Sun, 6 Oct 2019 13:32:06 -0400 Received: from mail.kernel.org ([198.145.29.99]:58468 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1729060AbfJFRcF (ORCPT ); Sun, 6 Oct 2019 13:32:05 -0400 Received: from sol.localdomain (c-24-5-143-220.hsd1.ca.comcast.net [24.5.143.220]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPSA id F13E52133F; Sun, 6 Oct 2019 17:32:03 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1570383124; bh=y24o64+df+bFr+XImQr+ZOI4HN+DUwtAyK/fL7vJzEc=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=121vaGNXc5JT+w6LQYymQyiF+FborDuGpo6Ui2V0O43iWVlulxml28rUZ13kv/E4a DoNY6rdxiTSf83cnUFu8sVhH6pPl62muoxkHCqy63gRp2TFT6A2hy4tpfwJhFuJOs5 8TJVRrxgdx+3GePIn8QezPRMNO+sIq8nuisUinns= Date: Sun, 6 Oct 2019 10:32:02 -0700 From: Eric Biggers To: Greg Kroah-Hartman Cc: linux-kernel@vger.kernel.org, stable@vger.kernel.org, Martijn Coenen , syzbot , Mattias Nissler Subject: Re: [PATCH 4.9 30/47] ANDROID: binder: remove waitqueue when thread exits. Message-ID: <20191006173202.GA832@sol.localdomain> Mail-Followup-To: Greg Kroah-Hartman , linux-kernel@vger.kernel.org, stable@vger.kernel.org, Martijn Coenen , syzbot , Mattias Nissler References: <20191006172016.873463083@linuxfoundation.org> <20191006172018.480360174@linuxfoundation.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20191006172018.480360174@linuxfoundation.org> User-Agent: Mutt/1.12.2 (2019-09-21) Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Sun, Oct 06, 2019 at 07:21:17PM +0200, Greg Kroah-Hartman wrote: > From: Martijn Coenen > > commit f5cb779ba16334b45ba8946d6bfa6d9834d1527f upstream. > > binder_poll() passes the thread->wait waitqueue that > can be slept on for work. When a thread that uses > epoll explicitly exits using BINDER_THREAD_EXIT, > the waitqueue is freed, but it is never removed > from the corresponding epoll data structure. When > the process subsequently exits, the epoll cleanup > code tries to access the waitlist, which results in > a use-after-free. > > Prevent this by using POLLFREE when the thread exits. > > Signed-off-by: Martijn Coenen > Reported-by: syzbot > Cc: stable # 4.14 > [backport BINDER_LOOPER_STATE_POLL logic as well] > Signed-off-by: Mattias Nissler > Signed-off-by: Greg Kroah-Hartman > --- > drivers/android/binder.c | 17 ++++++++++++++++- > 1 file changed, 16 insertions(+), 1 deletion(-) > > --- a/drivers/android/binder.c > +++ b/drivers/android/binder.c > @@ -334,7 +334,8 @@ enum { > BINDER_LOOPER_STATE_EXITED = 0x04, > BINDER_LOOPER_STATE_INVALID = 0x08, > BINDER_LOOPER_STATE_WAITING = 0x10, > - BINDER_LOOPER_STATE_NEED_RETURN = 0x20 > + BINDER_LOOPER_STATE_NEED_RETURN = 0x20, > + BINDER_LOOPER_STATE_POLL = 0x40, > }; > > struct binder_thread { > @@ -2628,6 +2629,18 @@ static int binder_free_thread(struct bin > } else > BUG(); > } > + > + /* > + * If this thread used poll, make sure we remove the waitqueue > + * from any epoll data structures holding it with POLLFREE. > + * waitqueue_active() is safe to use here because we're holding > + * the inner lock. > + */ > + if ((thread->looper & BINDER_LOOPER_STATE_POLL) && > + waitqueue_active(&thread->wait)) { > + wake_up_poll(&thread->wait, POLLHUP | POLLFREE); > + } > + > if (send_reply) > binder_send_failed_reply(send_reply, BR_DEAD_REPLY); > binder_release_work(&thread->todo); > @@ -2651,6 +2664,8 @@ static unsigned int binder_poll(struct f > return POLLERR; > } > > + thread->looper |= BINDER_LOOPER_STATE_POLL; > + > wait_for_proc_work = thread->transaction_stack == NULL && > list_empty(&thread->todo) && thread->return_error == BR_OK; > Are you sure this backport is correct, given that in 4.9, binder_poll() sometimes uses proc->wait instead of thread->wait?: wait_for_proc_work = thread->transaction_stack == NULL && list_empty(&thread->todo) && thread->return_error == BR_OK; binder_unlock(__func__); if (wait_for_proc_work) { if (binder_has_proc_work(proc, thread)) return POLLIN; poll_wait(filp, &proc->wait, wait); if (binder_has_proc_work(proc, thread)) return POLLIN; } else { if (binder_has_thread_work(thread)) return POLLIN; poll_wait(filp, &thread->wait, wait); if (binder_has_thread_work(thread)) return POLLIN; } return 0;