[PATCH 04/21] crypto: remove redundant access() checks before loading certs

Daniel P. Berrangé posted 21 patches 2 weeks, 1 day ago
Only 6 patches received!
There is a newer version of this series
[PATCH 04/21] crypto: remove redundant access() checks before loading certs
Posted by Daniel P. Berrangé 2 weeks, 1 day ago
The qcrypto_tls_creds_get_path method will perform an access()
check on the file and return a NULL path if it fails. By the
time we get to loading the cert files we know they must exist
on disk and thus the second access() check is redundant.

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 crypto/tlscredsx509.c | 22 ++++++++++------------
 1 file changed, 10 insertions(+), 12 deletions(-)

diff --git a/crypto/tlscredsx509.c b/crypto/tlscredsx509.c
index 75c70af522..0acb17b6ec 100644
--- a/crypto/tlscredsx509.c
+++ b/crypto/tlscredsx509.c
@@ -496,8 +496,7 @@ qcrypto_tls_creds_x509_sanity_check(QCryptoTLSCredsX509 *creds,
     size_t i;
     int ret = -1;
 
-    if (certFile &&
-        access(certFile, R_OK) == 0) {
+    if (certFile) {
         if (qcrypto_tls_creds_load_cert_list(creds,
                                              certFile,
                                              &certs,
@@ -508,16 +507,15 @@ qcrypto_tls_creds_x509_sanity_check(QCryptoTLSCredsX509 *creds,
             goto cleanup;
         }
     }
-    if (access(cacertFile, R_OK) == 0) {
-        if (qcrypto_tls_creds_load_cert_list(creds,
-                                             cacertFile,
-                                             &cacerts,
-                                             &ncacerts,
-                                             isServer,
-                                             true,
-                                             errp) < 0) {
-            goto cleanup;
-        }
+
+    if (qcrypto_tls_creds_load_cert_list(creds,
+                                         cacertFile,
+                                         &cacerts,
+                                         &ncacerts,
+                                         isServer,
+                                         true,
+                                         errp) < 0) {
+        goto cleanup;
     }
 
     for (i = 0; i < ncerts; i++) {
-- 
2.51.1


Re: [PATCH 04/21] crypto: remove redundant access() checks before loading certs
Posted by Marc-André Lureau 2 weeks ago
Hi

On Thu, Oct 30, 2025 at 6:48 PM Daniel P. Berrangé <berrange@redhat.com>
wrote:

> The qcrypto_tls_creds_get_path method will perform an access()
> check on the file and return a NULL path if it fails. By the
> time we get to loading the cert files we know they must exist
> on disk and thus the second access() check is redundant.
>
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
>

Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>


> ---
>  crypto/tlscredsx509.c | 22 ++++++++++------------
>  1 file changed, 10 insertions(+), 12 deletions(-)
>
> diff --git a/crypto/tlscredsx509.c b/crypto/tlscredsx509.c
> index 75c70af522..0acb17b6ec 100644
> --- a/crypto/tlscredsx509.c
> +++ b/crypto/tlscredsx509.c
> @@ -496,8 +496,7 @@
> qcrypto_tls_creds_x509_sanity_check(QCryptoTLSCredsX509 *creds,
>      size_t i;
>      int ret = -1;
>
> -    if (certFile &&
> -        access(certFile, R_OK) == 0) {
> +    if (certFile) {
>          if (qcrypto_tls_creds_load_cert_list(creds,
>                                               certFile,
>                                               &certs,
> @@ -508,16 +507,15 @@
> qcrypto_tls_creds_x509_sanity_check(QCryptoTLSCredsX509 *creds,
>              goto cleanup;
>          }
>      }
> -    if (access(cacertFile, R_OK) == 0) {
> -        if (qcrypto_tls_creds_load_cert_list(creds,
> -                                             cacertFile,
> -                                             &cacerts,
> -                                             &ncacerts,
> -                                             isServer,
> -                                             true,
> -                                             errp) < 0) {
> -            goto cleanup;
> -        }
> +
> +    if (qcrypto_tls_creds_load_cert_list(creds,
> +                                         cacertFile,
> +                                         &cacerts,
> +                                         &ncacerts,
> +                                         isServer,
> +                                         true,
> +                                         errp) < 0) {
> +        goto cleanup;
>      }
>
>      for (i = 0; i < ncerts; i++) {
> --
> 2.51.1
>
>