linux-kernel - Re: [PATCH 1/2] Staging: android: binder: Add some error checks

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]

Date:	Wed, 1 Feb 2012 14:29:36 -0800
From:	Arve Hjønnevåg <arve@...roid.com>
To:	Dan Carpenter <dan.carpenter@...cle.com>
Cc:	Greg KH <greg@...ah.com>, devel@...verdev.osuosl.org,
	Christopher Lais <chris+android@...thought.org>,
	Greg Kroah-Hartman <gregkh@...e.de>,
	linux-kernel@...r.kernel.org, Al Viro <viro@...iv.linux.org.uk>
Subject: Re: [PATCH 1/2] Staging: android: binder: Add some error checks

2012/1/31 Dan Carpenter <dan.carpenter@...cle.com>:
> On Tue, Jan 31, 2012 at 03:20:30PM -0800, Arve Hjønnevåg wrote:
>> 2012/1/31 Greg KH <greg@...ah.com>:
>> > On Sat, Jan 21, 2012 at 11:22:08AM +0300, Dan Carpenter wrote:
>> >> On Fri, Jan 20, 2012 at 07:56:20PM -0800, Arve Hjønnevåg wrote:
>> >> > - Add a mutex to protect against two processes mmapping the
>> >> >   same binder_proc.
>> >> > - After locking mmap_sem, check that the vma we want to access
>> >> >   (still) points to the same mm_struct.
>> >> > - Use proc->tsk instead of current to get the files struct since
>> >> >   this is where we get the rlimit from.
>> >>
>> >> This doesn't seem related to the locking change at all.  Probably
>> >> this patch should be split into three patches, one bugfix per
>> >> patch, unless they are very closely related.
>> >
>> > I agree.  Arve, is this all fixing one problem, or multiple ones?  If
>> > multiple ones, we need this split up into multiple patches.
>> >
>>
>> That depend on your point of view. It fixes crashes if you use the
>> same binder file pointer from multiple processes. It seemed excessive
>> to have three patches for this.
>
> It would have helped you to write a better changelog.  The subject
> says "[patch] android: grab bag of random fixes" and the the
> description matches that.  You have no idea how annoyed I get at
> grab bag patches.
>

Would the following be a better change description (or do you still
want three patches):

Staging: android: binder: Fix crashes when sharing a binder file
between processes

Opening the binder driver and sharing the file returned with
other processes (e.g. by calling fork) can crash the kernel.
Prevent these crashes with the following changes:
- Add a mutex to protect against two processes mmapping the
  same binder_proc.
- After locking mmap_sem, check that the vma we want to access
  (still) points to the same mm_struct.
- Use proc->tsk instead of current to get the files struct since
  this is where we get the rlimit from.



-- 
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/