linux-kernel - [PATCH] [RFC v2] qnx: avoid -Wstringop-overread warning, again

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-Id: <20210920121208.54732-1-arnd@kernel.org>
Date:   Mon, 20 Sep 2021 14:12:02 +0200
From:   Arnd Bergmann <arnd@...nel.org>
To:     Anders Larsen <al@...rsen.net>
Cc:     Linus Torvalds <torvalds@...ux-foundation.org>,
        Arnd Bergmann <arnd@...db.de>, linux-kernel@...r.kernel.org
Subject: [PATCH] [RFC v2] qnx: avoid -Wstringop-overread warning, again

From: Arnd Bergmann <arnd@...db.de>

There is still a gcc-11 warning about stringop-overread in some
configurations, despite the earlier fix that was meant to address
this issue:

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;
      |                      ^~~~~~~

The problem here is that we access the same pointer using two different
structure layouts, but gcc determines the object size based on
whatever it encounters first, in this case, the single-byte field.

Rework the union once more, to have a flexible-array member as the
thing that gets accessed first to deal with current gcc versions, but
leave everything else in place so that a future fixed compiler will
correctly see the struct member sizes.

Unfortunately, this makes the code really ugly again. In my original
patch, I just changed the if (!de->de_name) check to access the longer
of the two fields, which worked. Another option would be to make
de_name a longer array.

Fixes: b7213ffa0e58 ("qnx4: avoid stringop-overread errors")
Link: https://lore.kernel.org/all/20210322160253.4032422-6-arnd@kernel.org/
Link: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99578
Signed-off-by: Arnd Bergmann <arnd@...db.de>
---
v2: rebase on top of the earlier patch.
---
 fs/qnx4/dir.c | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/fs/qnx4/dir.c b/fs/qnx4/dir.c
index 2a66844b7ff8..d7eb5a9b79e4 100644
--- a/fs/qnx4/dir.c
+++ b/fs/qnx4/dir.c
@@ -23,8 +23,16 @@
  */
 union qnx4_directory_entry {
 	struct {
-		char de_name;
-		char de_pad[62];
+		/*
+		 * work around gcc-11.x using the first field it observes
+		 * to determing the actual length
+		 * https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99578
+		 */
+		char __empty[0];
+		char de_name[];
+	};
+	struct {
+		char de_pad[63];
 		char de_status;
 	};
 	struct qnx4_inode_entry inode;
@@ -58,7 +66,7 @@ static int qnx4_readdir(struct file *file, struct dir_context *ctx)
 			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;
-- 
2.29.2