Skip to content

Commit 37de5a8

Browse files
dhowellssmfrench
authored andcommitted
cifs: Fix encryption of cleared, but unset rq_iter data buffers
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 f1b4cb6[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: d08089f ("cifs: Change the I/O paths to use an iterator rather than a page list") Reported-by: Damian Tometzki <[email protected]> Closes: https://lore.kernel.org/r/[email protected]/ Tested-by: Damian Tometzki <[email protected]> Cc: [email protected] cc: Eric Biggers <[email protected]> cc: [email protected] cc: [email protected] Link: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=f1b4cb650b9a0eeba206d8f069fcdc532bfbcd74 [1] Reviewed-by: Paulo Alcantara (SUSE) <[email protected]> Signed-off-by: David Howells <[email protected]> Signed-off-by: Steve French <[email protected]>
1 parent 766e9cf commit 37de5a8

File tree

1 file changed

+9
-3
lines changed

1 file changed

+9
-3
lines changed

fs/smb/client/cifsglob.h

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2143,6 +2143,7 @@ static inline int cifs_get_num_sgs(const struct smb_rqst *rqst,
21432143
unsigned int len, skip;
21442144
unsigned int nents = 0;
21452145
unsigned long addr;
2146+
size_t data_size;
21462147
int i, j;
21472148

21482149
/*
@@ -2158,17 +2159,21 @@ static inline int cifs_get_num_sgs(const struct smb_rqst *rqst,
21582159
* rqst[1+].rq_iov[0+] data to be encrypted/decrypted
21592160
*/
21602161
for (i = 0; i < num_rqst; i++) {
2162+
data_size = iov_iter_count(&rqst[i].rq_iter);
2163+
21612164
/* We really don't want a mixture of pinned and unpinned pages
21622165
* in the sglist. It's hard to keep track of which is what.
21632166
* Instead, we convert to a BVEC-type iterator higher up.
21642167
*/
2165-
if (WARN_ON_ONCE(user_backed_iter(&rqst[i].rq_iter)))
2168+
if (data_size &&
2169+
WARN_ON_ONCE(user_backed_iter(&rqst[i].rq_iter)))
21662170
return -EIO;
21672171

21682172
/* We also don't want to have any extra refs or pins to clean
21692173
* up in the sglist.
21702174
*/
2171-
if (WARN_ON_ONCE(iov_iter_extract_will_pin(&rqst[i].rq_iter)))
2175+
if (data_size &&
2176+
WARN_ON_ONCE(iov_iter_extract_will_pin(&rqst[i].rq_iter)))
21722177
return -EIO;
21732178

21742179
for (j = 0; j < rqst[i].rq_nvec; j++) {
@@ -2184,7 +2189,8 @@ static inline int cifs_get_num_sgs(const struct smb_rqst *rqst,
21842189
}
21852190
skip = 0;
21862191
}
2187-
nents += iov_iter_npages(&rqst[i].rq_iter, INT_MAX);
2192+
if (data_size)
2193+
nents += iov_iter_npages(&rqst[i].rq_iter, INT_MAX);
21882194
}
21892195
nents += DIV_ROUND_UP(offset_in_page(sig) + SMB2_SIGNATURE_SIZE, PAGE_SIZE);
21902196
return nents;

0 commit comments

Comments
 (0)