lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20171117193146.GO21978@ZenIV.linux.org.uk>
Date:   Fri, 17 Nov 2017 19:31:46 +0000
From:   Al Viro <viro@...IV.linux.org.uk>
To:     Todd Kjos <tkjos@...roid.com>
Cc:     tkjos@...gle.com, gregkh@...uxfoundation.org, arve@...roid.com,
        devel@...verdev.osuosl.org, linux-kernel@...r.kernel.org,
        maco@...gle.com
Subject: Re: [PATCH v2] binder: fix proc->files use-after-free

On Thu, Nov 16, 2017 at 09:56:50AM -0800, Todd Kjos wrote:

> +static struct files_struct *binder_get_files_struct(struct binder_proc *proc)
> +{
> +	return get_files_struct(proc->tsk);
> +}

Hell, _no_.  You should never, ever use the result of get_files_struct() for
write access.  It's strictly for "let me look at other process' descriptor
table".  The whole reason for proc->files is that we want to keep a reference
that *could* be used for modifying the damn thing.  And such can be obtained
only by the process itself.

The rules are:
	* you can use current->files both for read and write
	* you can use get_files_struct(current) to get a reference that
can be used for modifying the damn thing.  Then it can be passed to
any other process and used by it.
	* you can use get_files_struct(some_other_task) to get a reference
that can be used for examining the descriptor table of that other task.

Violation of those rules means an exploitable race.  Here's the logics
fdget() and friends are based on: "I'm going to do something to file
refered by descriptor N.  If my descriptor table is not shared, all
struct file references in it will stay around - I'm not changing it,
nobody else has it as their ->current, so any additional references
to that descriptor table will *not* be used for modifying it.
In other words, I don't need to bump the refcount of struct file I'm
about to work with - the reference from my descriptor table will keep
it alive".

Your patch breaks those assumptions.  NAK.

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ