Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Replace one-element array placeholders for alignment and padding with basic type objects #86

Closed
GustavoARSilva opened this issue Jul 15, 2020 · 1 comment
Assignees
Labels
[Idiom] fake flexible array [Refactor] 1-element array Conversion away from one-element array

Comments

@GustavoARSilva
Copy link
Collaborator

GustavoARSilva commented Jul 15, 2020

It is a common practice to add 'reserved' fields for alignment (or 'pad' fields for padding) in structures, and it is not uncommon to use one-element arrays for this purpose, as can be seen in the structure below.

drivers/tty/tty_io.c:2675:

struct serial_struct32 {
        compat_int_t    type;
        compat_int_t    line;
        compat_uint_t   port;
        compat_int_t    irq;
        compat_int_t    flags;
        compat_int_t    xmit_fifo_size;
        compat_int_t    custom_divisor;
        compat_int_t    baud_base;
        unsigned short  close_delay;
        char    io_type;
        char    reserved_char[1];
        compat_int_t    hub6;
        unsigned short  closing_wait; /* time to wait before closing */
        unsigned short  closing_wait2; /* no longer used... */
        compat_uint_t   iomem_base;
        unsigned short  iomem_reg_shift;
        unsigned int    port_high;
     /* compat_ulong_t  iomap_base FIXME */
        compat_int_t    reserved[1];
};

One-element arrays are being deprecated, therefore such reserved fields should be replaced with basic type objects, as follows:

diff --git a/drivers/tty/tty_io.c b/drivers/tty/tty_io.c
index 5a6f36b391d9..3b6e5ec3ba54 100644
--- a/drivers/tty/tty_io.c
+++ b/drivers/tty/tty_io.c
@@ -2683,7 +2683,7 @@ struct serial_struct32 {
         compat_int_t    baud_base;
         unsigned short  close_delay;
         char    io_type;
-        char    reserved_char[1];
+        char    reserved_char;
         compat_int_t    hub6;
         unsigned short  closing_wait; /* time to wait before closing */
         unsigned short  closing_wait2; /* no longer used... */
@@ -2691,7 +2691,7 @@ struct serial_struct32 {
         unsigned short  iomem_reg_shift;
         unsigned int    port_high;
      /* compat_ulong_t  iomap_base FIXME */
-        compat_int_t    reserved[1];
+        compat_int_t    reserved;
 };
 
 static int compat_tty_tiocsserial(struct tty_struct *tty,
@GustavoARSilva GustavoARSilva self-assigned this Jul 15, 2020
@kees kees changed the title Replace one-element array placeholders for a lignment with simple value types Replace one-element array placeholders for alignment with simple value types Jul 15, 2020
fengguang pushed a commit to 0day-ci/linux that referenced this issue Jul 15, 2020
One-element arrays are being deprecated[1]. Replace the one-element
array with a simple value type 'u8 reserved'[2], once this is just
a placeholder for alignment.

[1] KSPP#79
[2] KSPP#86

Tested-by: kernel test robot <[email protected]>
Link: https://github.com/GustavoARSilva/linux-hardening/blob/master/cii/0-day/wil6210-20200715.md
Signed-off-by: Gustavo A. R. Silva <[email protected]>
fengguang pushed a commit to 0day-ci/linux that referenced this issue Jul 16, 2020
One-element arrays are being deprecated[1]. Replace the one-element arrays
with simple value types 'char reserved_char' and 'compat_int_t reserved'[2],
once it seems these are just placeholders for alignment.

Also, while there, use the preferred form for passing a size of a struct.
The alternative form where struct name is spelled out hurts readability
and introduces an opportunity for a bug when the variable type is changed
but the corresponding sizeof that is passed as argument is not.

Lastly, fix the checkpatch.pl warnings below:

ERROR: code indent should use tabs where possible
+        char    reserved_char;$

WARNING: please, no spaces at the start of a line
+        char    reserved_char;$

ERROR: code indent should use tabs where possible
+        compat_int_t    reserved;$

WARNING: please, no spaces at the start of a line
+        compat_int_t    reserved;$

[1] KSPP#79
[2] KSPP#86

Tested-by: kernel test robot <[email protected]>
Link: https://github.com/GustavoARSilva/linux-hardening/blob/master/cii/0-day/tty-20200716.md
Signed-off-by: Gustavo A. R. Silva <[email protected]>
fengguang pushed a commit to 0day-ci/linux that referenced this issue Jul 17, 2020
One-element arrays are being deprecated[1]. Replace the one-element
array with a simple value type 'u8 reserved'[2], once it seems this
is just a placeholder for alignment.

[1] KSPP#79
[2] KSPP#86

Tested-by: kernel test robot <[email protected]>
Link: https://github.com/GustavoARSilva/linux-hardening/blob/master/cii/0-day/skylake-20200717.md
Signed-off-by: Gustavo A. R. Silva <[email protected]>
fengguang pushed a commit to 0day-ci/linux that referenced this issue Jul 20, 2020
One-element arrays are being deprecated[1]. Replace the one-element
array with a simple value type 'u8 reserved'[2], once it seems this
is just a placeholder for alignment.

[1] KSPP#79
[2] KSPP#86

Signed-off-by: Gustavo A. R. Silva <[email protected]>
Tested-by: kernel test robot <[email protected]>
Reviewed-by: Amadeusz Sławiński <[email protected]>
Link: https://github.com/GustavoARSilva/linux-hardening/blob/master/cii/0-day/skylake-20200717.md
Link: https://lore.kernel.org/r/20200717215500.GA13910@embeddedor
Signed-off-by: Mark Brown <[email protected]>
fengguang pushed a commit to 0day-ci/linux that referenced this issue Jul 22, 2020
One-element arrays are being deprecated[1]. Replace the one-element
array with a simple value type 'u32 reserved2'[2], once it seems
this is just a placeholder for alignment.

[1] KSPP#79
[2] KSPP#86

Tested-by: kernel test robot <[email protected]>
Link: https://github.com/GustavoARSilva/linux-hardening/blob/master/cii/0-day/tg3-20200718.md
Signed-off-by: Gustavo A. R. Silva <[email protected]>
fengguang pushed a commit to 0day-ci/linux that referenced this issue Jul 22, 2020
One-element arrays are being deprecated[1]. Replace the one-element
array with a simple value type 'u8 rsvd'[2], once it seems this is
just a placeholder for alignment.

[1] KSPP#79
[2] KSPP#86

Tested-by: kernel test robot <[email protected]>
Link: https://github.com/GustavoARSilva/linux-hardening/blob/master/cii/0-day/bfi-20200718.md
Signed-off-by: Gustavo A. R. Silva <[email protected]>
fengguang pushed a commit to 0day-ci/linux that referenced this issue Jul 22, 2020
One-element arrays are being deprecated[1]. Replace the one-element arrays
with simple value types 'char reserved_char' and 'compat_int_t reserved'[2],
once it seems these are just placeholders for alignment.

Also, while there, use the preferred form for passing a size of a struct.
The alternative form where struct name is spelled out hurts readability
and introduces an opportunity for a bug when the variable type is changed
but the corresponding sizeof that is passed as argument is not.

Lastly, fix the checkpatch.pl warnings below:

ERROR: code indent should use tabs where possible
+        char    reserved_char;$

WARNING: please, no spaces at the start of a line
+        char    reserved_char;$

ERROR: code indent should use tabs where possible
+        compat_int_t    reserved;$

WARNING: please, no spaces at the start of a line
+        compat_int_t    reserved;$

[1] KSPP#79
[2] KSPP#86

Tested-by: kernel test robot <[email protected]>
Link: https://github.com/GustavoARSilva/linux-hardening/blob/master/cii/0-day/tty-20200716.md
Acked-by: Jiri Slaby <[email protected]>
Signed-off-by: Gustavo A. R. Silva <[email protected]>
fengguang pushed a commit to 0day-ci/linux that referenced this issue Jul 22, 2020
One-element arrays are being deprecated[1]. Replace the one-element
array with a simple value type '__le32 reserved1'[2], once it seems
this is just a placeholder for alignment.

[1] KSPP#79
[2] KSPP#86

Tested-by: kernel test robot <[email protected]>
Link: https://github.com/GustavoARSilva/linux-hardening/blob/master/cii/0-day/qed_hsi-20200718.md
Signed-off-by: Gustavo A. R. Silva <[email protected]>
fengguang pushed a commit to 0day-ci/linux that referenced this issue Jul 23, 2020
One-element arrays are being deprecated[1]. Replace the one-element arrays
with simple value types 'char reserved_char' and 'compat_int_t reserved'[2],
once it seems these are just placeholders for alignment.

[1] KSPP#79
[2] KSPP#86

Tested-by: kernel test robot <[email protected]>
Link: https://github.com/GustavoARSilva/linux-hardening/blob/master/cii/0-day/tty-20200716.md
Acked-by: Jiri Slaby <[email protected]>
Signed-off-by: Gustavo A. R. Silva <[email protected]>
ruscur pushed a commit to ruscur/linux that referenced this issue Jul 30, 2020
One-element arrays are being deprecated[1]. Replace the one-element arrays
with simple value types 'char reserved_char' and 'compat_int_t reserved'[2],
once it seems these are just placeholders for alignment.

[1] KSPP#79
[2] KSPP#86

Tested-by: kernel test robot <[email protected]>
Link: https://github.com/GustavoARSilva/linux-hardening/blob/master/cii/0-day/tty-20200716.md
Acked-by: Jiri Slaby <[email protected]>
Signed-off-by: Gustavo A. R. Silva <[email protected]>
Link: https://lore.kernel.org/r/f49bf0e27eaac396c96d21392c8c284f9f5ef52a.1595543280.git.gustavoars@kernel.org
Signed-off-by: Greg Kroah-Hartman <[email protected]>
kitakar5525 pushed a commit to kitakar5525/linux-kernel that referenced this issue Sep 16, 2020
One-element arrays are being deprecated[1]. Replace the one-element
array with a simple value type 'u8 reserved'[2], once it seems this
is just a placeholder for alignment.

[1] KSPP/linux#79
[2] KSPP/linux#86

Signed-off-by: Gustavo A. R. Silva <[email protected]>
Tested-by: kernel test robot <[email protected]>
Reviewed-by: Amadeusz Sławiński <[email protected]>
Link: https://github.com/GustavoARSilva/linux-hardening/blob/master/cii/0-day/skylake-20200717.md
Link: https://lore.kernel.org/r/20200717215500.GA13910@embeddedor
Signed-off-by: Mark Brown <[email protected]>
(cherry picked from commit 23f8d96)

BUG=b:162008784
TEST=Test audio on volteer.

Signed-off-by: Mike Mason <[email protected]>
@GustavoARSilva GustavoARSilva changed the title Replace one-element array placeholders for alignment with simple value types Replace one-element array placeholders for alignment and padding with simple value types Sep 28, 2020
fengguang pushed a commit to 0day-ci/linux that referenced this issue Sep 29, 2020
One-element arrays are being deprecated[1]. Replace the one-element array
with a simple object of type u_char: 'u_char rm_pad1'[2], once it seems
this is just a placeholder for padding.

[1] https://www.kernel.org/doc/html/v5.9-rc1/process/deprecated.html#zero-length-and-one-element-arrays
[2] KSPP#86

Built-tested-by: kernel test robot <[email protected]>
Link: https://lore.kernel.org/lkml/5f72c23f.%2FkPBWcZBu+W6HKH4%[email protected]/
Signed-off-by: Gustavo A. R. Silva <[email protected]>
fengguang pushed a commit to 0day-ci/linux that referenced this issue Sep 30, 2020
One-element arrays are being deprecated[1]. Replace the one-element array
with a simple object of type u_char: 'u_char rm_pad1'[2], once it seems
this is just a placeholder for padding.

[1] https://www.kernel.org/doc/html/v5.9-rc1/process/deprecated.html#zero-length-and-one-element-arrays
[2] KSPP#86

Built-tested-by: kernel test robot <[email protected]>
Link: https://lore.kernel.org/lkml/5f72c23f.%2FkPBWcZBu+W6HKH4%[email protected]/
Signed-off-by: Gustavo A. R. Silva <[email protected]>
Signed-off-by: David S. Miller <[email protected]>
fengguang pushed a commit to 0day-ci/linux that referenced this issue Oct 2, 2020
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] KSPP#86

Build-tested-by: kernel test robot <[email protected]>
Link: https://lore.kernel.org/lkml/5f76f5d0.qJ4t%2FHWuRzSW7bTa%[email protected]/
Signed-off-by: Gustavo A. R. Silva <[email protected]>
ruscur pushed a commit to ruscur/linux that referenced this issue Oct 6, 2020
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] KSPP#86

Signed-off-by: Gustavo A. R. Silva <[email protected]>
Link: https://lore.kernel.org/lkml/5f76f5d0.qJ4t%2FHWuRzSW7bTa%[email protected]/
Build-tested-by: kernel test robot <[email protected]>
Signed-off-by: Jens Axboe <[email protected]>
@GustavoARSilva GustavoARSilva changed the title Replace one-element array placeholders for alignment and padding with simple value types Replace one-element array placeholders for alignment and padding with basic type objects Jun 10, 2021
@kees
Copy link

kees commented Feb 6, 2024

I think with -fstrict-flex-arrays=3, we can safely ignore this now. If it's not being used as a flexible array, it will be handled sanely now.

@kees kees closed this as completed Feb 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Idiom] fake flexible array [Refactor] 1-element array Conversion away from one-element array
Projects
None yet
Development

No branches or pull requests

2 participants