linux-kernel - Re: [PATCH v3 0/6] Android Binder IPC 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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <517F829B.3080602@arm.com>
Date:	Tue, 30 Apr 2013 09:36:43 +0100
From:	Serban Constantinescu <Serban.Constantinescu@....com>
To:	Arve Hjønnevåg <arve@...roid.com>
CC:	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"gregkh@...uxfoundation.org" <gregkh@...uxfoundation.org>,
	"kernel-team@...roid.com" <kernel-team@...roid.com>,
	"john.stultz@...aro.org" <john.stultz@...aro.org>,
	Dave Butcher <Dave.Butcher@....com>
Subject: Re: [PATCH v3 0/6] Android Binder IPC Fixes

Hi Arve,

On 30/04/13 00:13, Arve Hjønnevåg wrote:
> On Mon, Apr 29, 2013 at 9:16 AM, Serban Constantinescu
> <Serban.Constantinescu@....com> wrote:
>> Hi all,
>>
>> Any feedback or comments on this patch set?
>>
>
> You don't seem to have addressed my feedback on the previous patch set.

For v3 I have modified the following according to your review:

> Changes for v3:
> 1:  Dropped the patch that was replacing uint32_t types with unsigned int
> 2:  Dropped the patch fixing the IOCTL types(since it has been added to Greg's
>     staging tree)
> 3:  Split one patch into two: 'modify binder_write_read' and '64bit changes'
> 4:  Modified BINDER_SET_MAX_THREADS ioctl definition accordint to Arve's review
> 5:  Modified the binder command IOCTL declarations according to Arve's review

The following were left out:

> On 11/04/13 22:40, Arve Hjønnevåg wrote:
> OK, relaxing the alignment requirement for *offp to what the hardware
>> requires makes sense. Is there any macros in the kernel to help with
>> this, instead of hard-coding it to 4 bytes?

There is no kernel macro that I know which will help here(one that 
springs to mind is PTR_ALIGN but it aligns to (unsigned long) - we need 
one that aligns to (u32)). Any ideas?

> On 11/04/13 21:38, Arve Hjønnevåg wrote:
> OK, but if you are using this change let a 64 bit user-space know that
>> the driver has been fixed, then this patch needs to go after the
>> patches that change the structures on 64 bit systems.

For 32bit systems nothing has changed so they will continue to work as 
before. For 64bit systems the size of binder_version was signed long 
before the patch and __s32 after the patch is applied. Thus a 64bit 
system using the old interface will fail immediately after opening the 
binder driver, while cheeking the binder version (since the 
BINDER_VERSION ioctl will be different pre/post patch - size of 
binder_version differs).

For 64/32 systems once I will have the userspace wrapper ready I will 
add another ioctl(as discussed) that will check if the driver is 64bit 
ready(among the first things to do on binder_open).

Please let me know if there is anything that skipped my review and you 
would like to integrate in this patch set.

Thanks for your feedback and help,
Serban

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