Re: [PATCH v2] smb: client: fix data loss due to broken rename(2)

[Paulo Alcantara <pc@manguebit.org>]

Ralph Boehme <slow@samba.org> writes:

> Hi Paulo,
>
> On 9/2/25 9:09 PM, Paulo Alcantara wrote:
>> Ralph Boehme <slow@samba.org> writes:
>> 
>>> Why not simply fail the rename instead of trying to implement some
>>> clever but complex and error prone fallback?
>> 
>> We're doing this for SMB1 for a very long time and haven't heard of any
>> issues so far.  I've got a "safer" version [1] that does everything a
>> single compound request but then implemented this non-compound version
>> due to an existing Azure bug that seems to limit the compound in 4
>> commands, AFAICT.  Most applications depend on such behavior working,
>> which is renaming open files.
>
> maybe I'm barking of the wrong tree, but you *can* rename open files:
>
> $ bin/smbclient -U 'USER%PASS' //IP/C\$
> smb: \> cd Users\administrator.WINCLUSTER\Desktop\
> smb: \Users\administrator.WINCLUSTER\Desktop\> open 
> t-ph-oplock-b-downgraded-s.cab
> open file 
> \Users\administrator.WINCLUSTER\Desktop\t-ph-oplock-b-downgraded-s.cab: 
> for read/write fnum 1
> smb: \Users\administrator.WINCLUSTER\Desktop\> rename 
> t-ph-oplock-b-downgraded-s.cab renamed
> smb: \Users\administrator.WINCLUSTER\Desktop\>
>
> ...given the open is with SHARE_DELETE (had to tweak smbclient to 
> actually allow that).

Interesting.

cifs.ko will get STATUS_ACCESS_DENIED when attempting to rename the open
file.  The file was open with SHARE_READ|SHARE_WRITE|SHARE_DELETE, BTW.

Also, note that cifs.ko will not reuse the open handle, rather it will
send a compound request of create+set_info(rename)+close to the file
which will fail with STATUS_ACCESS_DENIED.

What am I missing?

> If the rename destination is open and the server rightly fails the 
> rename for that reason, then masking that error is a mistake imho.
>
> When doing
>
> $ mv a b
>
> the user asked to rename a, he did NOT ask to rename b which becomes 
> important, because if you do
>
> rename("b", ".renamehackXXXX")
>
> under the hood and then reattempt the rename
>
> rename("a", "b")
>
> and then the user subsequently does
>
> $ mv b ..
> $ cd ..
> $ rmdir DIR
>
> where DIR is the directory all of the above was performed inside, the 
> rmdir will fail with ENOTEMPTY and *now* the user is confused.

Yes, I understand your point.  That's really confusing.

How can we resolve above cases without performing the silly renames
then?