Merge tag 'v7.2-rc5-smb3-client-fixes' of git://git.samba.org/sfrench/cifs-2.6

Pull smb client fixes from Steve French:

 - fix SMB1 read and write potential buffer leaks

 - netfs error handling fix

 - fix check for last write time in truncate and setattr and cleanup use
   of smb_store_release()

 - fscache fix and cleanup

 - validate idmap key payload length

 - minor SMB1 error mapping cleanup

 - witness protocol memory allocation fix

* tag 'v7.2-rc5-smb3-client-fixes' of git://git.samba.org/sfrench/cifs-2.6:
  cifs: add fscache_resize_cookie() to cifs_setsize()
  cifs: fix time_last_write stamp placement in setattr/truncate paths
  cifs: consolidate time_last_write stamp into _cifsFileInfo_put()
  smb: client: simplify cifs_fscache_get_super_cookie()
  smb: client: free partially allocated transform folio queue
  cifs: validate idmap key payload length
  smb: client: remove conditional return with no effect
  smb: client: fix buffer leaks in SMB1 read and write
  smb: client: use GFP_KERNEL for registry allocation

9 files changed, 63 insertions, 87 deletions

diff --git a/fs/smb/client/cifs_swn.c b/fs/smb/client/cifs_swn.c
index 9951817d0d7f..fe10719e627e 100644
--- a/fs/smb/client/cifs_swn.c
+++ b/fs/smb/client/cifs_swn.c
@@ -425,7 +425,7 @@ static struct cifs_swn_reg *cifs_find_swn_reg(struct cifs_tcon *tcon)
 
 /*
  * Get a registration for the tcon's server and share name, allocating a new one if it does not
- * exists
+ * exist.
  */
 static struct cifs_swn_reg *cifs_get_swn_reg(struct cifs_tcon *tcon)
 {
@@ -443,7 +443,7 @@ static struct cifs_swn_reg *cifs_get_swn_reg(struct cifs_tcon *tcon)
 		goto unlock;
 	}
 
-	reg = kmalloc_obj(struct cifs_swn_reg, GFP_ATOMIC);
+	reg = kmalloc_obj(struct cifs_swn_reg, GFP_KERNEL);
 	if (reg == NULL) {
 		ret = -ENOMEM;
 		goto fail_unlock;
@@ -451,7 +451,7 @@ static struct cifs_swn_reg *cifs_get_swn_reg(struct cifs_tcon *tcon)
 
 	kref_init(&reg->ref_count);
 
-	reg->id = idr_alloc(&cifs_swnreg_idr, reg, 1, 0, GFP_ATOMIC);
+	reg->id = idr_alloc(&cifs_swnreg_idr, reg, 1, 0, GFP_KERNEL);
 	if (reg->id < 0) {
 		cifs_dbg(FYI, "%s: failed to allocate registration id\n", __func__);
 		ret = reg->id;
diff --git a/fs/smb/client/cifsacl.c b/fs/smb/client/cifsacl.c
index 9424281a7674..12005f46307d 100644
--- a/fs/smb/client/cifsacl.c
+++ b/fs/smb/client/cifsacl.c
@@ -68,6 +68,9 @@ cifs_idmap_key_instantiate(struct key *key, struct key_preparsed_payload *prep)
 {
 	char *payload;
 
+	if (prep->datalen > U16_MAX)
+		return -EINVAL;
+
 	/*
 	 * If the payload is less than or equal to the size of a pointer, then
 	 * an allocation here is wasteful. Just copy the data directly to the
diff --git a/fs/smb/client/cifssmb.c b/fs/smb/client/cifssmb.c
index 40162d5554ea..1f77512252e7 100644
--- a/fs/smb/client/cifssmb.c
+++ b/fs/smb/client/cifssmb.c
@@ -1681,8 +1681,10 @@ CIFSSMBRead(const unsigned int xid, struct cifs_io_parms *io_parms,
 	pSMB->hdr.PidHigh = cpu_to_le16((__u16)(pid >> 16));
 
 	/* tcon and ses pointer are checked in smb_init */
-	if (tcon->ses->server == NULL)
+	if (!tcon->ses->server) {
+		cifs_small_buf_release(pSMB);
 		return -ECONNABORTED;
+	}
 
 	pSMB->AndXCommand = 0xFF;       /* none */
 	pSMB->Fid = netfid;
@@ -1796,8 +1798,10 @@ CIFSSMBWrite(const unsigned int xid, struct cifs_io_parms *io_parms,
 	pSMB->hdr.PidHigh = cpu_to_le16((__u16)(pid >> 16));
 
 	/* tcon and ses pointer are checked in smb_init */
-	if (tcon->ses->server == NULL)
+	if (!tcon->ses->server) {
+		cifs_buf_release(pSMB);
 		return -ECONNABORTED;
+	}
 
 	pSMB->AndXCommand = 0xFF;	/* none */
 	pSMB->Fid = netfid;
@@ -2077,8 +2081,10 @@ CIFSSMBWrite2(const unsigned int xid, struct cifs_io_parms *io_parms,
 	pSMB->hdr.PidHigh = cpu_to_le16((__u16)(pid >> 16));
 
 	/* tcon and ses pointer are checked in smb_init */
-	if (tcon->ses->server == NULL)
+	if (!tcon->ses->server) {
+		cifs_small_buf_release(pSMB);
 		return -ECONNABORTED;
+	}
 
 	pSMB->AndXCommand = 0xFF;	/* none */
 	pSMB->Fid = netfid;
diff --git a/fs/smb/client/file.c b/fs/smb/client/file.c
index b279a44be729..ac89c1ba56b1 100644
--- a/fs/smb/client/file.c
+++ b/fs/smb/client/file.c
@@ -915,6 +915,14 @@ void _cifsFileInfo_put(struct cifsFileInfo *cifs_file,
 		cifs_set_oplock_level(cifsi, 0);
 	}
 
+	if (OPEN_FMODE(cifs_file->f_flags) & FMODE_WRITE) {
+		/* Stamp while open_file_lock is held; covers all close paths
+		 * including background I/O. Pairs with smp_load_acquire() in
+		 * is_size_safe_to_change().
+		 */
+		smp_store_release(&cifsi->time_last_write, jiffies);
+	}
+
 	spin_unlock(&cifsi->open_file_lock);
 	spin_unlock(&tcon->open_file_lock);
 
@@ -1429,15 +1437,6 @@ void smb2_deferred_work_close(struct work_struct *work)
 	cifs_del_deferred_close(cfile);
 	cfile->deferred_close_scheduled = false;
 	spin_unlock(&cinode->deferred_lock);
-	/*
-	 * Refresh time_last_write immediately before the actual server close
-	 * so the protection window is anchored to the real close time, not
-	 * the earlier userspace close time stored by cifs_close().
-	 */
-	if (OPEN_FMODE(cfile->f_flags) & FMODE_WRITE) {
-		/* Pairs with smp_load_acquire() in is_size_safe_to_change(). */
-		smp_store_release(&cinode->time_last_write, jiffies);
-	}
 	_cifsFileInfo_put(cfile, true, false);
 }
 
@@ -1467,10 +1466,6 @@ int cifs_close(struct inode *inode, struct file *file)
 	if (file->private_data != NULL) {
 		cfile = file->private_data;
 		file->private_data = NULL;
-		if (file->f_mode & FMODE_WRITE) {
-			/* Pairs with smp_load_acquire() in is_size_safe_to_change(). */
-			smp_store_release(&cinode->time_last_write, jiffies);
-		}
 		dclose = kmalloc_obj(struct cifs_deferred_close);
 		if ((cfile->status_file_deleted == false) &&
 		    (smb2_can_defer_close(inode, dclose))) {
@@ -3276,13 +3271,13 @@ bool is_size_safe_to_change(struct cifsInodeInfo *cifsInode, __u64 end_of_file,
 	 * No writable handles open. Check whether we are within the attribute
 	 * cache validity window of a recent local modification.
 	 *
-	 * For the close() path: cifs_close() calls smp_store_release() on
-	 * time_last_write before _cifsFileInfo_put() removes the handle under
-	 * open_file_lock. That spin_unlock() is a store-release that pairs
-	 * with the spin_lock() (load-acquire) in is_inode_writable() above,
-	 * so if is_inode_writable() returned false the smp_load_acquire()
-	 * below is guaranteed to observe any time_last_write update from a
-	 * concurrent close().
+	 * For the close() path: _cifsFileInfo_put() stamps time_last_write
+	 * (via smp_store_release()) before releasing open_file_lock. That
+	 * spin_unlock() is a store-release that pairs with the spin_lock()
+	 * (load-acquire) in is_inode_writable() above, so if
+	 * is_inode_writable() returned false the smp_load_acquire() below is
+	 * guaranteed to observe any time_last_write update from a concurrent
+	 * close(), covering all close paths including background I/O.
 	 *
 	 * For the setattr/truncate paths: those callers use smp_store_release()
 	 * directly; the smp_load_acquire() below pairs with that store. There
@@ -3297,7 +3292,7 @@ bool is_size_safe_to_change(struct cifsInodeInfo *cifsInode, __u64 end_of_file,
 	 * jiffies is still close to INITIAL_JIFFIES on 32-bit systems.
 	 */
 	if (from_readdir) {
-		/* Pairs with smp_store_release() at close and truncate sites. */
+		/* Pairs with smp_store_release() in _cifsFileInfo_put() and setattr. */
 		tlw = smp_load_acquire(&cifsInode->time_last_write);
 		if (tlw && time_before(jiffies, tlw + cifs_sb->ctx->acregmax))
 			return false;
diff --git a/fs/smb/client/fscache.c b/fs/smb/client/fscache.c
index 01424a5cdb99..9f66bbcd3801 100644
--- a/fs/smb/client/fscache.c
+++ b/fs/smb/client/fscache.c
@@ -38,7 +38,6 @@ int cifs_fscache_get_super_cookie(struct cifs_tcon *tcon)
 	struct TCP_Server_Info *server = tcon->ses->server;
 	struct fscache_volume *vcookie;
 	const struct sockaddr *sa = (struct sockaddr *)&server->dstaddr;
-	size_t slen, i;
 	char *sharename;
 	char *key;
 	int ret = -ENOMEM;
@@ -73,10 +72,7 @@ int cifs_fscache_get_super_cookie(struct cifs_tcon *tcon)
 		return PTR_ERR(sharename);
 	}
 
-	slen = strlen(sharename);
-	for (i = 0; i < slen; i++)
-		if (sharename[i] == '/')
-			sharename[i] = ';';
+	strreplace(sharename, '/', ';');
 
 	key = kasprintf(GFP_KERNEL, "cifs,%pISpc,%s", sa, sharename);
 	if (!key)
diff --git a/fs/smb/client/inode.c b/fs/smb/client/inode.c
index b2806371bfde..0afff761aab9 100644
--- a/fs/smb/client/inode.c
+++ b/fs/smb/client/inode.c
@@ -3059,6 +3059,7 @@ void cifs_setsize(struct inode *inode, loff_t offset)
 	inode_set_mtime_to_ts(inode, inode_set_ctime_current(inode));
 	truncate_pagecache(inode, offset);
 	netfs_wait_for_outstanding_io(inode);
+	fscache_resize_cookie(cifs_inode_cookie(inode), offset);
 }
 
 int cifs_file_set_size(const unsigned int xid, struct dentry *dentry,
@@ -3190,6 +3191,17 @@ cifs_setattr_unix(struct dentry *direntry, struct iattr *attrs)
 	rc = 0;
 
 	if (attrs->ia_valid & ATTR_SIZE) {
+		if (attrs->ia_size != i_size_read(inode)) {
+			/* Stamp before RPC. On failure the stamp remains: restoring a
+			 * stale snapshot could silently erase a concurrent
+			 * _cifsFileInfo_put() close stamp.  readdir is suppressed
+			 * until the stamp expires; stat() bypasses this via the
+			 * from_readdir=false path in is_size_safe_to_change() and
+			 * always returns an authoritative QUERY_INFO result.
+			 * Pairs with smp_load_acquire() in is_size_safe_to_change().
+			 */
+			smp_store_release(&cifsInode->time_last_write, jiffies);
+		}
 		rc = cifs_file_set_size(xid, direntry, full_path,
 					open_file, attrs->ia_size);
 		if (rc != 0)
@@ -3279,8 +3291,6 @@ cifs_setattr_unix(struct dentry *direntry, struct iattr *attrs)
 
 	if ((attrs->ia_valid & ATTR_SIZE) &&
 	    attrs->ia_size != i_size_read(inode)) {
-		/* Pairs with smp_load_acquire() in is_size_safe_to_change(). */
-		smp_store_release(&cifsInode->time_last_write, jiffies);
 		truncate_setsize(inode, attrs->ia_size);
 		netfs_resize_file(&cifsInode->netfs, attrs->ia_size, true);
 		fscache_resize_cookie(cifs_inode_cookie(inode), attrs->ia_size);
@@ -3370,6 +3380,17 @@ cifs_setattr_nounix(struct dentry *direntry, struct iattr *attrs)
 	}
 
 	if (attrs->ia_valid & ATTR_SIZE) {
+		if (attrs->ia_size != i_size_read(inode)) {
+			/* Stamp before RPC. On failure the stamp remains: restoring a
+			 * stale snapshot could silently erase a concurrent
+			 * _cifsFileInfo_put() close stamp.  readdir is suppressed
+			 * until the stamp expires; stat() bypasses this via the
+			 * from_readdir=false path in is_size_safe_to_change() and
+			 * always returns an authoritative QUERY_INFO result.
+			 * Pairs with smp_load_acquire() in is_size_safe_to_change().
+			 */
+			smp_store_release(&cifsInode->time_last_write, jiffies);
+		}
 		rc = cifs_file_set_size(xid, direntry, full_path,
 					cfile, attrs->ia_size);
 		if (rc != 0)
@@ -3482,8 +3503,6 @@ cifs_setattr_nounix(struct dentry *direntry, struct iattr *attrs)
 
 	if ((attrs->ia_valid & ATTR_SIZE) &&
 	    attrs->ia_size != i_size_read(inode)) {
-		/* Pairs with smp_load_acquire() in is_size_safe_to_change(). */
-		smp_store_release(&cifsInode->time_last_write, jiffies);
 		truncate_setsize(inode, attrs->ia_size);
 		netfs_resize_file(&cifsInode->netfs, attrs->ia_size, true);
 		fscache_resize_cookie(cifs_inode_cookie(inode), attrs->ia_size);
diff --git a/fs/smb/client/misc.c b/fs/smb/client/misc.c
index 6edebc0807ea..46e1382e8e04 100644
--- a/fs/smb/client/misc.c
+++ b/fs/smb/client/misc.c
@@ -525,24 +525,11 @@ cifs_close_deferred_file(struct cifsInodeInfo *cifs_inode)
 	}
 	spin_unlock(&cifs_inode->open_file_lock);
 
-	if (failed_cfile) {
-		if (OPEN_FMODE(failed_cfile->f_flags) & FMODE_WRITE) {
-			/* Pairs with smp_load_acquire() in is_size_safe_to_change(). */
-			smp_store_release(&CIFS_I(d_inode(failed_cfile->dentry))->time_last_write,
-					  jiffies);
-		}
+	if (failed_cfile)
 		_cifsFileInfo_put(failed_cfile, false, false);
-	}
 
 	list_for_each_entry_safe(tmp_list, tmp_next_list, &file_head, list) {
-		struct cifsFileInfo *cfile = tmp_list->cfile;
-
-		if (OPEN_FMODE(cfile->f_flags) & FMODE_WRITE) {
-			/* Pairs with smp_load_acquire() in is_size_safe_to_change(). */
-			smp_store_release(&CIFS_I(d_inode(cfile->dentry))->time_last_write,
-					  jiffies);
-		}
-		_cifsFileInfo_put(cfile, false, false);
+		_cifsFileInfo_put(tmp_list->cfile, false, false);
 		list_del(&tmp_list->list);
 		kfree(tmp_list);
 	}
@@ -576,24 +563,11 @@ cifs_close_all_deferred_files(struct cifs_tcon *tcon)
 	}
 	spin_unlock(&tcon->open_file_lock);
 
-	if (failed_cfile) {
-		if (OPEN_FMODE(failed_cfile->f_flags) & FMODE_WRITE) {
-			/* Pairs with smp_load_acquire() in is_size_safe_to_change(). */
-			smp_store_release(&CIFS_I(d_inode(failed_cfile->dentry))->time_last_write,
-					  jiffies);
-		}
+	if (failed_cfile)
 		_cifsFileInfo_put(failed_cfile, true, false);
-	}
 
 	list_for_each_entry_safe(tmp_list, tmp_next_list, &file_head, list) {
-		struct cifsFileInfo *cfile = tmp_list->cfile;
-
-		if (OPEN_FMODE(cfile->f_flags) & FMODE_WRITE) {
-			/* Pairs with smp_load_acquire() in is_size_safe_to_change(). */
-			smp_store_release(&CIFS_I(d_inode(cfile->dentry))->time_last_write,
-					  jiffies);
-		}
-		_cifsFileInfo_put(cfile, true, false);
+		_cifsFileInfo_put(tmp_list->cfile, true, false);
 		list_del(&tmp_list->list);
 		kfree(tmp_list);
 	}
@@ -663,24 +637,11 @@ void cifs_close_deferred_file_under_dentry(struct cifs_tcon *tcon,
 	}
 	spin_unlock(&tcon->open_file_lock);
 
-	if (failed_cfile) {
-		if (OPEN_FMODE(failed_cfile->f_flags) & FMODE_WRITE) {
-			/* Pairs with smp_load_acquire() in is_size_safe_to_change(). */
-			smp_store_release(&CIFS_I(d_inode(failed_cfile->dentry))->time_last_write,
-					  jiffies);
-		}
+	if (failed_cfile)
 		_cifsFileInfo_put(failed_cfile, true, false);
-	}
 
 	list_for_each_entry_safe(tmp_list, tmp_next_list, &file_head, list) {
-		struct cifsFileInfo *cfile = tmp_list->cfile;
-
-		if (OPEN_FMODE(cfile->f_flags) & FMODE_WRITE) {
-			/* Pairs with smp_load_acquire() in is_size_safe_to_change(). */
-			smp_store_release(&CIFS_I(d_inode(cfile->dentry))->time_last_write,
-					  jiffies);
-		}
-		_cifsFileInfo_put(cfile, true, false);
+		_cifsFileInfo_put(tmp_list->cfile, true, false);
 		list_del(&tmp_list->list);
 		kfree(tmp_list);
 	}
diff --git a/fs/smb/client/smb1maperror.c b/fs/smb/client/smb1maperror.c
index ab3d09613c91..395299f9121b 100644
--- a/fs/smb/client/smb1maperror.c
+++ b/fs/smb/client/smb1maperror.c
@@ -234,11 +234,7 @@ int __init smb1_init_maperror(void)
 	if (rc)
 		return rc;
 
-	rc = mapping_table_ERRSRV_is_sorted();
-	if (rc)
-		return rc;
-
-	return rc;
+	return mapping_table_ERRSRV_is_sorted();
 }
 
 #if IS_ENABLED(CONFIG_SMB1_KUNIT_TESTS)
diff --git a/fs/smb/client/smb2ops.c b/fs/smb/client/smb2ops.c
index cbd51a08e97e..192649fec25d 100644
--- a/fs/smb/client/smb2ops.c
+++ b/fs/smb/client/smb2ops.c
@@ -4745,10 +4745,10 @@ smb3_init_transform_rq(struct TCP_Server_Info *server, int num_rqst,
 			size_t cur_size = 0;
 			rc = netfs_alloc_folioq_buffer(NULL, &buffer, &cur_size,
 						       size, GFP_NOFS);
+			new->rq_buffer = buffer;
 			if (rc < 0)
 				goto err_free;
 
-			new->rq_buffer = buffer;
 			iov_iter_folio_queue(&new->rq_iter, ITER_SOURCE,
 					     buffer, 0, 0, size);