From 4fbb7ccebe1fdcbae762e8fed6af7a810c81f85c Mon Sep 17 00:00:00 2001 From: Joel Brobecker Date: Tue, 24 Nov 2020 06:34:57 +0400 Subject: [PATCH] Fix stack smashing error during gdb_mpq_write_fixed_point selftest When building GDB using Ubuntu 20.04's system libgmp and compiler, running the "maintenance selftest" command triggers the following error: | Running selftest gdb_mpq_write_fixed_point. | *** stack smashing detected ***: terminated | [1] 1092790 abort (core dumped) ./gdb gdb This happens while trying to construct an mpq_t object (a rational) from two integers representing the numerator and denominator. In our test, the numerator is -8, and the denominator is 1. The problem was that the rational was constructed using the wrong function. This is what we were doing prior to this patch: mpq_set_ui (v.val, numerator, denominator); The 'u' in "ui" stands for *unsigned*, which is wrong because numerator and denominator's type is "int". As a result of the above, instead of getting a rational value of -8, we get a rational with a very large positive value (gmp_printf says "18446744073709551608"). From there, the test performs an operation which is expected to write this value into a buffer which was not dimensioned to fit such a number, thus leading GMP into a buffer overflow. This was verified by applying the formula that GMP's documentation gives for the required memory buffer size needed during export: | When an application is allocating space itself the required size can | be determined with a calculation like the following. Since | mpz_sizeinbase always returns at least 1, count here will be at | least one, which avoids any portability problems with malloc(0), | though if z is zero no space at all is actually needed (or written). | | numb = 8*size - nail; | count = (mpz_sizeinbase (z, 2) + numb-1) / numb; | p = malloc (count * size); With the very large number, mpz_sizeinbase returns 66 and thus the malloc size becomes 16 bytes instead of the 8 we allocated. This patch fixes the issue by using the correct "set" function. gdb/ChangeLog: * unittests/gmp-utils-selftests.c (write_fp_test): Use mpq_set_si instead of mpq_set_ui to initialize our GMP rational. --- gdb/ChangeLog | 5 +++++ gdb/unittests/gmp-utils-selftests.c | 2 +- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/gdb/ChangeLog b/gdb/ChangeLog index 47c68cbde03..bd74aa55e5b 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -1,3 +1,8 @@ +2020-11-24 Joel Brobecker + + * unittests/gmp-utils-selftests.c (write_fp_test): Use mpq_set_si + instead of mpq_set_ui to initialize our GMP rational. + 2020-11-23 Tom de Vries * debuginfod-support.c (debuginfod_source_query) diff --git a/gdb/unittests/gmp-utils-selftests.c b/gdb/unittests/gmp-utils-selftests.c index af5bc65d2f9..175ab3f9827 100644 --- a/gdb/unittests/gmp-utils-selftests.c +++ b/gdb/unittests/gmp-utils-selftests.c @@ -400,7 +400,7 @@ write_fp_test (int numerator, unsigned int denominator, memset (buf, 0, len); gdb_mpq v; - mpq_set_ui (v.val, numerator, denominator); + mpq_set_si (v.val, numerator, denominator); mpq_canonicalize (v.val); v.write_fixed_point (buf, len, byte_order, 0, scaling_factor); -- 2.30.2