• David Howells's avatar
    cifs: Fix encryption of cleared, but unset rq_iter data buffers · 37de5a80
    David Howells authored
    Each smb_rqst struct contains two things: an array of kvecs (rq_iov) that
    contains the protocol data for an RPC op and an iterator (rq_iter) that
    contains the data payload of an RPC op.  When an smb_rqst is allocated
    rq_iter is it always cleared, but we don't set it up unless we're going to
    use it.
    
    The functions that determines the size of the ciphertext buffer that will
    be needed to encrypt a request, cifs_get_num_sgs(), assumes that rq_iter is
    always initialised - and employs user_backed_iter() to check that the
    iterator isn't user-backed.  This used to incidentally work, because
    ->user_backed was set to false because the iterator has never been
    initialised, but with commit f1b4cb65[1]
    which changes user_backed_iter() to determine this based on the iterator
    type insted, a warning is now emitted:
    
            WARNING: CPU: 7 PID: 4584 at fs/smb/client/cifsglob.h:2165 smb2_get_aead_req+0x3fc/0x420 [cifs]
            ...
            RIP: 0010:smb2_get_aead_req+0x3fc/0x420 [cifs]
            ...
             crypt_message+0x33e/0x550 [cifs]
             smb3_init_transform_rq+0x27d/0x3f0 [cifs]
             smb_send_rqst+0xc7/0x160 [cifs]
             compound_send_recv+0x3ca/0x9f0 [cifs]
             cifs_send_recv+0x25/0x30 [cifs]
             SMB2_tcon+0x38a/0x820 [cifs]
             cifs_get_smb_ses+0x69c/0xee0 [cifs]
             cifs_mount_get_session+0x76/0x1d0 [cifs]
             dfs_mount_share+0x74/0x9d0 [cifs]
             cifs_mount+0x6e/0x2e0 [cifs]
             cifs_smb3_do_mount+0x143/0x300 [cifs]
             smb3_get_tree+0x15e/0x290 [cifs]
             vfs_get_tree+0x2d/0xe0
             do_new_mount+0x124/0x340
             __se_sys_mount+0x143/0x1a0
    
    The problem is that rq_iter was never set, so the type is 0 (ie. ITER_UBUF)
    which causes user_backed_iter() to return true.  The code doesn't
    malfunction because it checks the size of the iterator - which is 0.
    
    Fix cifs_get_num_sgs() to ignore rq_iter if its count is 0, thereby
    bypassing the warnings.
    
    It might be better to explicitly initialise rq_iter to a zero-length
    ITER_BVEC, say, as it can always be reinitialised later.
    
    Fixes: d08089f6 ("cifs: Change the I/O paths to use an iterator rather than a page list")
    Reported-by: default avatarDamian Tometzki <damian@riscv-rocks.de>
    Closes: https://lore.kernel.org/r/ZUfQo47uo0p2ZsYg@fedora.fritz.box/Tested-by: default avatarDamian Tometzki <damian@riscv-rocks.de>
    Cc: stable@vger.kernel.org
    cc: Eric Biggers <ebiggers@kernel.org>
    cc: linux-cifs@vger.kernel.org
    cc: linux-fsdevel@vger.kernel.org
    Link: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=f1b4cb650b9a0eeba206d8f069fcdc532bfbcd74 [1]
    Reviewed-by: default avatarPaulo Alcantara (SUSE) <pc@manguebit.com>
    Signed-off-by: default avatarDavid Howells <dhowells@redhat.com>
    Signed-off-by: default avatarSteve French <stfrench@microsoft.com>
    37de5a80
cifsglob.h 74.7 KB