openzfs / zfs

OpenZFS on Linux and FreeBSD
https://openzfs.github.io/openzfs-docs
Other
10.69k stars 1.76k forks source link

Fix gcc uninitialized warning in FreeBSD zio_crypt.c #16688

Closed DimitryAndric closed 1 month ago

DimitryAndric commented 1 month ago

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().

DimitryAndric commented 1 month ago

@tsoome @amotin