Fix qsort workaround (#2248502)

This commit is contained in:
Florian Weimer 2023-11-27 12:54:10 +01:00
parent 154785c591
commit 6f9b4d9d90
2 changed files with 91 additions and 22 deletions

View File

@ -1,36 +1,102 @@
commit d0987c7014d33e96a7a0d170fea8bcc97163cead
Author: Florian Weimer <fweimer@redhat.com> Author: Florian Weimer <fweimer@redhat.com>
Date: Thu Nov 23 08:34:30 2023 +0100 Date: Thu Nov 23 08:34:30 2023 +0100
stdlib: Add another workaround to the insertion sort phase of qsort stdlib: Fix array bounds protection in insertion sort phase of qsort
If the comparison function returns negative values incorrectly, it was The previous check did not do anything because tmp_ptr already
possible that we decrement tmp_ptr past the start of the array. points before run_ptr due to the way it is initialized.
Improves commit e4d8117b82065dc72e8df80097360e7c05a349b9 ("stdlib: Fixes commit e4d8117b82065dc72e8df80097360e7c05a349b9
Avoid another self-comparison in qsort"). ("stdlib: Avoid another self-comparison in qsort").
diff --git a/stdlib/Makefile b/stdlib/Makefile
index 6194d1cb22cee59c..0b154e57c586f65b 100644
--- a/stdlib/Makefile
+++ b/stdlib/Makefile
@@ -216,6 +216,7 @@ tests := \
tst-qsort2 \
tst-qsort3 \
tst-qsort5 \
+ tst-qsort6 \
tst-quick_exit \
tst-rand48 \
tst-rand48-2 \
diff --git a/stdlib/qsort.c b/stdlib/qsort.c diff --git a/stdlib/qsort.c b/stdlib/qsort.c
index be01fb5598de2257..6f28abbc7f9719fb 100644 index be01fb5598de2257..62477010b6270246 100644
--- a/stdlib/qsort.c --- a/stdlib/qsort.c
+++ b/stdlib/qsort.c +++ b/stdlib/qsort.c
@@ -238,8 +238,17 @@ insertion_sort_qsort_partitions (void *const pbase, size_t total_elems, @@ -238,7 +238,7 @@ insertion_sort_qsort_partitions (void *const pbase, size_t total_elems,
while ((run_ptr += size) <= end_ptr) while ((run_ptr += size) <= end_ptr)
{ {
tmp_ptr = run_ptr - size; tmp_ptr = run_ptr - size;
- while (run_ptr != tmp_ptr && cmp (run_ptr, tmp_ptr, arg) < 0) - while (run_ptr != tmp_ptr && cmp (run_ptr, tmp_ptr, arg) < 0)
- tmp_ptr -= size; + while (tmp_ptr != base_ptr && cmp (run_ptr, tmp_ptr, arg) < 0)
+ /* The initial pointer comparison avoids a call to cmp if the tmp_ptr -= size;
+ pointer arguments are identical (the call returns zero with a
+ correctly implemented comparison function). The final
+ pointer comparison cannot be reached because the element at
+ base_ptr is the smallest element, but it prevents the loop
+ from running beyond the start of the array with a broken
+ comparison function. */
+ while (run_ptr != tmp_ptr
+ && cmp (run_ptr, tmp_ptr, arg) < 0
+ && run_ptr != base_ptr)
+ tmp_ptr -= size;
tmp_ptr += size; tmp_ptr += size;
if (tmp_ptr != run_ptr) diff --git a/stdlib/tst-qsort6.c b/stdlib/tst-qsort6.c
new file mode 100644
index 0000000000000000..8ec0a6b633bc8398
--- /dev/null
+++ b/stdlib/tst-qsort6.c
@@ -0,0 +1,60 @@
+/* Test qsort with invalid comparison functions.
+ Copyright (C) 2023 Free Software Foundation, Inc.
+ This file is part of the GNU C Library.
+
+ The GNU C Library is free software; you can redistribute it and/or
+ modify it under the terms of the GNU Lesser General Public
+ License as published by the Free Software Foundation; either
+ version 2.1 of the License, or (at your option) any later version.
+
+ The GNU C Library is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ Lesser General Public License for more details.
+
+ You should have received a copy of the GNU Lesser General Public
+ License along with the GNU C Library; if not, see
+ <http://www.gnu.org/licenses/>. */
+
+#include <array_length.h>
+#include <stdlib.h>
+#include <support/check.h>
+
+/* Invalid comparison function that always returns -1. */
+static int
+invalid_compare_1 (const void *a1, const void *b1)
+{
+ const int *a = a1;
+ const int *b = b1;
+ /* Check that the marker value matches, which means that we are
+ likely within the array. */
+ TEST_COMPARE (*a, 842523635);
+ TEST_COMPARE (*b, 842523635);
+ TEST_VERIFY_EXIT (*a == 842523635);
+ TEST_VERIFY_EXIT (*b == 842523635);
+ return -1;
+}
+
+/* Invalid comparison function that always returns 1. */
+static int
+invalid_compare_2 (const void *a1, const void *b1)
+{
+ const int *a = a1;
+ const int *b = b1;
+ TEST_COMPARE (*a, 842523635);
+ TEST_COMPARE (*b, 842523635);
+ TEST_VERIFY_EXIT (*a == 842523635);
+ TEST_VERIFY_EXIT (*b == 842523635);
+ return 1;
+}
+
+static int
+do_test (void)
+{
+ int array[] = {842523635, 842523635, 842523635, 842523635, 842523635};
+ qsort (array, array_length (array), sizeof (array[0]), invalid_compare_1);
+ qsort (array, array_length (array), sizeof (array[0]), invalid_compare_2);
+ return 0;
+}
+
+#include <support/test-driver.c>

View File

@ -159,7 +159,7 @@ Version: %{glibcversion}
# - It allows using the Release number without the %%dist tag in the dependency # - It allows using the Release number without the %%dist tag in the dependency
# generator to make the generated requires interchangeable between Rawhide # generator to make the generated requires interchangeable between Rawhide
# and ELN (.elnYY < .fcXX). # and ELN (.elnYY < .fcXX).
%global baserelease 24 %global baserelease 25
Release: %{baserelease}%{?dist} Release: %{baserelease}%{?dist}
# In general, GPLv2+ is used by programs, LGPLv2+ is used for # In general, GPLv2+ is used by programs, LGPLv2+ is used for
@ -2202,6 +2202,9 @@ update_gconv_modules_cache ()
%files -f compat-libpthread-nonshared.filelist -n compat-libpthread-nonshared %files -f compat-libpthread-nonshared.filelist -n compat-libpthread-nonshared
%changelog %changelog
* Mon Nov 27 2023 Florian Weimer <fweimer@redhat.com> - 2.38.9000-25
- Fix qsort workaround (#2248502)
* Thu Nov 23 2023 Florian Weimer <fweimer@redhat.com> - 2.38.9000-24 * Thu Nov 23 2023 Florian Weimer <fweimer@redhat.com> - 2.38.9000-24
- Restore qsort workaround for 389-ds-base. (#2248502) - Restore qsort workaround for 389-ds-base. (#2248502)