linux-kernel - [PATCH v2] perf tools: fix type mismatch

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]

Date:	Tue, 16 Sep 2014 19:16:29 +0400
From:	Alexey Brodkin <Alexey.Brodkin@...opsys.com>
To:	acme@...hat.com
Cc:	linux-kernel@...r.kernel.org,
	Alexey Brodkin <Alexey.Brodkin@...opsys.com>,
	Vineet Gupta <Vineet.Gupta1@...opsys.com>,
	Borislav Petkov <bp@...e.de>, Jiri Olsa <jolsa@...nel.org>,
	Cody P Schafer <dev@...yps.com>
Subject: [PATCH v2] perf tools: fix type mismatch - long vs __statfs_word

In case of compilation with "-Wextra" following breakage happens:
--->---
fs.c: In function ‘fs__valid_mount’:
fs.c:76:24: error: comparison between signed and unsigned integer expressions [-Werror=sign-compare]
  else if (st_fs.f_type != magic)
                        ^
cc1: all warnings being treated as errors
--->---

Note that now when fs.c is in "lib/api/fs" and in "tools/lib/api/Makefile"
CFLAGS has hard-coded "-Wextra" this is inevitable even if one builds "perf"
with "WERROR=0".

Even though "debugfs.c" gets compiled successfully because DEBUGFS_MAGIC is
definitely positive (MSB bit is not set, so compiler doesn't care about
explisit cast to "long") it's good to cast "st_fs.f_type" to "long" as well.

And here's an explanation of observed issue.

Perf tools use libc headers instead of ones from kernel source tree.
This is because "perf" is just a user-space tool even thought its sources are
merged in Linux kernel source tree.
And so "statfs.h" comes from either uClibc, glibc or musl.

In case of uClibc most of architectures use "legacy" version from here:
http://git.uclibc.org/uClibc/tree/libc/sysdeps/linux/common/bits/statfs.h

New UAPI compliant architectures (looks like for now it's only ARC) uses UAPI
version of "statfs.h" from here:
http://git.uclibc.org/uClibc/tree/libc/sysdeps/linux/common-generic/bits/statfs.h

In this particular situation most interesting difference is in data type used
for "struct statfs" member "f_type".

In "legacy" version it's:
--->---
__SWORD_TYPE f_type;
--->---

In UAPI version it's:
--->---
__U32_TYPE f_type;
--->---

In its turn mentioned types are defined in
http://git.uclibc.org/uClibc/tree/libc/sysdeps/linux/common/bits/types.h
this way:
--->---
...
--->---

And for 32-bit architectures "int" is of the same length as "long" so compiler
is happy on data type check.

But if "f_type" is "unsigned int" it's definitely not an unsigned "long".

Essential fix is explicit casting of "f_type" to "long".

Signed-off-by: Alexey Brodkin <abrodkin@...opsys.com>

Cc: Vineet Gupta <vgupta@...opsys.com>
Cc: Borislav Petkov <bp@...e.de>
Cc: Jiri Olsa <jolsa@...nel.org>
Cc: Cody P Schafer <dev@...yps.com>
Cc: Arnaldo Carvalho de Melo <acme@...hat.com>
---

Changes in v2:

 * Added type cast to DEBUGFS_MAGIC in "debugfs.c"
 * Added verbose explanation of root cause

Thanks a lot to Arnaldo for mention of another instance of hidden missing
cast in "debugfs.c".

---
 tools/lib/api/fs/debugfs.c | 2 +-
 tools/lib/api/fs/fs.c      | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/lib/api/fs/debugfs.c b/tools/lib/api/fs/debugfs.c
index a74fba6..93aa4cd 100644
--- a/tools/lib/api/fs/debugfs.c
+++ b/tools/lib/api/fs/debugfs.c
@@ -67,7 +67,7 @@ int debugfs_valid_mountpoint(const char *debugfs)
 
 	if (statfs(debugfs, &st_fs) < 0)
 		return -ENOENT;
-	else if (st_fs.f_type != (long) DEBUGFS_MAGIC)
+	else if ((long) st_fs.f_type != (long) DEBUGFS_MAGIC)
 		return -ENOENT;
 
 	return 0;
diff --git a/tools/lib/api/fs/fs.c b/tools/lib/api/fs/fs.c
index c1b49c3..a9fd78b 100644
--- a/tools/lib/api/fs/fs.c
+++ b/tools/lib/api/fs/fs.c
@@ -75,7 +75,7 @@ static int fs__valid_mount(const char *fs, long magic)
 
 	if (statfs(fs, &st_fs) < 0)
 		return -ENOENT;
-	else if (st_fs.f_type != magic)
+	else if ((long) st_fs.f_type != magic)
 		return -ENOENT;
 
 	return 0;
-- 
1.9.3

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/