Skip site navigation (1)Skip section navigation (2)
Date:      Tue, 29 Oct 2024 21:51:21 GMT
From:      Dimitry Andric <dim@FreeBSD.org>
To:        src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-branches@FreeBSD.org
Subject:   git: b426202aaf76 - stable/14 - Fix gcc uninitialized warning in FreeBSD zio_crypt.c
Message-ID:  <202410292151.49TLpLHA066206@gitrepo.freebsd.org>

next in thread | raw e-mail | index | archive | help
The branch stable/14 has been updated by dim:

URL: https://cgit.FreeBSD.org/src/commit/?id=b426202aaf76c0f815db60dc5aa392ac0625b4f6

commit b426202aaf76c0f815db60dc5aa392ac0625b4f6
Author:     Dimitry Andric <dim@FreeBSD.org>
AuthorDate: 2024-10-25 12:51:05 +0000
Commit:     Dimitry Andric <dim@FreeBSD.org>
CommitDate: 2024-10-29 21:49:16 +0000

    Fix gcc uninitialized warning in FreeBSD zio_crypt.c
    
    With gcc we are seeing the following -Werror warning:
    
        In file included from /workspace/src/sys/contrib/openzfs/include/os/freebsd/spl/sys/sunddi.h:28,
                         from /workspace/src/sys/contrib/openzfs/include/sys/zfs_context.h:69:
        In function 'zfs_uio_init',
            inlined from 'zio_do_crypt_data' at /workspace/src/sys/contrib/openzfs/module/os/freebsd/zfs/zio_crypt.c:1690:2:
        /workspace/src/sys/contrib/openzfs/include/os/freebsd/spl/sys/uio.h:102:45: error: 'puio_s.uio_offset' is used uninitialized [-Werror=uninitialized]
          102 |                 zfs_uio_soffset(uio) = uio_s->uio_offset;
              |                                        ~~~~~^~~~~~~~~~~~
        /workspace/src/sys/contrib/openzfs/module/os/freebsd/zfs/zio_crypt.c: In function 'zio_do_crypt_data':
        /workspace/src/sys/contrib/openzfs/module/os/freebsd/zfs/zio_crypt.c:1683:20: note: 'puio_s' declared here
         1683 |         struct uio puio_s, cuio_s;
              |                    ^~~~~~
        In function 'zfs_uio_init',
            inlined from 'zio_do_crypt_data' at /workspace/src/sys/contrib/openzfs/module/os/freebsd/zfs/zio_crypt.c:1691:2:
        /workspace/src/sys/contrib/openzfs/include/os/freebsd/spl/sys/uio.h:102:45: error: 'cuio_s.uio_offset' is used uninitialized [-Werror=uninitialized]
          102 |                 zfs_uio_soffset(uio) = uio_s->uio_offset;
              |                                        ~~~~~^~~~~~~~~~~~
        /workspace/src/sys/contrib/openzfs/module/os/freebsd/zfs/zio_crypt.c: In function 'zio_do_crypt_data':
        /workspace/src/sys/contrib/openzfs/module/os/freebsd/zfs/zio_crypt.c:1683:28: note: 'cuio_s' declared here
         1683 |         struct uio puio_s, cuio_s;
              |                            ^~~~~~
    
    Indeed, `zfs_uio_init()` does:
    
        static __inline void
        zfs_uio_init(zfs_uio_t *uio, struct uio *uio_s)
        {
                memset(uio, 0, sizeof (zfs_uio_t));
                if (uio_s != NULL) {
                        GET_UIO_STRUCT(uio) = uio_s;
                        zfs_uio_soffset(uio) = uio_s->uio_offset;
                }
        }
    
    while the code in `zio_crypt.c` has:
    
        /*
         * Primary encryption / decryption entrypoint for zio data.
         */
        int
        zio_do_crypt_data(boolean_t encrypt, zio_crypt_key_t *key,
            dmu_object_type_t ot, boolean_t byteswap, uint8_t *salt, uint8_t *iv,
            uint8_t *mac, uint_t datalen, uint8_t *plainbuf, uint8_t *cipherbuf,
            boolean_t *no_crypt)
        {
                int ret;
                boolean_t locked = B_FALSE;
                uint64_t crypt = key->zk_crypt;
                uint_t keydata_len = zio_crypt_table[crypt].ci_keylen;
                uint_t enc_len, auth_len;
                zfs_uio_t puio, cuio;
                struct uio puio_s, cuio_s;
                uint8_t enc_keydata[MASTER_KEY_MAX_LEN];
                crypto_key_t tmp_ckey, *ckey = NULL;
                freebsd_crypt_session_t *tmpl = NULL;
                uint8_t *authbuf = NULL;
    
                zfs_uio_init(&puio, &puio_s);
                zfs_uio_init(&cuio, &cuio_s);
                memset(GET_UIO_STRUCT(&puio), 0, sizeof (struct uio));
                memset(GET_UIO_STRUCT(&cuio), 0, sizeof (struct uio));
    
    So between the declaration of `puio_s` and `cuio_s`, there is no
    initialization of these variables before `zfs_uio_init()` gets called.
    
    Similar to the Linux variant of zio_crypt.c, I think it would be better
    to memset the structs `puio_s` and `cuis_s` _before_ calling
    `zfs_uio_init()`.
    
    Reviewed by:    tsoome
    MFC after:      3 days
    Differential Revision: https://reviews.freebsd.org/D47281
    
    (cherry picked from commit 3ceba58a7509418b47b8fca2d2b6bbf088714e26)
---
 sys/contrib/openzfs/module/os/freebsd/zfs/zio_crypt.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/sys/contrib/openzfs/module/os/freebsd/zfs/zio_crypt.c b/sys/contrib/openzfs/module/os/freebsd/zfs/zio_crypt.c
index b08916b317f8..ebf18aaa3403 100644
--- a/sys/contrib/openzfs/module/os/freebsd/zfs/zio_crypt.c
+++ b/sys/contrib/openzfs/module/os/freebsd/zfs/zio_crypt.c
@@ -1692,11 +1692,10 @@ zio_do_crypt_data(boolean_t encrypt, zio_crypt_key_t *key,
 	freebsd_crypt_session_t *tmpl = NULL;
 	uint8_t *authbuf = NULL;
 
-
+	memset(&puio_s, 0, sizeof (puio_s));
+	memset(&cuio_s, 0, sizeof (cuio_s));
 	zfs_uio_init(&puio, &puio_s);
 	zfs_uio_init(&cuio, &cuio_s);
-	memset(GET_UIO_STRUCT(&puio), 0, sizeof (struct uio));
-	memset(GET_UIO_STRUCT(&cuio), 0, sizeof (struct uio));
 
 #ifdef FCRYPTO_DEBUG
 	printf("%s(%s, %p, %p, %d, %p, %p, %u, %s, %p, %p, %p)\n",



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?202410292151.49TLpLHA066206>