* [PATCH][next] block: scsi_ioctl: Avoid the use of one-element arrays
@ 2020-10-02 23:10 Gustavo A. R. Silva
2020-10-02 23:53 ` Jens Axboe
2020-10-05 7:25 ` Christoph Hellwig
0 siblings, 2 replies; 6+ messages in thread
From: Gustavo A. R. Silva @ 2020-10-02 23:10 UTC (permalink / raw)
To: Jens Axboe
Cc: linux-block, linux-kernel, Gustavo A. R. Silva, linux-hardening,
Kees Cook
One-element arrays are being deprecated[1]. Replace the one-element array
with a simple object of type compat_caddr_t: 'compat_caddr_t unused'[2],
once it seems this field is actually never used.
Also, update struct cdrom_generic_command in UAPI by adding an
anonimous union to avoid using the one-element array _reserved_.
[1] https://www.kernel.org/doc/html/v5.9-rc1/process/deprecated.html#zero-length-and-one-element-arrays
[2] https://github.com/KSPP/linux/issues/86
Build-tested-by: kernel test robot <lkp@intel.com>
Link: https://lore.kernel.org/lkml/5f76f5d0.qJ4t%2FHWuRzSW7bTa%25lkp@intel.com/
Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org>
---
block/scsi_ioctl.c | 6 +++---
include/uapi/linux/cdrom.h | 5 ++++-
2 files changed, 7 insertions(+), 4 deletions(-)
diff --git a/block/scsi_ioctl.c b/block/scsi_ioctl.c
index 600e38cb69b2..2dfb699389df 100644
--- a/block/scsi_ioctl.c
+++ b/block/scsi_ioctl.c
@@ -643,7 +643,7 @@ struct compat_cdrom_generic_command {
unsigned char data_direction;
compat_int_t quiet;
compat_int_t timeout;
- compat_caddr_t reserved[1];
+ compat_caddr_t unused;
};
#endif
@@ -665,7 +665,7 @@ static int scsi_get_cdrom_generic_arg(struct cdrom_generic_command *cgc,
.data_direction = cgc32.data_direction,
.quiet = cgc32.quiet,
.timeout = cgc32.timeout,
- .reserved[0] = compat_ptr(cgc32.reserved[0]),
+ .unused = compat_ptr(cgc32.unused),
};
memcpy(&cgc->cmd, &cgc32.cmd, CDROM_PACKET_SIZE);
return 0;
@@ -690,7 +690,7 @@ static int scsi_put_cdrom_generic_arg(const struct cdrom_generic_command *cgc,
.data_direction = cgc->data_direction,
.quiet = cgc->quiet,
.timeout = cgc->timeout,
- .reserved[0] = (uintptr_t)(cgc->reserved[0]),
+ .unused = (uintptr_t)(cgc->unused),
};
memcpy(&cgc32.cmd, &cgc->cmd, CDROM_PACKET_SIZE);
diff --git a/include/uapi/linux/cdrom.h b/include/uapi/linux/cdrom.h
index 2817230148fd..6c34f6e2f1f7 100644
--- a/include/uapi/linux/cdrom.h
+++ b/include/uapi/linux/cdrom.h
@@ -289,7 +289,10 @@ struct cdrom_generic_command
unsigned char data_direction;
int quiet;
int timeout;
- void __user *reserved[1]; /* unused, actually */
+ union {
+ void __user *reserved[1]; /* unused, actually */
+ void __user *unused;
+ };
};
/*
--
2.27.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH][next] block: scsi_ioctl: Avoid the use of one-element arrays
2020-10-02 23:10 [PATCH][next] block: scsi_ioctl: Avoid the use of one-element arrays Gustavo A. R. Silva
@ 2020-10-02 23:53 ` Jens Axboe
2020-10-03 0:03 ` Gustavo A. R. Silva
2020-10-05 7:25 ` Christoph Hellwig
1 sibling, 1 reply; 6+ messages in thread
From: Jens Axboe @ 2020-10-02 23:53 UTC (permalink / raw)
To: Gustavo A. R. Silva; +Cc: linux-block, linux-kernel, linux-hardening, Kees Cook
On 10/2/20 5:10 PM, Gustavo A. R. Silva wrote:
> diff --git a/include/uapi/linux/cdrom.h b/include/uapi/linux/cdrom.h
> index 2817230148fd..6c34f6e2f1f7 100644
> --- a/include/uapi/linux/cdrom.h
> +++ b/include/uapi/linux/cdrom.h
> @@ -289,7 +289,10 @@ struct cdrom_generic_command
> unsigned char data_direction;
> int quiet;
> int timeout;
> - void __user *reserved[1]; /* unused, actually */
> + union {
> + void __user *reserved[1]; /* unused, actually */
> + void __user *unused;
> + };
What's the point of this union, why not just turn it into
void * __user *unused;
?
--
Jens Axboe
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH][next] block: scsi_ioctl: Avoid the use of one-element arrays
2020-10-03 0:03 ` Gustavo A. R. Silva
@ 2020-10-02 23:58 ` Jens Axboe
2020-10-03 0:07 ` Gustavo A. R. Silva
0 siblings, 1 reply; 6+ messages in thread
From: Jens Axboe @ 2020-10-02 23:58 UTC (permalink / raw)
To: Gustavo A. R. Silva; +Cc: linux-block, linux-kernel, linux-hardening, Kees Cook
On 10/2/20 6:03 PM, Gustavo A. R. Silva wrote:
> On Fri, Oct 02, 2020 at 05:53:05PM -0600, Jens Axboe wrote:
>> On 10/2/20 5:10 PM, Gustavo A. R. Silva wrote:
>>> diff --git a/include/uapi/linux/cdrom.h b/include/uapi/linux/cdrom.h
>>> index 2817230148fd..6c34f6e2f1f7 100644
>>> --- a/include/uapi/linux/cdrom.h
>>> +++ b/include/uapi/linux/cdrom.h
>>> @@ -289,7 +289,10 @@ struct cdrom_generic_command
>>> unsigned char data_direction;
>>> int quiet;
>>> int timeout;
>>> - void __user *reserved[1]; /* unused, actually */
>>> + union {
>>> + void __user *reserved[1]; /* unused, actually */
>>> + void __user *unused;
>>> + };
>>
>> What's the point of this union, why not just turn it into
>>
>> void * __user *unused;
>>
>> ?
>
> I just don't want to take any chances of breaking any user-space
> application that, for some reason, may be considering that field.
I guess that's a valid concern, who knows what applications are doing
to an ignored field.
I'll apply it, thanks.
--
Jens Axboe
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH][next] block: scsi_ioctl: Avoid the use of one-element arrays
2020-10-02 23:53 ` Jens Axboe
@ 2020-10-03 0:03 ` Gustavo A. R. Silva
2020-10-02 23:58 ` Jens Axboe
0 siblings, 1 reply; 6+ messages in thread
From: Gustavo A. R. Silva @ 2020-10-03 0:03 UTC (permalink / raw)
To: Jens Axboe; +Cc: linux-block, linux-kernel, linux-hardening, Kees Cook
On Fri, Oct 02, 2020 at 05:53:05PM -0600, Jens Axboe wrote:
> On 10/2/20 5:10 PM, Gustavo A. R. Silva wrote:
> > diff --git a/include/uapi/linux/cdrom.h b/include/uapi/linux/cdrom.h
> > index 2817230148fd..6c34f6e2f1f7 100644
> > --- a/include/uapi/linux/cdrom.h
> > +++ b/include/uapi/linux/cdrom.h
> > @@ -289,7 +289,10 @@ struct cdrom_generic_command
> > unsigned char data_direction;
> > int quiet;
> > int timeout;
> > - void __user *reserved[1]; /* unused, actually */
> > + union {
> > + void __user *reserved[1]; /* unused, actually */
> > + void __user *unused;
> > + };
>
> What's the point of this union, why not just turn it into
>
> void * __user *unused;
>
> ?
I just don't want to take any chances of breaking any user-space
application that, for some reason, may be considering that field.
--
Gustavo
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH][next] block: scsi_ioctl: Avoid the use of one-element arrays
2020-10-02 23:58 ` Jens Axboe
@ 2020-10-03 0:07 ` Gustavo A. R. Silva
0 siblings, 0 replies; 6+ messages in thread
From: Gustavo A. R. Silva @ 2020-10-03 0:07 UTC (permalink / raw)
To: Jens Axboe; +Cc: linux-block, linux-kernel, linux-hardening, Kees Cook
On Fri, Oct 02, 2020 at 05:58:33PM -0600, Jens Axboe wrote:
> >>> - void __user *reserved[1]; /* unused, actually */
> >>> + union {
> >>> + void __user *reserved[1]; /* unused, actually */
> >>> + void __user *unused;
> >>> + };
> >>
> >> What's the point of this union, why not just turn it into
> >>
> >> void * __user *unused;
> >>
> >> ?
> >
> > I just don't want to take any chances of breaking any user-space
> > application that, for some reason, may be considering that field.
>
> I guess that's a valid concern, who knows what applications are doing
> to an ignored field.
>
> I'll apply it, thanks.
Awesome. :)
Thanks.
--
Gustavo
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH][next] block: scsi_ioctl: Avoid the use of one-element arrays
2020-10-02 23:10 [PATCH][next] block: scsi_ioctl: Avoid the use of one-element arrays Gustavo A. R. Silva
2020-10-02 23:53 ` Jens Axboe
@ 2020-10-05 7:25 ` Christoph Hellwig
1 sibling, 0 replies; 6+ messages in thread
From: Christoph Hellwig @ 2020-10-05 7:25 UTC (permalink / raw)
To: Gustavo A. R. Silva
Cc: Jens Axboe, linux-block, linux-kernel, linux-hardening, Kees Cook
On Fri, Oct 02, 2020 at 06:10:33PM -0500, Gustavo A. R. Silva wrote:
> One-element arrays are being deprecated[1]. Replace the one-element array
> with a simple object of type compat_caddr_t: 'compat_caddr_t unused'[2],
> once it seems this field is actually never used.
They are only deprecated when abused as variable length array. That is
not the case here.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-10-05 7:26 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-10-02 23:10 [PATCH][next] block: scsi_ioctl: Avoid the use of one-element arrays Gustavo A. R. Silva
2020-10-02 23:53 ` Jens Axboe
2020-10-03 0:03 ` Gustavo A. R. Silva
2020-10-02 23:58 ` Jens Axboe
2020-10-03 0:07 ` Gustavo A. R. Silva
2020-10-05 7:25 ` Christoph Hellwig
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).