[Libreoffice-commits] core.git: cppu/source external/gpgmepp
Stephan Bergmann (via logerrit)
logerrit at kemper.freedesktop.org
Tue Sep 3 13:54:58 UTC 2019
cppu/source/uno/copy.hxx | 26 ++++++++++++--
external/gpgmepp/UnpackedTarball_gpgmepp.mk | 1
external/gpgmepp/ubsan.patch | 52 ++++++++++++++++++++++++++++
3 files changed, 76 insertions(+), 3 deletions(-)
New commits:
commit cf2696efa0d0ea55c950d7b2898e7b7aab74812e
Author: Stephan Bergmann <sbergman at redhat.com>
AuthorDate: Tue Sep 3 11:26:44 2019 +0200
Commit: Stephan Bergmann <sbergman at redhat.com>
CommitDate: Tue Sep 3 15:54:08 2019 +0200
Silence -fsanitize=object-size in --enable-optimized builds
<http://clang.llvm.org/docs/UndefinedBehaviorSanitizer.html#available-checks>
(as of recent LLVM trunk towards LLVM 10) states:
"-fsanitize=object-size: An attempt to potentially use bytes which the optimizer
can determine are not part of the object being accessed. This will also detect
some types of undefined behavior that may not directly access memory, but are
provably incorrect given the size of the objects involved, such as invalid
downcasts and calling methods on invalid pointers. These checks are made in
terms of __builtin_object_size, and consequently may be able to detect more
problems at higher optimization levels."
A `make check screenshot` with --enabled-optimized runs into two such issues
that were not diagnosed with --disable-optimized, in both cases because a struct
with an "idiomatic trailing dynamic array member" (statically declared to be an
array of size 1) is allocated without any space for that array member, as the
dynamic array size is 0 in the specific case:
* For
> [CUT] sc_ucalc
> cppu/source/uno/copy.hxx:46:19: runtime error: member access within address 0x6020001aaa70 with insufficient space for an object of type 'uno_Sequence' (aka '_sal_Sequence')
> 0x6020001aaa70: note: pointer points here
> 2b 00 80 6a be be be be be be be be 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> ^
> #0 in cppu::allocSeq(int, int) at cppu/source/uno/copy.hxx:46:19 (instdir/program/libuno_cppu.so.3 +0x41f03f)
> #1 in uno_type_sequence_reference2One at cppu/source/uno/sequence.cxx:813:20 (instdir/program/libuno_cppu.so.3 +0x41f03f)
[...]
the call to
pNew = allocSeq( 0, 0 );
in uno_type_sequence_reference2One is inlined, so
sal_uInt32 nSize = calcSeqMemSize( nElementSize, nElements );
in allocSeq is known to be too small.
* For
> testSignatureLineODF::TestBody finished in: 2044ms
> engine-gpg.c:302:6: runtime error: member access within address 0x604001f24f10 with insufficient space for an object of type 'struct arg_and_data_s'
> 0x604001f24f10: note: pointer points here
> 6e 01 00 26 be be be be be be be be be be be be be be be be be be be be be be be be be be be be
> ^
> #0 in add_data at workdir/UnpackedTarball/gpgmepp/src/engine-gpg.c:302:6 (instdir/program/libgpgme.so.11 +0x120c2b)
[...]
> make[1]: *** [solenv/gbuild/CppunitTest.mk:114: workdir/CppunitTest/xmlsecurity_signing.test] Error 1
the
a = malloc (sizeof *a - 1);
is apparently detected to be too small only with optimization enabled.
In both cases, the solution is to operate on the too-small dynamically allocated
object via a reinterpret_cast'ed pointer to some newly introduced "struct
prefix" type.
Change-Id: Ie814db1d00a439bb9189474b91d729e538e81984
Reviewed-on: https://gerrit.libreoffice.org/78548
Tested-by: Jenkins
Reviewed-by: Stephan Bergmann <sbergman at redhat.com>
diff --git a/cppu/source/uno/copy.hxx b/cppu/source/uno/copy.hxx
index b0811b5fab10..b24e0e337bf5 100644
--- a/cppu/source/uno/copy.hxx
+++ b/cppu/source/uno/copy.hxx
@@ -22,7 +22,9 @@
#include "prim.hxx"
#include "constr.hxx"
#include <cassert>
+#include <cstddef>
#include <cstdlib>
+#include <type_traits>
namespace cppu
{
@@ -30,6 +32,22 @@ namespace cppu
//#### copy construction ###########################################################################
+namespace {
+
+// The non-dynamic prefix of sal_Sequence (aka uno_Sequence):
+struct SequencePrefix {
+ sal_Int32 nRefCount;
+ sal_Int32 nElements;
+};
+static_assert(sizeof (SequencePrefix) < sizeof (uno_Sequence));
+static_assert(offsetof(SequencePrefix, nRefCount) == offsetof(uno_Sequence, nRefCount));
+static_assert(
+ std::is_same_v<decltype(SequencePrefix::nRefCount), decltype(uno_Sequence::nRefCount)>);
+static_assert(offsetof(SequencePrefix, nElements) == offsetof(uno_Sequence, nElements));
+static_assert(
+ std::is_same_v<decltype(SequencePrefix::nElements), decltype(uno_Sequence::nElements)>);
+
+}
inline uno_Sequence * allocSeq(
sal_Int32 nElementSize, sal_Int32 nElements )
@@ -42,9 +60,11 @@ inline uno_Sequence * allocSeq(
pSeq = static_cast<uno_Sequence *>(std::malloc( nSize ));
if (pSeq != nullptr)
{
- // header init
- pSeq->nRefCount = 1;
- pSeq->nElements = nElements;
+ // header init, going via SequencePrefix to avoid UBSan insufficient-object-size
+ // warnings when `nElements == 0` and thus `nSize < sizeof (uno_Sequence)`:
+ auto const header = reinterpret_cast<SequencePrefix *>(pSeq);
+ header->nRefCount = 1;
+ header->nElements = nElements;
}
}
return pSeq;
diff --git a/external/gpgmepp/UnpackedTarball_gpgmepp.mk b/external/gpgmepp/UnpackedTarball_gpgmepp.mk
index 9fc213c7e51c..1b0468fc6f23 100644
--- a/external/gpgmepp/UnpackedTarball_gpgmepp.mk
+++ b/external/gpgmepp/UnpackedTarball_gpgmepp.mk
@@ -32,5 +32,6 @@ $(eval $(call gb_UnpackedTarball_add_patches,gpgmepp, \
$(if $(filter LINUX,$(OS)),external/gpgmepp/rpath.patch) \
external/gpgmepp/gcc9.patch \
external/gpgmepp/version.patch \
+ external/gpgmepp/ubsan.patch \
))
# vim: set noet sw=4 ts=4:
diff --git a/external/gpgmepp/ubsan.patch b/external/gpgmepp/ubsan.patch
new file mode 100644
index 000000000000..5a6e6dcdc9b2
--- /dev/null
+++ b/external/gpgmepp/ubsan.patch
@@ -0,0 +1,52 @@
+--- src/engine-gpg.c
++++ src/engine-gpg.c
+@@ -60,6 +60,15 @@
+ building command line to this location. */
+ char arg[1]; /* Used if data above is not used. */
+ };
++struct arg_without_data_s
++{
++ struct arg_and_data_s *next;
++ gpgme_data_t data;
++ int inbound;
++ int dup_to;
++ int print_fd;
++ int *arg_locp;
++};
+
+
+ struct fd_data_map_s
+@@ -299,23 +308,24 @@
+ a = malloc (sizeof *a - 1);
+ if (!a)
+ return gpg_error_from_syserror ();
+- a->next = NULL;
+- a->data = data;
+- a->inbound = inbound;
+- a->arg_locp = NULL;
++ struct arg_without_data_s *a2 = (struct arg_without_data_s *)a;
++ a2->next = NULL;
++ a2->data = data;
++ a2->inbound = inbound;
++ a2->arg_locp = NULL;
+
+ if (dup_to == -2)
+ {
+- a->print_fd = 1;
+- a->dup_to = -1;
++ a2->print_fd = 1;
++ a2->dup_to = -1;
+ }
+ else
+ {
+- a->print_fd = 0;
+- a->dup_to = dup_to;
++ a2->print_fd = 0;
++ a2->dup_to = dup_to;
+ }
+ *gpg->argtail = a;
+- gpg->argtail = &a->next;
++ gpg->argtail = &a2->next;
+ return 0;
+ }
+
More information about the Libreoffice-commits
mailing list