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

["Arve Hjønnevåg" <arve@android.com>]

2012/1/31 Dan Carpenter <dan.carpenter@oracle.com>:
> On Tue, Jan 31, 2012 at 03:20:30PM -0800, Arve Hjønnevåg wrote:
>> 2012/1/31 Greg KH <greg@kroah.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