commit:     268c2076b5276fbce37df3f751619646c4b8d7a4
Author:     Boris Staletic <boris.staletic <AT> protonmail <DOT> com>
AuthorDate: Thu Mar 28 18:08:00 2024 +0000
Commit:     Fabian Groffen <grobian <AT> gentoo <DOT> org>
CommitDate: Fri Mar 29 10:57:17 2024 +0000
URL:        https://gitweb.gentoo.org/proj/portage-utils.git/commit/?id=268c2076

qlop: Properly handle atom_compar_cb when called from qsort

`qsort` passes pointers to elements ("iterators" in C++ lingo) to the
callback, not elements directly.
Hence `l` and `r` in `atom_compar_cb` actually receives `depend_atom**`,
but only when called from `qsort`. The other two call sites
(`tree_pkg_compar` and `pkg_sort_cb`) actually apssed `depend_atom*`.

This leads to type casting confusion and undefined behaviour for any
invocation of `qlop -p`.

First discovered by SEGFAULT-ing with the following invocation:

    qlop -p `cat /var/lib/portage/world`

Valgrind and ASAN made triggering the SEGFAULT easier - any invocation
with two or more atoms triggered a NULL dereference.

This commit addresses the above problem:

1. Expect that `atom_compar_cb` is actually called with two
   `depend_atom**`.
2. Make `tree_pkg_compar` and `pkg_sort_cb` comply with the above
   change, by passing  `&al` and `&ar`, instead of `al` and `ar`.

Signed-off-by: Fabian Groffen <grobian <AT> gentoo.org>

 libq/atom.c | 4 ++--
 libq/tree.c | 2 +-
 qlop.c      | 2 +-
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/libq/atom.c b/libq/atom.c
index b1a150a..3cc2100 100644
--- a/libq/atom.c
+++ b/libq/atom.c
@@ -1242,8 +1242,8 @@ atom_format(const char *format, const depend_atom *atom)
 inline int
 atom_compar_cb(const void *l, const void *r)
 {
-       const depend_atom *al = l;
-       const depend_atom *ar = r;
+       const depend_atom *al = *(const depend_atom**)l;
+       const depend_atom *ar = *(const depend_atom**)r;
 
        switch (atom_compare(al, ar)) {
                case EQUAL:  return  0;

diff --git a/libq/tree.c b/libq/tree.c
index 4678634..335ac79 100644
--- a/libq/tree.c
+++ b/libq/tree.c
@@ -463,7 +463,7 @@ tree_pkg_compar(const void *l, const void *r)
        depend_atom *al = tree_get_atom(pl, false);
        depend_atom *ar = tree_get_atom(pr, false);
 
-       return atom_compar_cb(al, ar);
+       return atom_compar_cb(&al, &ar);
 }
 
 static tree_pkg_ctx *

diff --git a/qlop.c b/qlop.c
index 3e6db53..cfad246 100644
--- a/qlop.c
+++ b/qlop.c
@@ -309,7 +309,7 @@ pkg_sort_cb(const void *l, const void *r)
        depend_atom *al = pl->atom;
        depend_atom *ar = pr->atom;
 
-       return atom_compar_cb(al, ar);
+       return atom_compar_cb(&al, &ar);
 }
 
 /* The format of the sync log has changed over time.

Reply via email to