mirror of
https://github.com/torvalds/linux.git
synced 2026-10-13 22:01:02 +02:00
smb/client: send lease break ACKs thru correct session for multiuser mounts
Currently, when cifs_oplock_break handles a break request from the server it searches for the appropriate tlink to handle the request but incorrectly uses the current fsuid as the search key, eventually causing read errors for users with multiuser mounts on NetApp. Fix this by using the tlink from the cfile struct instead to respond through the correct session. As breaks are handled in a worker thread, the current fsuid isn't guaranteed to match the session that the break is intended for. This means that cifs_sb_tlink may search the rbtree using the wrong fsuid, and return a tlink with an incorrect session than the lease break was intended for. As a result, the breaks may be ACKed through an incorrect session. While it seems that Samba/Windows Servers 2016-2025 ignore this as long as the lease key is correct, we ran into a case where if you're using NetApp ONTAP or Azure NetApp Files they will reject the ACK and return `STATUS_LOCK_NOT_GRANTED` errors on any future read requests a user may initiate through their still held open file handle, and the server will eventually close the file. In the dmesg logs, the user may see errors like these: CIFS: Status code returned 0xc0000128 STATUS_FILE_CLOSED CIFS: VFS: Send error in read = -9 With a multiuser mount using NetApp, this issue is really easy for users to hit on a wide variety of kernel versions by attempting to copy a file from the share to the local machine through GNOME Files/Nautilus. This copy will always result in Nautilus throwing a `Bad File Descriptor` error to the user and fail. With this fix, you can copy files through Nautilus without issue. >From looking at the traces, it seems that glib will open the file first, and call listxattr before actually attempting to copy the file data. The listxattr call always triggers a break, causing the copy to fail. The proposed fix returns to the way the client grabbed the tlink before commite8f5f849ff("cifs: fix potential oops in cifs_oplock_break"). The bulk of that commit (checking for list empty) remains untouched, and I think the change to using cifs_sb_tlink was intended to avoid a NULL/ERR deference on the tlink as well as update the reference count. I believe this fix should preserve those safety properties, but of course I'd appreciate any corrections here. Fixes:e8f5f849ff("cifs: fix potential oops in cifs_oplock_break") Cc: stable@vger.kernel.org Signed-off-by: April Cardenas <april.cardenas@canonical.com> Reviewed-by: Namjae Jeon <linkinjeon@kernel.org> Reviewed-by: Bharath S M <bharathsm@microsoft.com> Signed-off-by: Paulo Alcantara <pc@manguebit.org>
This commit is contained in:
parent
23c240d950
commit
ebc5660132
|
|
@ -3354,8 +3354,8 @@ void cifs_oplock_break(struct work_struct *work)
|
|||
wait_on_bit(&cinode->flags, CIFS_INODE_PENDING_WRITERS,
|
||||
TASK_UNINTERRUPTIBLE);
|
||||
|
||||
tlink = cifs_sb_tlink(cifs_sb);
|
||||
if (IS_ERR(tlink)) {
|
||||
tlink = cifs_get_tlink(cfile->tlink);
|
||||
if (IS_ERR_OR_NULL(tlink)) {
|
||||
/* drop the reference taken when the break was queued */
|
||||
_cifsFileInfo_put(cfile, false /* do not wait for ourself */, false);
|
||||
goto out;
|
||||
|
|
|
|||
Loading…
Reference in New Issue
Block a user