Re: [PATCH] staging: android: uapi: drop definitions of removed ION_IOC_{FREE,SHARE} ioctls
On Wed, Jun 14, 2017 at 07:08:39AM +0200, Greg Kroah-Hartman wrote: > On Tue, Jun 13, 2017 at 09:17:05PM +0300, Gleb Fotengauer-Malinovskiy wrote: > > On Tue, May 30, 2017 at 04:33:57PM -0700, Laura Abbott wrote: > > > On 05/30/2017 07:11 AM, Gleb Fotengauer-Malinovskiy wrote: > > > > This problem was found by strace ioctl list generator. > > > > > > > > Fixes: 15c6098cfec5 ("staging: android: ion: Remove ion_handle and > > > > ion_client") > > > > As this commit fixes a regression, please apply it to the tree which will > > be merged before 4.12 release, too. > > What "regression" is there? The fact that a staging driver has a few > spare ioctls floating around in a header file? How is that bad? I thought it was pretty obvious. OK, here is a bit more detailed explanation. There is an uapi header that after commit 15c6098cfec5 provides definitions of ioctl macros that do not compile when used. Imagine a userspace code that does something as harmless as #ifdef ION_IOC_FREE use(ION_IOC_FREE); #endif This simple code is broken by commit 15c6098cfec5. The regression is not a pure virtual one, there is a quite real userspace (strace ioctl list generator) broken by the change. -- ldv signature.asc Description: PGP signature
Re: [PATCH] staging: android: uapi: drop definitions of removed ION_IOC_{FREE,SHARE} ioctls
On Wed, Jun 14, 2017 at 07:08:39AM +0200, Greg Kroah-Hartman wrote: > On Tue, Jun 13, 2017 at 09:17:05PM +0300, Gleb Fotengauer-Malinovskiy wrote: > > On Tue, May 30, 2017 at 04:33:57PM -0700, Laura Abbott wrote: > > > On 05/30/2017 07:11 AM, Gleb Fotengauer-Malinovskiy wrote: > > > > This problem was found by strace ioctl list generator. > > > > > > > > Fixes: 15c6098cfec5 ("staging: android: ion: Remove ion_handle and > > > > ion_client") > > > > As this commit fixes a regression, please apply it to the tree which will > > be merged before 4.12 release, too. > > What "regression" is there? The fact that a staging driver has a few > spare ioctls floating around in a header file? How is that bad? I thought it was pretty obvious. OK, here is a bit more detailed explanation. There is an uapi header that after commit 15c6098cfec5 provides definitions of ioctl macros that do not compile when used. Imagine a userspace code that does something as harmless as #ifdef ION_IOC_FREE use(ION_IOC_FREE); #endif This simple code is broken by commit 15c6098cfec5. The regression is not a pure virtual one, there is a quite real userspace (strace ioctl list generator) broken by the change. -- ldv signature.asc Description: PGP signature
Re: [PATCH] staging: android: uapi: drop definitions of removed ION_IOC_{FREE,SHARE} ioctls
On Tue, Jun 13, 2017 at 09:17:05PM +0300, Gleb Fotengauer-Malinovskiy wrote: > On Tue, May 30, 2017 at 04:33:57PM -0700, Laura Abbott wrote: > > On 05/30/2017 07:11 AM, Gleb Fotengauer-Malinovskiy wrote: > > > This problem was found by strace ioctl list generator. > > > > > > Fixes: 15c6098cfec5 ("staging: android: ion: Remove ion_handle and > > > ion_client") > > As this commit fixes a regression, please apply it to the tree which will > be merged before 4.12 release, too. What "regression" is there? The fact that a staging driver has a few spare ioctls floating around in a header file? How is that bad? thanks, greg k-h
Re: [PATCH] staging: android: uapi: drop definitions of removed ION_IOC_{FREE,SHARE} ioctls
On Tue, Jun 13, 2017 at 09:17:05PM +0300, Gleb Fotengauer-Malinovskiy wrote: > On Tue, May 30, 2017 at 04:33:57PM -0700, Laura Abbott wrote: > > On 05/30/2017 07:11 AM, Gleb Fotengauer-Malinovskiy wrote: > > > This problem was found by strace ioctl list generator. > > > > > > Fixes: 15c6098cfec5 ("staging: android: ion: Remove ion_handle and > > > ion_client") > > As this commit fixes a regression, please apply it to the tree which will > be merged before 4.12 release, too. What "regression" is there? The fact that a staging driver has a few spare ioctls floating around in a header file? How is that bad? thanks, greg k-h
Re: [PATCH] staging: android: uapi: drop definitions of removed ION_IOC_{FREE,SHARE} ioctls
On Tue, May 30, 2017 at 04:33:57PM -0700, Laura Abbott wrote: > On 05/30/2017 07:11 AM, Gleb Fotengauer-Malinovskiy wrote: > > This problem was found by strace ioctl list generator. > > > > Fixes: 15c6098cfec5 ("staging: android: ion: Remove ion_handle and > > ion_client") As this commit fixes a regression, please apply it to the tree which will be merged before 4.12 release, too. Otherwise the regression will be introduced to the release. -- glebfm
Re: [PATCH] staging: android: uapi: drop definitions of removed ION_IOC_{FREE,SHARE} ioctls
On Tue, May 30, 2017 at 04:33:57PM -0700, Laura Abbott wrote: > On 05/30/2017 07:11 AM, Gleb Fotengauer-Malinovskiy wrote: > > This problem was found by strace ioctl list generator. > > > > Fixes: 15c6098cfec5 ("staging: android: ion: Remove ion_handle and > > ion_client") As this commit fixes a regression, please apply it to the tree which will be merged before 4.12 release, too. Otherwise the regression will be introduced to the release. -- glebfm
Re: [PATCH] staging: android: uapi: drop definitions of removed ION_IOC_{FREE,SHARE} ioctls
On 05/30/2017 07:11 AM, Gleb Fotengauer-Malinovskiy wrote: > This problem was found by strace ioctl list generator. > > Fixes: 15c6098cfec5 ("staging: android: ion: Remove ion_handle and > ion_client") > Signed-off-by: Gleb Fotengauer-Malinovskiy> --- > drivers/staging/android/uapi/ion.h | 18 -- > 1 file changed, 18 deletions(-) > > diff --git a/drivers/staging/android/uapi/ion.h > b/drivers/staging/android/uapi/ion.h > index b76db1b..a291b12 100644 > --- a/drivers/staging/android/uapi/ion.h > +++ b/drivers/staging/android/uapi/ion.h > @@ -131,24 +131,6 @@ struct ion_heap_query { > struct ion_allocation_data) > > /** > - * DOC: ION_IOC_FREE - free memory > - * > - * Takes an ion_handle_data struct and frees the handle. > - */ > -#define ION_IOC_FREE _IOWR(ION_IOC_MAGIC, 1, struct ion_handle_data) > - > -/** > - * DOC: ION_IOC_SHARE - creates a file descriptor to use to share an > allocation > - * > - * Takes an ion_fd_data struct with the handle field populated with a valid > - * opaque handle. Returns the struct with the fd field set to a file > - * descriptor open in the current address space. This file descriptor > - * can then be passed to another process. The corresponding opaque handle > can > - * be retrieved via ION_IOC_IMPORT. > - */ > -#define ION_IOC_SHARE_IOWR(ION_IOC_MAGIC, 4, struct > ion_fd_data) > - > -/** > * DOC: ION_IOC_HEAP_QUERY - information about available heaps > * > * Takes an ion_heap_query structure and populates information about > Acked-by: Laura Abbott
Re: [PATCH] staging: android: uapi: drop definitions of removed ION_IOC_{FREE,SHARE} ioctls
On 05/30/2017 07:11 AM, Gleb Fotengauer-Malinovskiy wrote: > This problem was found by strace ioctl list generator. > > Fixes: 15c6098cfec5 ("staging: android: ion: Remove ion_handle and > ion_client") > Signed-off-by: Gleb Fotengauer-Malinovskiy > --- > drivers/staging/android/uapi/ion.h | 18 -- > 1 file changed, 18 deletions(-) > > diff --git a/drivers/staging/android/uapi/ion.h > b/drivers/staging/android/uapi/ion.h > index b76db1b..a291b12 100644 > --- a/drivers/staging/android/uapi/ion.h > +++ b/drivers/staging/android/uapi/ion.h > @@ -131,24 +131,6 @@ struct ion_heap_query { > struct ion_allocation_data) > > /** > - * DOC: ION_IOC_FREE - free memory > - * > - * Takes an ion_handle_data struct and frees the handle. > - */ > -#define ION_IOC_FREE _IOWR(ION_IOC_MAGIC, 1, struct ion_handle_data) > - > -/** > - * DOC: ION_IOC_SHARE - creates a file descriptor to use to share an > allocation > - * > - * Takes an ion_fd_data struct with the handle field populated with a valid > - * opaque handle. Returns the struct with the fd field set to a file > - * descriptor open in the current address space. This file descriptor > - * can then be passed to another process. The corresponding opaque handle > can > - * be retrieved via ION_IOC_IMPORT. > - */ > -#define ION_IOC_SHARE_IOWR(ION_IOC_MAGIC, 4, struct > ion_fd_data) > - > -/** > * DOC: ION_IOC_HEAP_QUERY - information about available heaps > * > * Takes an ion_heap_query structure and populates information about > Acked-by: Laura Abbott
[PATCH] staging: android: uapi: drop definitions of removed ION_IOC_{FREE,SHARE} ioctls
This problem was found by strace ioctl list generator. Fixes: 15c6098cfec5 ("staging: android: ion: Remove ion_handle and ion_client") Signed-off-by: Gleb Fotengauer-Malinovskiy--- drivers/staging/android/uapi/ion.h | 18 -- 1 file changed, 18 deletions(-) diff --git a/drivers/staging/android/uapi/ion.h b/drivers/staging/android/uapi/ion.h index b76db1b..a291b12 100644 --- a/drivers/staging/android/uapi/ion.h +++ b/drivers/staging/android/uapi/ion.h @@ -131,24 +131,6 @@ struct ion_heap_query { struct ion_allocation_data) /** - * DOC: ION_IOC_FREE - free memory - * - * Takes an ion_handle_data struct and frees the handle. - */ -#define ION_IOC_FREE _IOWR(ION_IOC_MAGIC, 1, struct ion_handle_data) - -/** - * DOC: ION_IOC_SHARE - creates a file descriptor to use to share an allocation - * - * Takes an ion_fd_data struct with the handle field populated with a valid - * opaque handle. Returns the struct with the fd field set to a file - * descriptor open in the current address space. This file descriptor - * can then be passed to another process. The corresponding opaque handle can - * be retrieved via ION_IOC_IMPORT. - */ -#define ION_IOC_SHARE _IOWR(ION_IOC_MAGIC, 4, struct ion_fd_data) - -/** * DOC: ION_IOC_HEAP_QUERY - information about available heaps * * Takes an ion_heap_query structure and populates information about -- glebfm
[PATCH] staging: android: uapi: drop definitions of removed ION_IOC_{FREE,SHARE} ioctls
This problem was found by strace ioctl list generator. Fixes: 15c6098cfec5 ("staging: android: ion: Remove ion_handle and ion_client") Signed-off-by: Gleb Fotengauer-Malinovskiy --- drivers/staging/android/uapi/ion.h | 18 -- 1 file changed, 18 deletions(-) diff --git a/drivers/staging/android/uapi/ion.h b/drivers/staging/android/uapi/ion.h index b76db1b..a291b12 100644 --- a/drivers/staging/android/uapi/ion.h +++ b/drivers/staging/android/uapi/ion.h @@ -131,24 +131,6 @@ struct ion_heap_query { struct ion_allocation_data) /** - * DOC: ION_IOC_FREE - free memory - * - * Takes an ion_handle_data struct and frees the handle. - */ -#define ION_IOC_FREE _IOWR(ION_IOC_MAGIC, 1, struct ion_handle_data) - -/** - * DOC: ION_IOC_SHARE - creates a file descriptor to use to share an allocation - * - * Takes an ion_fd_data struct with the handle field populated with a valid - * opaque handle. Returns the struct with the fd field set to a file - * descriptor open in the current address space. This file descriptor - * can then be passed to another process. The corresponding opaque handle can - * be retrieved via ION_IOC_IMPORT. - */ -#define ION_IOC_SHARE _IOWR(ION_IOC_MAGIC, 4, struct ion_fd_data) - -/** * DOC: ION_IOC_HEAP_QUERY - information about available heaps * * Takes an ion_heap_query structure and populates information about -- glebfm