Messages in this thread | | | From | "Gustavo A. R. Silva" <> | Subject | Re: [PATCH] char: hpet: Use flexible-array member | Date | Thu, 23 Jan 2020 12:46:42 -0600 |
| |
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
Thanks -- Gustavo
| |