PATCH: testsuite debuginfod

2021-09-30 Thread Frank Ch. Eigler via Elfutils-devel
commit 85602ff68179053f19a2005df4fc653a69757584 (HEAD -> master)
Author: Frank Ch. Eigler 
Date:   Thu Sep 30 10:48:00 2021 -0400

debuginfod testsuite: Use ! CMD syntax.

Previously, we had a mishmash of iffy && || constructs to
reverse the rc of a subprocess that we expected to fail.
Now use
   ! CMD
or
   ! (CMD | CMD)
more systematically where possible.

Signed-off-by: Frank Ch. Eigler 

diff --git a/tests/ChangeLog b/tests/ChangeLog
index b62bb3507fd1..578e8cb15502 100644
--- a/tests/ChangeLog
+++ b/tests/ChangeLog
@@ -1,3 +1,7 @@
+2021-09-30  Frank Ch. Eigler  
+
+   * run-debuginfod-*.sh: Use ! CMD to assert expected failure.
+
 2021-09-17  Noah Sanci  
 
* run-debuginfod-response-header.sh: removed checking for Connection
diff --git a/tests/run-debuginfod-000-permission.sh 
b/tests/run-debuginfod-000-permission.sh
index 1e92bdb8cc34..8480ad486886 100755
--- a/tests/run-debuginfod-000-permission.sh
+++ b/tests/run-debuginfod-000-permission.sh
@@ -50,7 +50,7 @@ ps -q $PID1 -e -L -o '%p %c %a' | grep traverse
 rm -rf $DEBUGINFOD_CACHE_PATH # clean it from previous tests
 
 # The query is designed to fail, while the 000-permission file should be 
created.
-testrun ${abs_top_builddir}/debuginfod/debuginfod-find debuginfo 01234567 || 
true
+! testrun ${abs_top_builddir}/debuginfod/debuginfod-find debuginfo 01234567
 if [ ! -f $DEBUGINFOD_CACHE_PATH/01234567/debuginfo ]; then
   echo "could not find cache in $DEBUGINFOD_CACHE_PATH"
   err
@@ -62,7 +62,7 @@ if [ -r $DEBUGINFOD_CACHE_PATH/01234567/debuginfo ]; then
 fi
 
 bytecount_before=`curl -s http://127.0.0.1:$PORT1/metrics | grep 
'http_responses_transfer_bytes_count{code="404"}'`
-testrun ${abs_top_builddir}/debuginfod/debuginfod-find debuginfo 01234567 || 
true
+! testrun ${abs_top_builddir}/debuginfod/debuginfod-find debuginfo 01234567
 bytecount_after=`curl -s http://127.0.0.1:$PORT1/metrics | grep 
'http_responses_transfer_bytes_count{code="404"}'`
 if [ "$bytecount_before" != "$bytecount_after" ]; then
   echo "http_responses_transfer_bytes_count{code="404"} has changed."
@@ -73,7 +73,7 @@ fi
 echo 0 > $DEBUGINFOD_CACHE_PATH/cache_miss_s
 sleep 1
 bytecount_before=`curl -s http://127.0.0.1:$PORT1/metrics | grep 
'http_responses_transfer_bytes_count{code="404"}'`
-testrun ${abs_top_builddir}/debuginfod/debuginfod-find debuginfo 01234567 || 
true
+! testrun ${abs_top_builddir}/debuginfod/debuginfod-find debuginfo 01234567
 bytecount_after=`curl -s http://127.0.0.1:$PORT1/metrics | grep 
'http_responses_transfer_bytes_count{code="404"}'`
 if [ "$bytecount_before" == "$bytecount_after" ]; then
   echo "http_responses_transfer_bytes_count{code="404"} should be incremented."
diff --git a/tests/run-debuginfod-archive-groom.sh 
b/tests/run-debuginfod-archive-groom.sh
index 7813ee28b8f2..3a062ed278d6 100755
--- a/tests/run-debuginfod-archive-groom.sh
+++ b/tests/run-debuginfod-archive-groom.sh
@@ -154,10 +154,10 @@ wait_ready $PORT1 'groomed_total{decision="stale"}' 4
 rm -rf $DEBUGINFOD_CACHE_PATH # clean it from previous tests
 
 # this is one of the buildids from the groom-deleted rpms
-testrun ${abs_top_builddir}/debuginfod/debuginfod-find executable $RPM_BUILDID 
&& false || true
+! testrun ${abs_top_builddir}/debuginfod/debuginfod-find executable 
$RPM_BUILDID
 
 # but this one was not deleted so should be still around
-testrun ${abs_top_builddir}/debuginfod/debuginfod-find executable $BUILDID || 
true
+testrun ${abs_top_builddir}/debuginfod/debuginfod-find executable $BUILDID
 
 kill $PID1
 wait $PID1
diff --git a/tests/run-debuginfod-duplicate-urls.sh 
b/tests/run-debuginfod-duplicate-urls.sh
index 50e39cb2df95..fbcae4716977 100755
--- a/tests/run-debuginfod-duplicate-urls.sh
+++ b/tests/run-debuginfod-duplicate-urls.sh
@@ -41,8 +41,8 @@ wait_ready $PORT1 'ready' 1
 ## PR27983
 # Ensure no duplicate urls are used in when querying servers for files
 rm -rf $DEBUGINFOD_CACHE_PATH # clean it from previous tests
-env DEBUGINFOD_URLS="http://127.0.0.1:$PORT1 http://127.0.0.1:$PORT1 
http://127.0.0.1:$PORT1 http://127.0.0.1:7999"; \
- LD_LIBRARY_PATH=$ldpath ${abs_top_builddir}/debuginfod/debuginfod-find -vvv 
executable 0 > vlog1 2>&1 || true
+! env DEBUGINFOD_URLS="http://127.0.0.1:$PORT1 http://127.0.0.1:$PORT1 
http://127.0.0.1:$PORT1 http://127.0.0.1:7999"; \
+ LD_LIBRARY_PATH=$ldpath ${abs_top_builddir}/debuginfod/debuginfod-find -vvv 
executable 0 > vlog1 2>&1
 tempfiles vlog1
 cat vlog1
 if [ $( grep -c 'duplicate url: http://127.0.0.1:'$PORT1'.*' vlog1 ) -ne 2 ]; 
then
diff --git a/tests/run-debuginfod-fd-prefetch-caches.sh 
b/tests/run-debuginfod-fd-prefetch-caches.sh
index 7fbf7b20a6e8..f65ec31b9a9c 100755
--- a/tests/run-debuginfod-fd-prefetch-caches.sh
+++ b/tests/run-debuginfod-fd-prefetch-caches.sh
@@ -50,7 +50,7 @@ grep 'fdcache mbs ' vlog$PORT1 #$FDCACHE_MBS
 grep 'prefetch fds ' vlog$PORT1 #$PREFETCH_FDS
 grep 'prefetch mbs ' vlog$PORT1 #$PREFETCH_MBS
 # search the vlog to find w

patch resend updated: PR28240, debuginfod cache for root

2021-09-30 Thread Frank Ch. Eigler via Elfutils-devel
commit 56f40abf2cf18775deb25797e1b89e7788b80630 (HEAD -> master)
Author: Frank Ch. Eigler 
Date:   Wed Aug 18 18:29:34 2021 -0400

PR28240: debuginfod client root-safe negative caching

Negative cache (000-permission) files were incorrectly treated as
valid cached files for the root user, because root can open even
000-perm files without -EACCES.  Corrected this checking sequence.

Fixed the debuginfod testsuite to run to completion as root or
as an ordinary user, correcting corresponding permission checks:
stat -c %A $FILE
is right and
[ -w $FILE]  [ -r $FILE ]
were wrong.

Signed-off-by: Frank Ch. Eigler 

diff --git a/debuginfod/ChangeLog b/debuginfod/ChangeLog
index c2bfce98fbd1..b37d6482dba3 100644
--- a/debuginfod/ChangeLog
+++ b/debuginfod/ChangeLog
@@ -1,3 +1,9 @@
+2021-09-30  Frank Ch. Eigler  
+
+   PR28240
+   * debuginfod-client.c (debuginfod_query_server): Correct
+   negative-hit cache check sequence for root user.
+
 2021-09-17  Noah Sanci  
 
* debuginfod-client.c (debuginfod_query_server): curl_multi_perform
diff --git a/debuginfod/debuginfod-client.c b/debuginfod/debuginfod-client.c
index 88e45567d44a..43ed4b3b9191 100644
--- a/debuginfod/debuginfod-client.c
+++ b/debuginfod/debuginfod-client.c
@@ -1,5 +1,5 @@
 /* Retrieve ELF / DWARF / source files from the debuginfod.
-   Copyright (C) 2019-2020 Red Hat, Inc.
+   Copyright (C) 2019-2021 Red Hat, Inc.
This file is part of elfutils.
 
This file is free software; you can redistribute it and/or modify
@@ -766,24 +766,14 @@ debuginfod_query_server (debuginfod_client *c,
   if (rc != 0)
 goto out;
 
-  /* If the target is already in the cache then we are done.  */
-  int fd = open (target_cache_path, O_RDONLY);
-  if (fd >= 0)
-{
-  /* Success */
-  if (path != NULL)
-*path = strdup(target_cache_path);
-  rc = fd;
-  goto out;
-}
-
   struct stat st;
-  time_t cache_miss;
-  /* Check if the file exists and it's of permission 000*/
-  if (errno == EACCES
-  && stat(target_cache_path, &st) == 0
+  /* Check if the file exists and it's of permission 000; must check
+ explicitly rather than trying to open it first (PR28240). */
+  if (stat(target_cache_path, &st) == 0
   && (st.st_mode & 0777) == 0)
 {
+  time_t cache_miss;
+
   rc = debuginfod_config_cache(cache_miss_path, cache_miss_default_s, &st);
   if (rc < 0)
 goto out;
@@ -797,6 +787,18 @@ debuginfod_query_server (debuginfod_client *c,
   else
 unlink(target_cache_path);
 }
+  
+  /* If the target is already in the cache, and not a 000 file (PR28240), 
+ then we are done. */
+  int fd = open (target_cache_path, O_RDONLY);
+  if (fd >= 0)
+{
+  /* Success */
+  if (path != NULL)
+*path = strdup(target_cache_path);
+  rc = fd;
+  goto out;
+}
 
   long timeout = default_timeout;
   const char* timeout_envvar = getenv(DEBUGINFOD_TIMEOUT_ENV_VAR);
diff --git a/tests/ChangeLog b/tests/ChangeLog
index 578e8cb15502..e5aca513f1e8 100644
--- a/tests/ChangeLog
+++ b/tests/ChangeLog
@@ -1,3 +1,9 @@
+2021-09-30  Frank Ch. Eigler  
+
+   PR28240
+   * run-debuginfod-000-permission.sh, -writable.sh:
+   Correct negative-cache file permission checking.
+
 2021-09-30  Frank Ch. Eigler  
 
* run-debuginfod-*.sh: Use ! CMD to assert expected failure.
diff --git a/tests/run-debuginfod-000-permission.sh 
b/tests/run-debuginfod-000-permission.sh
index 8480ad486886..301ce7e7d9d3 100755
--- a/tests/run-debuginfod-000-permission.sh
+++ b/tests/run-debuginfod-000-permission.sh
@@ -56,7 +56,7 @@ if [ ! -f $DEBUGINFOD_CACHE_PATH/01234567/debuginfo ]; then
   err
 fi
 
-if [ -r $DEBUGINFOD_CACHE_PATH/01234567/debuginfo ]; then
+if [ `stat -c "%A" $DEBUGINFOD_CACHE_PATH/01234567/debuginfo` != "--" 
]; then
   echo "The cache $DEBUGINFOD_CACHE_PATH/01234567/debuginfo is readable"
   err
 fi
diff --git a/tests/run-debuginfod-artifact-running.sh 
b/tests/run-debuginfod-artifact-running.sh
index 51fa9c0a40bd..b9267ade 100755
--- a/tests/run-debuginfod-artifact-running.sh
+++ b/tests/run-debuginfod-artifact-running.sh
@@ -90,10 +90,6 @@ wait_ready $PORT1 'thread_busy{role="scan"}' 0
 rm -rf $DEBUGINFOD_CACHE_PATH # clean it from previous tests
 filename=`testrun ${abs_top_builddir}/debuginfod/debuginfod-find debuginfo 
$BUILDID`
 cmp $filename F/prog.debug
-if [ -w $filename ]; then
-echo "cache file writable, boo"
-err
-fi
 
 filename=`testrun ${abs_top_builddir}/debuginfod/debuginfod-find executable 
F/prog`
 cmp $filename F/prog
diff --git a/tests/run-debuginfod-writable.sh b/tests/run-debuginfod-writable.sh
index 69ececb39454..9cc4ea1db92c 100755
--- a/tests/run-debuginfod-writable.sh
+++ b/tests/run-debuginfod-writable.sh
@@ -79,7 +79,7 @@ wait_ready $PORT1 'thread_busy{role="scan"}' 0
 rm -rf $DEBUGINFOD_CACHE_PATH # clean it from previous tests
 filename=`t