* [PATCH] ion_test: Add compat_ioctl support
@ 2013-12-19 22:40 John Stultz
2013-12-19 22:52 ` Colin Cross
0 siblings, 1 reply; 6+ messages in thread
From: John Stultz @ 2013-12-19 22:40 UTC (permalink / raw)
To: LKML; +Cc: John Stultz, Colin Cross, Greg KH, Android Kernel Team
Prior to subitting this, Colin reworked the compat_ioctl support
for the ion_test driver, moving the structure to be the same size
on both 32 and 64 bit architectures.
Two small things were left out. The compat_ioctl ptr assignment,
and the fact that despite having uniform sized types in the
structure, the structure pads out to different sizes on different
arches.
This patch resolves this issue by setting the write flag as
a __u64, and adding the compat_ioctl ptr.
While this does affect the ABI for 32bit users, its only
the ABI for the ion_test driver, not ion itself.
Cc: Colin Cross <ccross@android.com>
Cc: Greg KH <gregkh@linuxfoundation.org>
Cc: Android Kernel Team <kernel-team@android.com>
Signed-off-by: John Stultz <john.stultz@linaro.org>
---
drivers/staging/android/ion/ion_test.c | 1 +
drivers/staging/android/uapi/ion_test.h | 2 +-
2 files changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/staging/android/ion/ion_test.c b/drivers/staging/android/ion/ion_test.c
index 3e20349..654acb5 100644
--- a/drivers/staging/android/ion/ion_test.c
+++ b/drivers/staging/android/ion/ion_test.c
@@ -231,6 +231,7 @@ static int ion_test_release(struct inode *inode, struct file *file)
static const struct file_operations ion_test_fops = {
.owner = THIS_MODULE,
.unlocked_ioctl = ion_test_ioctl,
+ .compat_ioctl = ion_test_ioctl,
.open = ion_test_open,
.release = ion_test_release,
};
diff --git a/drivers/staging/android/uapi/ion_test.h b/drivers/staging/android/uapi/ion_test.h
index 614d1e3..f1727f5 100644
--- a/drivers/staging/android/uapi/ion_test.h
+++ b/drivers/staging/android/uapi/ion_test.h
@@ -31,7 +31,7 @@ struct ion_test_rw_data {
__u64 ptr;
__u64 offset;
__u64 size;
- int write;
+ __u64 write;
};
#define ION_IOC_MAGIC 'I'
--
1.8.3.2
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH] ion_test: Add compat_ioctl support
2013-12-19 22:40 [PATCH] ion_test: Add compat_ioctl support John Stultz
@ 2013-12-19 22:52 ` Colin Cross
2013-12-19 23:30 ` John Stultz
0 siblings, 1 reply; 6+ messages in thread
From: Colin Cross @ 2013-12-19 22:52 UTC (permalink / raw)
To: John Stultz; +Cc: LKML, Greg KH, Android Kernel Team
On Thu, Dec 19, 2013 at 2:40 PM, John Stultz <john.stultz@linaro.org> wrote:
> Prior to subitting this, Colin reworked the compat_ioctl support
> for the ion_test driver, moving the structure to be the same size
> on both 32 and 64 bit architectures.
>
> Two small things were left out. The compat_ioctl ptr assignment,
> and the fact that despite having uniform sized types in the
> structure, the structure pads out to different sizes on different
> arches.
>
> This patch resolves this issue by setting the write flag as
> a __u64, and adding the compat_ioctl ptr.
>
> While this does affect the ABI for 32bit users, its only
> the ABI for the ion_test driver, not ion itself.
>
> Cc: Colin Cross <ccross@android.com>
> Cc: Greg KH <gregkh@linuxfoundation.org>
> Cc: Android Kernel Team <kernel-team@android.com>
> Signed-off-by: John Stultz <john.stultz@linaro.org>
> ---
> drivers/staging/android/ion/ion_test.c | 1 +
> drivers/staging/android/uapi/ion_test.h | 2 +-
> 2 files changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/staging/android/ion/ion_test.c b/drivers/staging/android/ion/ion_test.c
> index 3e20349..654acb5 100644
> --- a/drivers/staging/android/ion/ion_test.c
> +++ b/drivers/staging/android/ion/ion_test.c
> @@ -231,6 +231,7 @@ static int ion_test_release(struct inode *inode, struct file *file)
> static const struct file_operations ion_test_fops = {
> .owner = THIS_MODULE,
> .unlocked_ioctl = ion_test_ioctl,
> + .compat_ioctl = ion_test_ioctl,
Setting the compat_ioctl to the same thing as unlocked_ioctl shouldn't
be necessary, compat_sys_ioctl will fall through to do_vfs_ioctl if
compat_ioctl is not set, which will call unlocked_ioctl.
> .open = ion_test_open,
> .release = ion_test_release,
> };
> diff --git a/drivers/staging/android/uapi/ion_test.h b/drivers/staging/android/uapi/ion_test.h
> index 614d1e3..f1727f5 100644
> --- a/drivers/staging/android/uapi/ion_test.h
> +++ b/drivers/staging/android/uapi/ion_test.h
> @@ -31,7 +31,7 @@ struct ion_test_rw_data {
> __u64 ptr;
> __u64 offset;
> __u64 size;
> - int write;
> + __u64 write;
> };
Whoops, missed that one. Can you add "int padding" after "int write"
instead? That at least lets us use the 32 bits later.
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] ion_test: Add compat_ioctl support
2013-12-19 22:52 ` Colin Cross
@ 2013-12-19 23:30 ` John Stultz
2013-12-19 23:45 ` Colin Cross
0 siblings, 1 reply; 6+ messages in thread
From: John Stultz @ 2013-12-19 23:30 UTC (permalink / raw)
To: Colin Cross; +Cc: LKML, Greg KH, Android Kernel Team
On 12/19/2013 02:52 PM, Colin Cross wrote:
> On Thu, Dec 19, 2013 at 2:40 PM, John Stultz <john.stultz@linaro.org> wrote:
>> Prior to subitting this, Colin reworked the compat_ioctl support
>> for the ion_test driver, moving the structure to be the same size
>> on both 32 and 64 bit architectures.
>>
>> Two small things were left out. The compat_ioctl ptr assignment,
>> and the fact that despite having uniform sized types in the
>> structure, the structure pads out to different sizes on different
>> arches.
>>
>> This patch resolves this issue by setting the write flag as
>> a __u64, and adding the compat_ioctl ptr.
>>
>> While this does affect the ABI for 32bit users, its only
>> the ABI for the ion_test driver, not ion itself.
>>
>> Cc: Colin Cross <ccross@android.com>
>> Cc: Greg KH <gregkh@linuxfoundation.org>
>> Cc: Android Kernel Team <kernel-team@android.com>
>> Signed-off-by: John Stultz <john.stultz@linaro.org>
>> ---
>> drivers/staging/android/ion/ion_test.c | 1 +
>> drivers/staging/android/uapi/ion_test.h | 2 +-
>> 2 files changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/staging/android/ion/ion_test.c b/drivers/staging/android/ion/ion_test.c
>> index 3e20349..654acb5 100644
>> --- a/drivers/staging/android/ion/ion_test.c
>> +++ b/drivers/staging/android/ion/ion_test.c
>> @@ -231,6 +231,7 @@ static int ion_test_release(struct inode *inode, struct file *file)
>> static const struct file_operations ion_test_fops = {
>> .owner = THIS_MODULE,
>> .unlocked_ioctl = ion_test_ioctl,
>> + .compat_ioctl = ion_test_ioctl,
> Setting the compat_ioctl to the same thing as unlocked_ioctl shouldn't
> be necessary, compat_sys_ioctl will fall through to do_vfs_ioctl if
> compat_ioctl is not set, which will call unlocked_ioctl.
Hrm. I've not looked into the implementation, but doesn't seem to be the
case on x86_64. Without it I get -1 back when calling the ioctl from a
32bit application. With it the ioctls work.
>
>> .open = ion_test_open,
>> .release = ion_test_release,
>> };
>> diff --git a/drivers/staging/android/uapi/ion_test.h b/drivers/staging/android/uapi/ion_test.h
>> index 614d1e3..f1727f5 100644
>> --- a/drivers/staging/android/uapi/ion_test.h
>> +++ b/drivers/staging/android/uapi/ion_test.h
>> @@ -31,7 +31,7 @@ struct ion_test_rw_data {
>> __u64 ptr;
>> __u64 offset;
>> __u64 size;
>> - int write;
>> + __u64 write;
>> };
> Whoops, missed that one. Can you add "int padding" after "int write"
> instead? That at least lets us use the 32 bits later.
Will do.
thanks
-john
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] ion_test: Add compat_ioctl support
2013-12-19 23:30 ` John Stultz
@ 2013-12-19 23:45 ` Colin Cross
2013-12-19 23:56 ` [PATCH] ion_test: Add compat_ioctl support (v2) John Stultz
0 siblings, 1 reply; 6+ messages in thread
From: Colin Cross @ 2013-12-19 23:45 UTC (permalink / raw)
To: John Stultz; +Cc: LKML, Greg KH, Android Kernel Team
On Thu, Dec 19, 2013 at 3:30 PM, John Stultz <john.stultz@linaro.org> wrote:
> On 12/19/2013 02:52 PM, Colin Cross wrote:
>> On Thu, Dec 19, 2013 at 2:40 PM, John Stultz <john.stultz@linaro.org> wrote:
>>> Prior to subitting this, Colin reworked the compat_ioctl support
>>> for the ion_test driver, moving the structure to be the same size
>>> on both 32 and 64 bit architectures.
>>>
>>> Two small things were left out. The compat_ioctl ptr assignment,
>>> and the fact that despite having uniform sized types in the
>>> structure, the structure pads out to different sizes on different
>>> arches.
>>>
>>> This patch resolves this issue by setting the write flag as
>>> a __u64, and adding the compat_ioctl ptr.
>>>
>>> While this does affect the ABI for 32bit users, its only
>>> the ABI for the ion_test driver, not ion itself.
>>>
>>> Cc: Colin Cross <ccross@android.com>
>>> Cc: Greg KH <gregkh@linuxfoundation.org>
>>> Cc: Android Kernel Team <kernel-team@android.com>
>>> Signed-off-by: John Stultz <john.stultz@linaro.org>
>>> ---
>>> drivers/staging/android/ion/ion_test.c | 1 +
>>> drivers/staging/android/uapi/ion_test.h | 2 +-
>>> 2 files changed, 2 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/staging/android/ion/ion_test.c b/drivers/staging/android/ion/ion_test.c
>>> index 3e20349..654acb5 100644
>>> --- a/drivers/staging/android/ion/ion_test.c
>>> +++ b/drivers/staging/android/ion/ion_test.c
>>> @@ -231,6 +231,7 @@ static int ion_test_release(struct inode *inode, struct file *file)
>>> static const struct file_operations ion_test_fops = {
>>> .owner = THIS_MODULE,
>>> .unlocked_ioctl = ion_test_ioctl,
>>> + .compat_ioctl = ion_test_ioctl,
>> Setting the compat_ioctl to the same thing as unlocked_ioctl shouldn't
>> be necessary, compat_sys_ioctl will fall through to do_vfs_ioctl if
>> compat_ioctl is not set, which will call unlocked_ioctl.
>
> Hrm. I've not looked into the implementation, but doesn't seem to be the
> case on x86_64. Without it I get -1 back when calling the ioctl from a
> 32bit application. With it the ioctls work.
I missed that compat_sys_ioctl returns -ENOTTY if do_ioctl_trans
fails, so the compat_ioctl is necessary.
>>
>>> .open = ion_test_open,
>>> .release = ion_test_release,
>>> };
>>> diff --git a/drivers/staging/android/uapi/ion_test.h b/drivers/staging/android/uapi/ion_test.h
>>> index 614d1e3..f1727f5 100644
>>> --- a/drivers/staging/android/uapi/ion_test.h
>>> +++ b/drivers/staging/android/uapi/ion_test.h
>>> @@ -31,7 +31,7 @@ struct ion_test_rw_data {
>>> __u64 ptr;
>>> __u64 offset;
>>> __u64 size;
>>> - int write;
>>> + __u64 write;
>>> };
>> Whoops, missed that one. Can you add "int padding" after "int write"
>> instead? That at least lets us use the 32 bits later.
>
> Will do.
>
>
> thanks
> -john
>
>
> To unsubscribe from this group and stop receiving emails from it, send an email to kernel-team+unsubscribe@android.com.
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH] ion_test: Add compat_ioctl support (v2)
2013-12-19 23:45 ` Colin Cross
@ 2013-12-19 23:56 ` John Stultz
2013-12-20 0:06 ` Colin Cross
0 siblings, 1 reply; 6+ messages in thread
From: John Stultz @ 2013-12-19 23:56 UTC (permalink / raw)
To: LKML; +Cc: John Stultz, Colin Cross, Greg KH, Android Kernel Team
Prior to subitting this, Colin reworked the compat_ioctl support
for the ion_test driver, moving the structure to be the same size
on both 32 and 64 bit architectures.
Two small things were left out. The compat_ioctl ptr assignment,
and the fact that despite having uniform sized types in the
structure, the structure pads out to different sizes on different
arches.
This patch resolves this issue by adding a padding entry after
the write flag, and adding the compat_ioctl ptr.
Changes in v2:
- Add a padding int rather then making write a u64
Cc: Colin Cross <ccross@android.com>
Cc: Greg KH <gregkh@linuxfoundation.org>
Cc: Android Kernel Team <kernel-team@android.com>
Signed-off-by: John Stultz <john.stultz@linaro.org>
---
drivers/staging/android/ion/ion_test.c | 1 +
drivers/staging/android/uapi/ion_test.h | 1 +
2 files changed, 2 insertions(+)
diff --git a/drivers/staging/android/ion/ion_test.c b/drivers/staging/android/ion/ion_test.c
index 3e20349..654acb5 100644
--- a/drivers/staging/android/ion/ion_test.c
+++ b/drivers/staging/android/ion/ion_test.c
@@ -231,6 +231,7 @@ static int ion_test_release(struct inode *inode, struct file *file)
static const struct file_operations ion_test_fops = {
.owner = THIS_MODULE,
.unlocked_ioctl = ion_test_ioctl,
+ .compat_ioctl = ion_test_ioctl,
.open = ion_test_open,
.release = ion_test_release,
};
diff --git a/drivers/staging/android/uapi/ion_test.h b/drivers/staging/android/uapi/ion_test.h
index 614d1e3..ffef06f 100644
--- a/drivers/staging/android/uapi/ion_test.h
+++ b/drivers/staging/android/uapi/ion_test.h
@@ -32,6 +32,7 @@ struct ion_test_rw_data {
__u64 offset;
__u64 size;
int write;
+ int __padding;
};
#define ION_IOC_MAGIC 'I'
--
1.8.3.2
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH] ion_test: Add compat_ioctl support (v2)
2013-12-19 23:56 ` [PATCH] ion_test: Add compat_ioctl support (v2) John Stultz
@ 2013-12-20 0:06 ` Colin Cross
0 siblings, 0 replies; 6+ messages in thread
From: Colin Cross @ 2013-12-20 0:06 UTC (permalink / raw)
To: John Stultz; +Cc: LKML, Greg KH, Android Kernel Team
On Thu, Dec 19, 2013 at 3:56 PM, John Stultz <john.stultz@linaro.org> wrote:
> Prior to subitting this, Colin reworked the compat_ioctl support
> for the ion_test driver, moving the structure to be the same size
> on both 32 and 64 bit architectures.
>
> Two small things were left out. The compat_ioctl ptr assignment,
> and the fact that despite having uniform sized types in the
> structure, the structure pads out to different sizes on different
> arches.
>
> This patch resolves this issue by adding a padding entry after
> the write flag, and adding the compat_ioctl ptr.
>
> Changes in v2:
> - Add a padding int rather then making write a u64
>
> Cc: Colin Cross <ccross@android.com>
> Cc: Greg KH <gregkh@linuxfoundation.org>
> Cc: Android Kernel Team <kernel-team@android.com>
> Signed-off-by: John Stultz <john.stultz@linaro.org>
> ---
> drivers/staging/android/ion/ion_test.c | 1 +
> drivers/staging/android/uapi/ion_test.h | 1 +
> 2 files changed, 2 insertions(+)
>
> diff --git a/drivers/staging/android/ion/ion_test.c b/drivers/staging/android/ion/ion_test.c
> index 3e20349..654acb5 100644
> --- a/drivers/staging/android/ion/ion_test.c
> +++ b/drivers/staging/android/ion/ion_test.c
> @@ -231,6 +231,7 @@ static int ion_test_release(struct inode *inode, struct file *file)
> static const struct file_operations ion_test_fops = {
> .owner = THIS_MODULE,
> .unlocked_ioctl = ion_test_ioctl,
> + .compat_ioctl = ion_test_ioctl,
> .open = ion_test_open,
> .release = ion_test_release,
> };
> diff --git a/drivers/staging/android/uapi/ion_test.h b/drivers/staging/android/uapi/ion_test.h
> index 614d1e3..ffef06f 100644
> --- a/drivers/staging/android/uapi/ion_test.h
> +++ b/drivers/staging/android/uapi/ion_test.h
> @@ -32,6 +32,7 @@ struct ion_test_rw_data {
> __u64 offset;
> __u64 size;
> int write;
> + int __padding;
> };
>
> #define ION_IOC_MAGIC 'I'
> --
> 1.8.3.2
>
> To unsubscribe from this group and stop receiving emails from it, send an email to kernel-team+unsubscribe@android.com.
Acked-by: Colin Cross <ccross@android.com>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2013-12-20 0:06 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-12-19 22:40 [PATCH] ion_test: Add compat_ioctl support John Stultz
2013-12-19 22:52 ` Colin Cross
2013-12-19 23:30 ` John Stultz
2013-12-19 23:45 ` Colin Cross
2013-12-19 23:56 ` [PATCH] ion_test: Add compat_ioctl support (v2) John Stultz
2013-12-20 0:06 ` Colin Cross
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.