Bug#670963: lintian: Warn (or at least inform) of non-LFS-enabled binaries
Hi!
On Sat, 2013-01-19 at 00:26:43 +0100, Niels Thykier wrote:
> #lintian (2.5.12) UNRELEASED; urgency=low
> #
> # * checks/binaries{,.desc}:
> # + [NT] Check for binaries built without LFS. This can
> # only be checked for 32bit binaries as 64bit binaries
> # have LFS by definition. Thanks to Guillem Jover for
> # the report. (Closes: #670963)
> #
Cool, thanks! Here's a small patch series hopefully improving the
check a bit. It does the following:
* Adds the non-LFS-safe fts(3) family of funtions, these do not have
an LFS counterpart.
* Expands the tag description to describe a bit more how to add LFS,
and possible problems when enabling it on a shared library.
* Handles partially enabled LFS binaries, so that if an object which
is part of the binary has been correctly compiled with LFS but not
another part it will trigger. Add a regression test for this.
Also something to note is that some of the functions will most
probably not match as they get renamed to something else, for example
stat(2) will end up as __xstat on the binary's symbol table. There's
probably others like that.
Of course feel free to squash them into a single patch if that's your
preferred form.
Thanks,
Guillem
From 4ffeb3d1d4cd45a6e6c13c64596769c28440f77e Mon Sep 17 00:00:00 2001
From: Guillem Jover <guillem@debian.org>
Date: Wed, 23 Jan 2013 02:22:08 +0100
Subject: [PATCH 1/3] Add non-LFS-safe fts(3) family of functions
Signed-off-by: Guillem Jover <guillem@debian.org>
---
data/binaries/lfs-symbols | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/data/binaries/lfs-symbols b/data/binaries/lfs-symbols
index b39cf61..b593115 100644
--- a/data/binaries/lfs-symbols
+++ b/data/binaries/lfs-symbols
@@ -27,6 +27,11 @@ fstatfs
fstatvfs
ftello
ftruncate
+fts_open
+fts_read
+fts_children
+fts_set
+fts_close
ftw
getdirentries
getrlimit
--
1.8.1.1
From a90b47a49caa67d166c01394f4a4396c3fc56b32 Mon Sep 17 00:00:00 2001
From: Guillem Jover <guillem@debian.org>
Date: Wed, 23 Jan 2013 02:24:22 +0100
Subject: [PATCH 2/3] c/binaries.desc: Expands tag description to explain how
to add LFS
Signed-off-by: Guillem Jover <guillem@debian.org>
---
checks/binaries.desc | 15 +++++++++++----
1 file changed, 11 insertions(+), 4 deletions(-)
diff --git a/checks/binaries.desc b/checks/binaries.desc
index d24e045..9f0f24a 100644
--- a/checks/binaries.desc
+++ b/checks/binaries.desc
@@ -352,7 +352,14 @@ Info: The listed ELF binary appears to be built without "Large File
Support" (LFS). If so, it may not be able to handle large files
correctly.
.
- To support large files, ensure that <tt>_FILE_OFFSET_BITS</tt> is
- defined and set to 64 before the relevant files are included. This
- can be done by using the <tt>AC_SYS_LARGEFILE</tt> macro with
- autoconf.
+ To support large files, code review might be needed to make sure that
+ those files are not slurped into memory or mmap(2)ed, and that correct
+ 64-bit data types are used (ex: off_t instead of ssize_t), etc. Once
+ that has been done ensure <tt>_FILE_OFFSET_BITS</tt> is defined and
+ set to 64 before the relevant files are included. This can be done by
+ using the <tt>AC_SYS_LARGEFILE</tt> macro with autoconf. Take into
+ account that even if this tag is not emitted, that does not mean the
+ binary is LFS-safe (ie. no OOM conditions, file truncation or overwrite
+ will happen). Also note that enabling LFS on a shared library is not
+ always safe as it might break ABI in case some of the exported types
+ change size, in those cases a SOVERSION bump might be required.
--
1.8.1.1
From dea6f5e01875f127b0639dbb3801e57f50bf456c Mon Sep 17 00:00:00 2001
From: Guillem Jover <guillem@debian.org>
Date: Wed, 23 Jan 2013 02:27:01 +0100
Subject: [PATCH 3/3] c/binaries{,.desc}: Handle partially LFS-enabled binaries
If an object which is part of the binary has been correctly built with
LFS but not another the check should trigger, as it means at least parts
of the binary will be unable to correctly handle large files. Add a
regression test for the partially shared library.
Signed-off-by: Guillem Jover <guillem@debian.org>
---
checks/binaries | 12 +++---------
checks/binaries.desc | 6 +++---
t/tests/binaries-missing-lfs/debian/Makefile | 2 +-
.../binaries-missing-lfs/debian/debian/libbasic2.symbols | 1 +
t/tests/binaries-missing-lfs/debian/lfs.c | 13 +++++++++++++
5 files changed, 21 insertions(+), 13 deletions(-)
create mode 100644 t/tests/binaries-missing-lfs/debian/lfs.c
diff --git a/checks/binaries b/checks/binaries
index a390298..e2a2bc4 100644
--- a/checks/binaries
+++ b/checks/binaries
@@ -128,17 +128,11 @@ foreach my $file (sort keys %{$info->objdump_info}) {
foreach my $symbol (@{$objdump->{SYMBOLS}}) {
my ($foo, $sec, $sym) = @{$symbol};
- unless ($has_lfs) {
+ unless (defined $has_lfs) {
if ($LFS_SYMBOLS->known ($sym)) {
- # Using a 32bit only interface call, assume it is
- # built without LFS (for now). We may disapprove this
- # later if a 64 variant comes along.
+ # Using a 32bit only interface call, some parts of the
+ # binary are built without LFS.
$has_lfs = 0;
- } elsif ($sym =~ m/^(.+)64$/) {
- # all LFS symbols appear to be end with 64; exploit
- # that to check if LFS is enabled.
- my $base = $1;
- $has_lfs = 1 if $LFS_SYMBOLS->known ($base);
}
}
if ($arch ne 'hppa') {
diff --git a/checks/binaries.desc b/checks/binaries.desc
index 9f0f24a..3f59a25 100644
--- a/checks/binaries.desc
+++ b/checks/binaries.desc
@@ -348,9 +348,9 @@ Tag: binary-file-built-without-LFS-support
Severity: minor
Certainty: possible
Experimental: yes
-Info: The listed ELF binary appears to be built without "Large File
- Support" (LFS). If so, it may not be able to handle large files
- correctly.
+Info: The listed ELF binary appears to be (partially) built without
+ "Large File Support" (LFS). If so, it may not be able to handle large
+ files correctly.
.
To support large files, code review might be needed to make sure that
those files are not slurped into memory or mmap(2)ed, and that correct
diff --git a/t/tests/binaries-missing-lfs/debian/Makefile b/t/tests/binaries-missing-lfs/debian/Makefile
index 6fc3968..637a9ca 100644
--- a/t/tests/binaries-missing-lfs/debian/Makefile
+++ b/t/tests/binaries-missing-lfs/debian/Makefile
@@ -1,5 +1,5 @@
all:
- gcc $(CFLAGS) $(CPPFLAGS) $(LDFLAGS) -fPIC -shared -Wl,-z,defs -Wl,-soname,libbasic.so.2 -o libbasic.so.2 basic.c
+ gcc $(CFLAGS) $(CPPFLAGS) $(LDFLAGS) -fPIC -shared -Wl,-z,defs -Wl,-soname,libbasic.so.2 -o libbasic.so.2 lfs.c basic.c
install:
# install it under the correct triplet directory
diff --git a/t/tests/binaries-missing-lfs/debian/debian/libbasic2.symbols b/t/tests/binaries-missing-lfs/debian/debian/libbasic2.symbols
index b072f19..c67f613 100644
--- a/t/tests/binaries-missing-lfs/debian/debian/libbasic2.symbols
+++ b/t/tests/binaries-missing-lfs/debian/debian/libbasic2.symbols
@@ -1,3 +1,4 @@
libbasic.so.2 libbasic2 #MINVER#
do_open@Base 1.0
lib_interface@Base 1.0
+ zz_open@Base 1.0
diff --git a/t/tests/binaries-missing-lfs/debian/lfs.c b/t/tests/binaries-missing-lfs/debian/lfs.c
new file mode 100644
index 0000000..8289c82
--- /dev/null
+++ b/t/tests/binaries-missing-lfs/debian/lfs.c
@@ -0,0 +1,13 @@
+#define _FILE_OFFSET_BITS 64
+
+#include <sys/types.h>
+#include <sys/stat.h>
+#include <fcntl.h>
+
+/* Name function so that it comes last in the symbol table, to make
+ * sure the presence of an LFS function does not mark the whole binary
+ * as LFS-enabled. */
+int
+zz_open (char *file) {
+ return open (file, O_RDONLY);
+}
--
1.8.1.1
Reply to: