From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by sourceware.org (Postfix) with ESMTP id 17ACC3857C51 for ; Thu, 30 Sep 2021 18:37:04 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 17ACC3857C51 Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-206-AO87FpF3NOaCF7teeiuc9Q-1; Thu, 30 Sep 2021 14:37:00 -0400 X-MC-Unique: AO87FpF3NOaCF7teeiuc9Q-1 Received: from smtp.corp.redhat.com (int-mx07.intmail.prod.int.phx2.redhat.com [10.5.11.22]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 013D218D6A2C for ; Thu, 30 Sep 2021 18:37:00 +0000 (UTC) Received: from redhat.com (ovpn-112-152.phx2.redhat.com [10.3.112.152]) by smtp.corp.redhat.com (Postfix) with ESMTPS id C7B8310023AE for ; Thu, 30 Sep 2021 18:36:59 +0000 (UTC) Received: from fche by redhat.com with local (Exim 4.94.2) (envelope-from ) id 1mW0vQ-0005y8-Sf for elfutils-devel@sourceware.org; Thu, 30 Sep 2021 14:36:57 -0400 Date: Thu, 30 Sep 2021 14:36:56 -0400 From: "Frank Ch. Eigler" To: elfutils-devel@sourceware.org Subject: patch resend updated: PR28240, debuginfod cache for root Message-ID: <20210930183656.GA21634@redhat.com> MIME-Version: 1.0 User-Agent: Mutt/1.12.0 (2019-05-25) X-Scanned-By: MIMEDefang 2.84 on 10.5.11.22 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=us-ascii Content-Disposition: inline X-Spam-Status: No, score=-12.3 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, RCVD_IN_DNSWL_LOW, RCVD_IN_MSPIKE_H2, SPF_HELO_NONE, SPF_NONE, TXREP autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on server2.sourceware.org X-BeenThere: elfutils-devel@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Elfutils-devel mailing list List-Unsubscribe: , List-Archive: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 30 Sep 2021 18:37:05 -0000 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..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