commit 56f40abf2cf18775deb25797e1b89e7788b80630 (HEAD -> master)
Author: Frank Ch. Eigler <f...@redhat.com>
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 <f...@redhat.com>
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 <f...@redhat.com>
+
+ PR28240
+ * debuginfod-client.c (debuginfod_query_server): Correct
+ negative-hit cache check sequence for root user.
+
2021-09-17 Noah Sanci <nsa...@redhat.com>
* 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 <f...@redhat.com>
+
+ PR28240
+ * run-debuginfod-000-permission.sh, -writable.sh:
+ Correct negative-cache file permission checking.
+
2021-09-30 Frank Ch. Eigler <f...@redhat.com>
* 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..b94444267ade 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=`testrun ${abs_top_builddir}/debuginfod/debuginfod-find debuginfo
$BUILDID`
cmp $filename F/p+r%o\$g.debug
-if [ -w $filename ]; then
+if [ `stat -c "%A" $filename` != "-r--------" ]; then
echo "cache file writable, boo"
err
fi
