From: Adhemerval Zanella Netto <adhemerval.zanella@linaro.org>
To: libc-alpha@sourceware.org, Stefan Liebler <stli@linux.ibm.com>
Subject: Re: [PATCH] test-container: Use nftw instead of rm -rf
Date: Thu, 28 Sep 2023 08:11:45 -0300 [thread overview]
Message-ID: <64a68058-0371-40fd-b65d-369f53539e35@linaro.org> (raw)
In-Reply-To: <6a83644f-98e9-5329-a8e9-c8a731e716ac@linux.ibm.com>
On 28/09/23 06:40, Stefan Liebler wrote:
> On 27.09.23 21:20, Adhemerval Zanella wrote:
>> If the binary to run is 'env', test-containers skips it and adds
>> any required environment variable on the process envs variables.
>> This simplifies the required code to spawn new process (no need
>> to build an env-like program).
>>
>> However, this is an issue for recursive_remove if there is any
>> LD_PRELOAD, since test-container will not prepend the loader command
>> along with required paths. If the required preloaded library can
>> not be loaded by the system glibc, the 'post-clean rsync' will
>> eventually fail.
>>
>> One example is if system glibc does not support DT_RELR and the
>> built glibc does, the nss/tst-nss-gai-hv2-canonname test fails
>> with:
>>
>> ../scripts/evaluate-test.sh nss/tst-nss-gai-hv2-canonname $? false false
>> 86_64-linux-gnu/nss/tst-nss-gai-hv2-canonname.test-result
>> rm: /lib/x86_64-linux-gnu/libc.so.6: version `GLIBC_ABI_DT_RELR' not
>> found (required by x86_64-linux-gnu/malloc/libc_malloc_debug.so)
>>
>> Instead trying to figure out the required loader arguments on how
>> to spawn the 'rm -rf', replace the command with a nftw call.
>>
> Just as information, I've also recognized this issue. For me (on
> x86_64/s390x), I got "*** stack smashing detected ***: terminated" in
> this rm due to preloading with the fresh build libc_malloc_debug.so.
> In malloc_hook_ini -> generic_hook_ini -> initialize_malloc_check, we have:
> TUNABLE_GET (check, int32_t, TUNABLE_CALLBACK (set_mallopt_check));
> In __tunable_get_val, the tunables ID from current-build and system for
> glibc.malloc.check differs. In my case it is 30, which is beyond the
> range of the systems tunable_list[]. This leads to storing a 8byte value
> to passed valp. Unfortunately it points to a 4 byte int32_t and the
> other 4 bytes belong to the stack smashing canary.
Indeed the tunables might another problem, thanks for checking it.
>
> Your patch solves the issue for me.
Is it a reviewed-by ;) ?
>> Checked on x86_64-linux-gnu.
>> ---
>> support/test-container.c | 34 +++++++++++-----------------------
>> 1 file changed, 11 insertions(+), 23 deletions(-)
>>
>> diff --git a/support/test-container.c b/support/test-container.c
>> index 788b091ea0..95dfef1a99 100644
>> --- a/support/test-container.c
>> +++ b/support/test-container.c
>> @@ -37,6 +37,7 @@
>> #include <errno.h>
>> #include <error.h>
>> #include <libc-pointer-arith.h>
>> +#include <ftw.h>
>>
>> #ifdef __linux__
>> #include <sys/mount.h>
>> @@ -405,32 +406,19 @@ file_exists (char *path)
>> return 0;
>> }
>>
>> +static int
>> +unlink_cb (const char *fpath, const struct stat *sb, int typeflag,
>> + struct FTW *ftwbuf)
>> +{
>> + return remove (fpath);
>> +}
>> +
>> static void
>> recursive_remove (char *path)
>> {
>> - pid_t child;
>> - int status;
>> -
>> - child = fork ();
>> -
>> - switch (child) {
>> - case -1:
>> - perror("fork");
>> - FAIL_EXIT1 ("Unable to fork");
>> - case 0:
>> - /* Child. */
>> - execlp ("rm", "rm", "-rf", path, NULL);
>> - FAIL_EXIT1 ("exec rm: %m");
>> - default:
>> - /* Parent. */
>> - waitpid (child, &status, 0);
>> - /* "rm" would have already printed a suitable error message. */
>> - if (! WIFEXITED (status)
>> - || WEXITSTATUS (status) != 0)
>> - FAIL_EXIT1 ("exec child returned status: %d", status);
>> -
>> - break;
>> - }
>> + int r = nftw (path, unlink_cb, 1000, FTW_DEPTH | FTW_PHYS);
>> + if (r == -1)
>> + FAIL_EXIT1 ("recursive_remove failed");
>> }
>>
>> /* Used for both rsync and the mytest.script "cp" command. */
>
next prev parent reply other threads:[~2023-09-28 11:11 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-09-27 19:20 Adhemerval Zanella
2023-09-28 9:40 ` Stefan Liebler
2023-09-28 11:11 ` Adhemerval Zanella Netto [this message]
2023-09-28 11:44 ` Stefan Liebler
2023-09-28 11:30 ` Siddhesh Poyarekar
2023-10-01 17:27 ` Andreas K. Huettel
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=64a68058-0371-40fd-b65d-369f53539e35@linaro.org \
--to=adhemerval.zanella@linaro.org \
--cc=libc-alpha@sourceware.org \
--cc=stli@linux.ibm.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).