From 57aa660de4d1b8375cd56f7b8b5fcaf8ad9a5af7 Mon Sep 17 00:00:00 2001 From: chantra Date: Fri, 25 Mar 2022 08:13:08 -0700 Subject: [PATCH 28/30] [reflink] remove requirement for executable stack flag reflink was calling `bsearch` with a nested function comparator which make GCC require the executable stack flag (see `man execstack`). selinux prevents the use of this flag: ``` error: Failed to dlopen /usr/lib64/rpm-plugins/reflink.so /usr/lib64/rpm-plugins/reflink.so: cannot enable executable stack as shared object requires: Permission denied ``` To fix this, either rpm could be granted the correct selinux permission, but this would open up execstack for more the whole rpm process. Fundamentally, this is happening because there is no re-entrant version of `bsearch`. We could probably use a global variable and be done with it given that each rpm is processed sequencially, but that contract may not hold true for ever. Here we are copying stdlib's `bsearch` and making it re-entrant by allowing to pass an void * data parameter where we can pass the key size. After applying this patch, when reflink.o is installed, it has the executable flag cleared: ``` - /usr/lib64/rpm-plugins/reflink.so ``` --- plugins/reflink.c | 60 +++++++++++++++++++++++++++++++++++++---------- 1 file changed, 48 insertions(+), 12 deletions(-) diff --git a/plugins/reflink.c b/plugins/reflink.c index 4fc1d74d1..69e6b51e6 100644 --- a/plugins/reflink.c +++ b/plugins/reflink.c @@ -59,6 +59,50 @@ struct reflink_state_s { typedef struct reflink_state_s * reflink_state; +/* + * bsearch_r: implements a re-entrant version of stdlib's bsearch. + * code taken and adapted from /usr/include/bits/stdlib-bsearch.h + */ +inline void * +bsearch_r (const void *__key, const void *__base, size_t __nmemb, size_t __size, + __compar_d_fn_t __compar, void *__arg) +{ + size_t __l, __u, __idx; + const void *__p; + int __comparison; + + __l = 0; + __u = __nmemb; + while (__l < __u) + { + __idx = (__l + __u) / 2; + __p = (const void *) (((const char *) __base) + (__idx * __size)); + __comparison = (*__compar) (__key, __p, __arg); + if (__comparison < 0) + __u = __idx; + else if (__comparison > 0) + __l = __idx + 1; + else + { +#if __GNUC_PREREQ(4, 6) +# pragma GCC diagnostic push +# pragma GCC diagnostic ignored "-Wcast-qual" +#endif + return (void *) __p; +#if __GNUC_PREREQ(4, 6) +# pragma GCC diagnostic pop +#endif + } + } + + return NULL; +} + +static int cmpdigest(const void *k1, const void *k2, void *data) { + rpmlog(RPMLOG_DEBUG, _("reflink: cmpdigest k1=%p k2=%p\n"), k1, k2); + return memcmp(k1, k2, *(int *)data); +} + static int inodeCmp(rpm_ino_t a, rpm_ino_t b) { return (a != b); @@ -198,21 +242,13 @@ static rpmRC reflink_psm_post(rpmPlugin plugin, rpmte te, int res) rpm_loff_t find(const unsigned char *digest, reflink_state state); rpm_loff_t find(const unsigned char *digest, reflink_state state) { -# if defined(__GNUC__) - /* GCC nested function because bsearch's comparison function can't access - * state-keysize otherwise - */ - int cmpdigest(const void *k1, const void *k2) { - rpmlog(RPMLOG_DEBUG, _("reflink: cmpdigest k1=%p k2=%p\n"), k1, k2); - return memcmp(k1, k2, state->keysize); - } -# endif rpmlog(RPMLOG_DEBUG, - _("reflink: bsearch(key=%p, base=%p, nmemb=%d, size=%lu)\n"), + _("reflink: bsearch_r(key=%p, base=%p, nmemb=%d, size=%lu)\n"), digest, state->table, state->keys, state->keysize + sizeof(rpm_loff_t)); - char *entry = bsearch(digest, state->table, state->keys, - state->keysize + sizeof(rpm_loff_t), cmpdigest); + char *entry = bsearch_r(digest, state->table, state->keys, + state->keysize + sizeof(rpm_loff_t), cmpdigest, + &state->keysize); if (entry == NULL) { return NOT_FOUND; } -- 2.35.1