linux-kernel - Re: [patches] VM-related fixes

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAMP5XgeWQpMP2Q_29aybHwfMJ1_k5GkGozqocSVVYDoRTpKD2w@mail.gmail.com>
Date:	Mon, 5 Mar 2012 19:38:21 -0800
From:	Arve Hjønnevåg <arve@...roid.com>
To:	Al Viro <viro@...iv.linux.org.uk>
Cc:	Linus Torvalds <torvalds@...ux-foundation.org>,
	linux-kernel@...r.kernel.org, airlied@...ux.ie,
	carsteno@...ibm.com, steiner@....com
Subject: Re: [patches] VM-related fixes

On Sun, Mar 4, 2012 at 10:37 PM, Al Viro <viro@...iv.linux.org.uk> wrote:
>        I'd been crawling through VMA-related code for the last couple
> of weeks; the obvious parts of fallout are in followups and IMO they
> need to go into -stable as well; there's more, including some bugs I
> don't know what to do about and patches that need more testing...
>
...
> [unsolved] binder is insane, even more than usual for drivers/staging.
> It stores a reference to mm at open() time,

It stores a reference to the task struct, not the mm.

> then it stores a reference to
> vma at mmap() time, then it cheerfully works with that ->vma assuming that
> ->mmap_sem on stored reference to ->mm would suffice.  Guess what happens
> if somebody opens it and then forks and does mmap in child?

mmap succeeds, but binder_update_page_range will fail when trying to
create a transaction? (assuming you are talking about the current
version of the driver)

>  Moreover, if
> we fork() and have child exit(), we get ->close() called on each VMA we'd
> copied into child.

The driver sets VM_DONTCOPY which should avoid this.

>  Since they have stored reference to vma invalidated by
> ->close(), that has unpleasant side effects, to put it mildly.

The side effect of ->close() should be the same as when the process
that opened the driver exits. In other words, the process that
misbehaved is considered dying. It does not however send death
notifications until the driver is closed, so other processes do not
notice until they try to talk to it.

>  And yes,
> I realize that android userland probably doesn't do anything of that kind;
> fat lot of good it does us...
>

What usage case do you have in mind? The binder driver uses the file
pointer as the binder process identifier, so you need to open and mmap
the driver in every process that want to use the driver. While it is
not common to fork a process in android, the driver should work as
expected if you do (assuming each process opens and mmaps the driver
on its own and don't share the file).

-- 
Arve Hjønnevåg
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/