Message-ID: <20250902163053.zzgd5ee4qguciajj@pali>
Date: Tue, 2 Sep 2025 18:30:53 +0200
From: Pali Rohár <pali@...nel.org>
To: Stefan Metzmacher <metze@...ba.org>
Cc: Steve French <sfrench@...ba.org>, Paulo Alcantara <pc@...guebit.com>,
	ronnie sahlberg <ronniesahlberg@...il.com>,
	Ralph Böhme <slow@...ba.org>,
	linux-cifs@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 00/35] cifs: Fix SMB rmdir() and unlink() against Windows
 SMB servers

Hello!

On Tuesday 02 September 2025 17:17:14 Stefan Metzmacher wrote:
> Hi Pali,
> 
> > > > This patch series improves Linux rmdir() and unlink() syscalls called on
> > > > SMB mounts exported from Windows SMB servers which do not implement
> > > > POSIX semantics of the file and directory removal.
> > > > 
> > > > This patch series should have no impact and no function change when
> > > > communicating with the POSIX SMB servers, as they should implement
> > > > proper rmdir and unlink logic.
> > > 
> > > Please note that even servers implementing posix/unix extensions,
> > > may also have windows clients connected operating on the same files/directories.
> > > And in that case even posix clients will see the windows behaviour
> > > of DELETE_PENDING for set disposition or on rename
> > > NT_STATUS_ACCESS_DENIED or NT_STATUS_DIRECTORY_NOT_EMPTY.
> > 
> > Ok. So does it mean that the issue described here applies also for POSIX
> > SMB server?
> 
> I guess so.
> 
> > If yes, then I would propose to first fix the problem with
> > Windows/non-POSIX SMB server and then with others. So it is not too big.
> 
> That's up to Steve. But isn't it just a matter of removing the
> if statement that checks for posix?

I can modify that if statement, no problem. I just did not wanted to
touch POSIX code path as my focus is on the Windows code path.
I do not know all those case which POSIX paths against POSIX server may
trigger, so it was safer for me to let POSIX as is.

> > > > When issuing remove path command against non-POSIX / Windows SMB server,
> > > > it let the directory entry which is being removed in the directory until
> > > > all users / clients close all handles / references to that path.
> > > > 
> > > > POSIX requires from rmdir() and unlink() syscalls that after successful
> > > > call, the requested path / directory entry is released and allows to
> > > > create a new file or directory with that name. This is currently not
> > > > working against non-POSIX / Windows SMB servers.
> > > > 
> > > > To workaround this problem fix and improve existing cifs silly rename
> > > > code and extend it also to SMB2 and SMB3 dialects when communicating
> > > > with Windows SMB servers. Silly rename is applied only when it is
> > > > necessary (when some other client has opened file or directory).
> > > > If no other client has the file / dir open then silly rename is not
> > > > used.
> > > 
> > > If I 'git grep -i silly fs/smb/client' there's no hit, can you
> > > please explain what code do you mean with silly rename?
> > 
> > Currently (without this patch series) it is CIFSSMBRenameOpenFile()
> > function when called with NULL as 3rd argument.
> > 
> > Cleanup is done in PATCH 11/35, where are more details.
> > 
> > Originally the "Silly rename" is the term used by NFS client, when it
> > does rename instead of unlink when the path is in use.
> > I reused this term.
> > 
> > 
> > So for SMB this "silly rename" means:
> > - open path with DELETE access and get its handle
> > - rename path (via opened handle) to some unique (auto generated) name
> > - set delete pending state on the path (via opened handle)
> > - close handle
> > 
> > (plus some stuff around to remove READ_ONLY attr which may disallow to
> > open path with DELETE ACCESS)
> > 
> > So above silly rename means that the original path is not occupied
> > anymore (thanks to rename) and the original file / dir is removed after
> > all clients / users release handles (thanks to set delete pending).
> > 
> > It is clear now clear? Or do you need to explain some other steps?
> > Sometimes some parts are too obvious for me and I unintentionally omit
> > description for something which is important. And seems that this is
> > such case. So it is my mistake, I should have explain it better.
> 
> I think I understand what it tries to do, thanks for explaining.
> 
> I was just wondering why the rename on a busy handle would work
> while delete won't work. I'd guess the chances are high that both fail.

Both "set delete pending" and "rename" operations are working (if open
pass). Just "set delete pending" does not unlink file/dir immediately
but rather wait until path is not busy anymore. "rename" on the other
hand is executed immediately.

So we can rename the in-use/busy file on Windows server, but we cannot
remove it immediately. We can only "schedule" its removal on the Windows
server. So combination of "rename" + "schedule removal" is what are
patches doing.

In case open fails (e.g. due to conflicting DENY shared reservations or
because path is already in delete pending state) then obviously it is
not possible to continue with either rename or set delete pending
operation, both then fails.

> Do you have network captures showing the old and new behavior
> that's often easier to understand than looking at patches alone.
> 
> metze

I do not have them right now, but I can run test scenario and capture
them, this is not problem. Test case is pretty straightforward.

Powered by blists - more mailing lists