tpm: eventlog: Replace zero-length array with flexible-array member

As mentioned above: "Flexible array members have incomplete type, and so the sizeof operator may not be applied. As a quirk of the original implementation of zero-length arrays, sizeof evaluates to zero."[1] So, the sizeof(flexible-array) can be safely removed to fix the error above.

Quoted 4 lines · Gustavo A. R. Silva

As mentioned above: "Flexible array members have incomplete type, and so the sizeof operator may not be applied. As a quirk of the original implementation of zero-length arrays, sizeof evaluates to zero."[1] So, the sizeof(flexible-array) can be safely removed to fix the error above.

As in "sizeof(event_header->event) always evaluated to 0, so removing it has no effect".

drivers/char/tpm/eventlog/tpm2.c static void *tpm2_bios_measurements_start(struct seq_file *m, loff_t *pos) · line 54

Quoted 12 lines

[...]

drivers/char/tpm/eventlog/tpm2.c +1 -2

index e741b1157525..351a2989b3c6 100644
@@ -51,8 +51,7 @@ static void *tpm2_bios_measurements_start(struct seq_file *m, loff_t *pos)
 	int i;
 
 	event_header = addr;
-	size = sizeof(struct tcg_pcr_event) - sizeof(event_header->event)
-		+ event_header->event_size;
+	size = sizeof(*event_header) + event_header->event_size;

That said, I think it would be better to stick to the struct_size() idiom for dealing with flexible arrays here:

	size = struct_size(event_header, event, event_size);

The current codebase makes use of the zero-length array language extension to the C90 standard, but the preferred mechanism to declare variable-length types such as these ones is a flexible array member[1][2], introduced in C99:

struct foo {
        int stuff;
        struct boo array[];
};

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 inadvertently introduced[3] to the codebase from now on.

Also, notice that, dynamic memory allocations won't be affected by this change:

"Flexible array members have incomplete type, and so the sizeof operator may not be applied. As a quirk of the original implementation of zero-length arrays, sizeof evaluates to zero."[1]

sizeof(flexible-array-member) triggers a warning because flexible array members have incomplete type[1]. There are some instances of code in which the sizeof operator is being incorrectly/erroneously applied to zero-length arrays and the result is zero. Such instances may be hiding some bugs. So, this work (flexible-array member conversions) will also help to get completely rid of those sorts of issues.

Also, the following issue shows up due to the flexible-array member having incomplete type[4]:

drivers/char/tpm/eventlog/tpm2.c: In function ‘tpm2_bios_measurements_start’:
drivers/char/tpm/eventlog/tpm2.c:54:46: error: invalid application of ‘sizeof’ to incomplete type ‘u8[]’ {aka ‘unsigned char[]’}
54 | size = sizeof(struct tcg_pcr_event) - sizeof(event_header->event)
| ^
drivers/char/tpm/eventlog/tpm2.c: In function ‘tpm2_bios_measurements_next’:
drivers/char/tpm/eventlog/tpm2.c:102:10: error: invalid application of ‘sizeof’ to incomplete type ‘u8[]’ {aka ‘unsigned char[]’}
102 | sizeof(event_header->event) + event_header->event_size;
| ^
drivers/char/tpm/eventlog/tpm2.c: In function ‘tpm2_binary_bios_measurements_show’:
drivers/char/tpm/eventlog/tpm2.c:140:10: error: invalid application of ‘sizeof’ to incomplete type ‘u8[]’ {aka ‘unsigned char[]’}
140 | sizeof(event_header->event) + event_header->event_size;
| ^
scripts/Makefile.build:266: recipe for target 'drivers/char/tpm/eventlog/tpm2.o' failed
make[3]: *** [drivers/char/tpm/eventlog/tpm2.o] Error 1

As mentioned above: "Flexible array members have incomplete type, and so the sizeof operator may not be applied. As a quirk of the original implementation of zero-length arrays, sizeof evaluates to zero."[1] So, the sizeof(flexible-array) can be safely removed to fix the error above.

Lastly, prefer sizeof(*ptr) over sizeof(struct foo).

This issue was found with the help of Coccinelle.

[1] https://gcc.gnu.org/onlinedocs/gcc/Zero-Length.html
[2] https://github.com/KSPP/linux/issues/21
[3] commit 76497732932f ("cxgb3/l2t: Fix undefined behaviour")
[4] https://github.com/KSPP/linux/issues/43

Signed-off-by Gustavo A. R. Silva <[email protected]>

Quoted 7 lines

[1] https://gcc.gnu.org/onlinedocs/gcc/Zero-Length.html
[2] https://github.com/KSPP/linux/issues/21
[3] commit 76497732932f ("cxgb3/l2t: Fix undefined behaviour")
[4] https://github.com/KSPP/linux/issues/43

Signed-off-by Gustavo A. R. Silva <[email protected]>

Reviewed-by Jarkko Sakkinen <[email protected]>

Quoted 9 lines · Jarkko Sakkinen

Quoted 6 lines

[1] https://gcc.gnu.org/onlinedocs/gcc/Zero-Length.html
[2] https://github.com/KSPP/linux/issues/21
[3] commit 76497732932f ("cxgb3/l2t: Fix undefined behaviour")
[4] https://github.com/KSPP/linux/issues/43

Signed-off-by Gustavo A. R. Silva <[email protected]>

Reviewed-by Jarkko Sakkinen <[email protected]>

Hi Jarkko,

Thanks for your RB.

There is a v2 of this patch:

https://lore.kernel.org/lkml/20200508163826.GA768@embeddedor/

Thanks
--
Gustavo