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;
Date:   Thu, 16 Nov 2017 21:27:06 +0100
From:   Greg KH <gregkh@linuxfoundation.org>
To:     Todd Kjos <tkjos@android.com>
Cc:     tkjos@google.com, arve@android.com, devel@driverdev.osuosl.org,
        linux-kernel@vger.kernel.org, maco@google.com
Subject: Re: [PATCH v2] binder: fix proc->files use-after-free
Message-ID: <20171116202706.GA10790@kroah.com>
References: <20171116175650.40362-1-tkjos@google.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20171116175650.40362-1-tkjos@google.com>
User-Agent: Mutt/1.9.1 (2017-09-22)
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Thu, Nov 16, 2017 at 09:56:50AM -0800, Todd Kjos wrote:
> proc->files cleanup is initiated by binder_vma_close. Therefore
> a reference on the binder_proc is not enough to prevent the
> files_struct from being released while the binder_proc still has
> a reference. This can lead to an attempt to dereference the
> stale pointer obtained from proc->files prior to proc->files
> cleanup. This has been seen once in task_get_unused_fd_flags()
> when __alloc_fd() is called with a stale "files".
> 
> The fix is to always use get_files_struct() to obtain struct_files
> so that the refcount on the files_struct is used to prevent
> a premature free. proc->files is removed since we get it every
> time.
> 
> Signed-off-by: Todd Kjos <tkjos@google.com>
> ---
>  drivers/android/binder.c | 63 +++++++++++++++++++++++-------------------------
>  1 file changed, 30 insertions(+), 33 deletions(-)

For a v2 patch (or v3 or whatever), you need to put below the --- line
what changed from the previous version(s).
Documentation/SubmittingPatches describes this pretty well :)

thanks,

greg k-h

From 1584271463192671265@xxx Fri Nov 17 00:36:44 +0000 2017
X-GM-THRID: 1584246731040362261
X-Gmail-Labels: Inbox,Category Forums,HistoricalUnread