History

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

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] As in "sizeof(event_header->event) always evaluated to 0, so removing it has no effect". Lastly, make use of the struct_size() helper to deal with the flexible array member and its host structure. 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

Gustavo A. R. Silva committed 6 years ago

Commit ab91c2a1 parent 8c872862 files · +6 −8Browse files

Unified · Split

2 files
  1. drivers/char/tpm/eventlog/tpm2.c +5 −7
  2. include/linux/tpm_eventlog.h +1 −1
drivers/char/tpm/eventlog/tpm2.c+5−7
@@ -51,8 +51,7 @@ static void *tpm2_bios_measurements_start(struct seq_file *m, loff_t *pos) · 50 unmodified lines
5151 int i;
5252
5353 event_header = addr;
54− size = sizeof(struct tcg_pcr_event) - sizeof(event_header->event)
55− + event_header->event_size;
54+ size = struct_size(event_header, event, event_header->event_size);
5655
5756 if (*pos == 0) {
5857 if (addr + size < limit) {
@@ -98,8 +97,8 @@ static void *tpm2_bios_measurements_next(struct seq_file *m, void *v, · 39 unmodified lines
9897 event_header = log->bios_event_log;
9998
10099 if (v == SEQ_START_TOKEN) {
101− event_size = sizeof(struct tcg_pcr_event) -
102− sizeof(event_header->event) + event_header->event_size;
100+ event_size = struct_size(event_header, event,
101+ event_header->event_size);
103102 marker = event_header;
104103 } else {
105104 event = v;
@@ -136,9 +135,8 @@ static int tpm2_binary_bios_measurements_show(struct seq_file *m, void *v) · 30 unmodified lines
136135 size_t size;
137136
138137 if (v == SEQ_START_TOKEN) {
139− size = sizeof(struct tcg_pcr_event) -
140− sizeof(event_header->event) + event_header->event_size;
141−
138+ size = struct_size(event_header, event,
139+ event_header->event_size);
142140 temp_ptr = event_header;
143141
144142 if (size > 0)
include/linux/tpm_eventlog.h+1−1
@@ -97,7 +97,7 @@ struct tcg_pcr_event { · 96 unmodified lines
9797 u32 event_type;
9898 u8 digest[20];
9999 u32 event_size;
100− u8 event[0];
100+ u8 event[];
101101 } __packed;
102102
103103 struct tcg_event_field {