lkml.org 
[lkml]   [2020]   [Jan]   [23]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH] char: hpet: Use flexible-array member
On Thu, Jan 23, 2020 at 12:46:42PM -0600, Gustavo A. R. Silva wrote:
>
>
> On 1/23/20 12:25, Greg Kroah-Hartman wrote:
> > On Mon, Jan 20, 2020 at 05:53:26PM -0600, Gustavo A. R. Silva wrote:
> >> Old code in the kernel uses 1-byte and 0-byte arrays to indicate the
> >> presence of a "variable length array":
> >>
> >> struct something {
> >> int length;
> >> u8 data[1];
> >> };
> >>
> >> struct something *instance;
> >>
> >> instance = kmalloc(sizeof(*instance) + size, GFP_KERNEL);
> >> instance->length = size;
> >> memcpy(instance->data, source, size);
> >>
> >> There is also 0-byte arrays. Both cases pose confusion for things like
> >> sizeof(), CONFIG_FORTIFY_SOURCE, etc.[1] Instead, the preferred mechanism
> >> to declare variable-length types such as the one above is a flexible array
> >> member[2] which need to be the last member of a structure and empty-sized:
> >>
> >> struct something {
> >> int stuff;
> >> u8 data[];
> >> };
> >>
> >> Also, by making use of the mechanism above, we will get a compiler warning
> >> in case the flexible array does not occur last in the structure, which
> >> will help us prevent some kind of undefined behavior bugs from being
> >> unadvertenly introduced[3] to the codebase from now on.
> >>
> >> [1] https://github.com/KSPP/linux/issues/21
> >> [2] https://gcc.gnu.org/onlinedocs/gcc/Zero-Length.html
> >> [3] commit 76497732932f ("cxgb3/l2t: Fix undefined behaviour")
> >>
> >> Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>
> >> ---
> >> drivers/char/hpet.c | 2 +-
> >> 1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/char/hpet.c b/drivers/char/hpet.c
> >> index 9ac6671bb514..aed2c45f7968 100644
> >> --- a/drivers/char/hpet.c
> >> +++ b/drivers/char/hpet.c
> >> @@ -110,7 +110,7 @@ struct hpets {
> >> unsigned long hp_delta;
> >> unsigned int hp_ntimer;
> >> unsigned int hp_which;
> >> - struct hpet_dev hp_dev[1];
> >> + struct hpet_dev hp_dev[];
> >
> > Are you sure the allocation size is the same again? Much like the
> > n_hdlc patch was, I think you need to adjust the variable size here.
> > Maybe, it's a bit of a pain to figure out at a quick glance, I just want
> > to make sure you at least do look at that :)
> >
>
> Yep. The allocation thing was already handled almost a year ago by the
> following patch, and it didn't require to increase the size at that time:
>
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=401c9bd10beef4b030eb9e34d16b5341dc6c683b

Great, thanks for verifying, I'll go queue this up now.

greg k-h

\
 
 \ /
  Last update: 2020-01-23 19:56    [W:0.080 / U:0.048 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site