Messages in this thread | | | Date | Thu, 23 Jan 2020 19:56:09 +0100 | From | Greg Kroah-Hartman <> | Subject | Re: [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
| |