From: Greg Kroah-Hartman Date: Mon, 27 Sep 2021 13:20:22 +0000 (+0200) Subject: 4.4-stable patches X-Git-Tag: v5.4.150~11 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=3698c4d68c5e9be393ef6994377e0c65ba1e47ea;p=thirdparty%2Fkernel%2Fstable-queue.git 4.4-stable patches added patches: qnx4-work-around-gcc-false-positive-warning-bug.patch --- diff --git a/queue-4.4/qnx4-work-around-gcc-false-positive-warning-bug.patch b/queue-4.4/qnx4-work-around-gcc-false-positive-warning-bug.patch new file mode 100644 index 00000000000..d50566baf0d --- /dev/null +++ b/queue-4.4/qnx4-work-around-gcc-false-positive-warning-bug.patch @@ -0,0 +1,120 @@ +From d5f6545934c47e97c0b48a645418e877b452a992 Mon Sep 17 00:00:00 2001 +From: Linus Torvalds +Date: Mon, 20 Sep 2021 10:26:21 -0700 +Subject: qnx4: work around gcc false positive warning bug + +From: Linus Torvalds + +commit d5f6545934c47e97c0b48a645418e877b452a992 upstream. + +In commit b7213ffa0e58 ("qnx4: avoid stringop-overread errors") I tried +to teach gcc about how the directory entry structure can be two +different things depending on a status flag. It made the code clearer, +and it seemed to make gcc happy. + +However, Arnd points to a gcc bug, where despite using two different +members of a union, gcc then gets confused, and uses the size of one of +the members to decide if a string overrun happens. And not necessarily +the rigth one. + +End result: with some configurations, gcc-11 will still complain about +the source buffer size being overread: + + fs/qnx4/dir.c: In function 'qnx4_readdir': + fs/qnx4/dir.c:76:32: error: 'strnlen' specified bound [16, 48] exceeds source size 1 [-Werror=stringop-overread] + 76 | size = strnlen(name, size); + | ^~~~~~~~~~~~~~~~~~~ + fs/qnx4/dir.c:26:22: note: source object declared here + 26 | char de_name; + | ^~~~~~~ + +because gcc will get confused about which union member entry is actually +getting accessed, even when the source code is very clear about it. Gcc +internally will have combined two "redundant" pointers (pointing to +different union elements that are at the same offset), and takes the +size checking from one or the other - not necessarily the right one. + +This is clearly a gcc bug, but we can work around it fairly easily. The +biggest thing here is the big honking comment about why we do what we +do. + +Link: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99578#c6 +Reported-and-tested-by: Arnd Bergmann +Signed-off-by: Linus Torvalds +Signed-off-by: Greg Kroah-Hartman +--- + fs/qnx4/dir.c | 36 +++++++++++++++++++++++++++--------- + 1 file changed, 27 insertions(+), 9 deletions(-) + +--- a/fs/qnx4/dir.c ++++ b/fs/qnx4/dir.c +@@ -19,12 +19,33 @@ + * depending on the status field in the last byte. The + * first byte is where the name start either way, and a + * zero means it's empty. ++ * ++ * Also, due to a bug in gcc, we don't want to use the ++ * real (differently sized) name arrays in the inode and ++ * link entries, but always the 'de_name[]' one in the ++ * fake struct entry. ++ * ++ * See ++ * ++ * https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99578#c6 ++ * ++ * for details, but basically gcc will take the size of the ++ * 'name' array from one of the used union entries randomly. ++ * ++ * This use of 'de_name[]' (48 bytes) avoids the false positive ++ * warnings that would happen if gcc decides to use 'inode.di_name' ++ * (16 bytes) even when the pointer and size were to come from ++ * 'link.dl_name' (48 bytes). ++ * ++ * In all cases the actual name pointer itself is the same, it's ++ * only the gcc internal 'what is the size of this field' logic ++ * that can get confused. + */ + union qnx4_directory_entry { + struct { +- char de_name; +- char de_pad[62]; +- char de_status; ++ const char de_name[48]; ++ u8 de_pad[15]; ++ u8 de_status; + }; + struct qnx4_inode_entry inode; + struct qnx4_link_info link; +@@ -52,29 +73,26 @@ static int qnx4_readdir(struct file *fil + ix = (ctx->pos >> QNX4_DIR_ENTRY_SIZE_BITS) % QNX4_INODES_PER_BLOCK; + for (; ix < QNX4_INODES_PER_BLOCK; ix++, ctx->pos += QNX4_DIR_ENTRY_SIZE) { + union qnx4_directory_entry *de; +- const char *name; + + offset = ix * QNX4_DIR_ENTRY_SIZE; + de = (union qnx4_directory_entry *) (bh->b_data + offset); + +- if (!de->de_name) ++ if (!de->de_name[0]) + continue; + if (!(de->de_status & (QNX4_FILE_USED|QNX4_FILE_LINK))) + continue; + if (!(de->de_status & QNX4_FILE_LINK)) { + size = sizeof(de->inode.di_fname); +- name = de->inode.di_fname; + ino = blknum * QNX4_INODES_PER_BLOCK + ix - 1; + } else { + size = sizeof(de->link.dl_fname); +- name = de->link.dl_fname; + ino = ( le32_to_cpu(de->link.dl_inode_blk) - 1 ) * + QNX4_INODES_PER_BLOCK + + de->link.dl_inode_ndx; + } +- size = strnlen(name, size); ++ size = strnlen(de->de_name, size); + QNX4DEBUG((KERN_INFO "qnx4_readdir:%.*s\n", size, name)); +- if (!dir_emit(ctx, name, size, ino, DT_UNKNOWN)) { ++ if (!dir_emit(ctx, de->de_name, size, ino, DT_UNKNOWN)) { + brelse(bh); + return 0; + } diff --git a/queue-4.4/series b/queue-4.4/series index 63ff76bb29b..ae715fe23b7 100644 --- a/queue-4.4/series +++ b/queue-4.4/series @@ -21,3 +21,4 @@ arm64-mark-__stack_chk_guard-as-__ro_after_init.patch alpha-declare-virt_to_phys-and-virt_to_bus-parameter.patch net-6pack-fix-tx-timeout-and-slot-time.patch spi-fix-tegra20-build-with-config_pm-n.patch +qnx4-work-around-gcc-false-positive-warning-bug.patch