Bug 1694201

the upstream client is also broken, but in a different way.  Instead of an smb request, it appears to send some invalid bytes, followed by an smb request (which may or may not be the same as what rhel 7 sends).  Instead of closing the connection with FIN, the server sends RST.

However, the upstream client still loops forever, with the 'mv' operation hanging (just with different invalid communication).

I can confirm that the upstream client does send the same 'NT Create AndX Request', however the NBSS header is repeated, making the payload invalid.

Here is the tcp payload sent by the upstream client:

0000   00 00 00 5d 00 00 00 5d ff 53 4d 42 a2 00 00 00  ...]...].SMB....
0010   00 00 01 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
0020   5e 15 77 09 df 6a 21 03 18 ff 00 00 00 00 0a 00  ^.w..j!.........
0030   00 00 00 00 00 00 00 00 00 00 01 00 00 00 00 00  ................
0040   00 00 00 00 80 00 00 00 07 00 00 00 01 00 00 00  ................
0050   40 00 00 00 02 00 00 00 03 0a 00 5c 74 65 73 74  @..........\test
0060   66 69 6c 64 00                                   file.

the first 4 bytes decode as nbss:
NetBIOS Session Service
    Message Type: Session message (0x00)
    Flags: 0x00
    Length: 0x005d

However, note that these 4 bytes are then repeated:
0000   00 00 00 5d 00 00 00 5d

Also, due to the addition of these 4 bytes, the nbss encapsulates 4 fewer bytes at the end, so the filename ends with 'testf', and there are 4 extra bytes of tcp payload at the end.

I modified the frame to get rid of the additional 4 bytes of header, and it decodes as valid SMB again:
3231 97.377091804 192.168.122.150 → 192.168.122.71 SMB 167 NT Create AndX Request, Path: \testfile 

Sending SMB over the SMB2 session is still invalid, however this does explain the difference between RHEL 7 and upstream behavior.  For some reason, in addition to switching from smb2 to smb, it adds a second header.

in cifs_do_rename(), we first try to call the smb-version-specific path-based rename function, then later try to open the file using the function CIFS_open()


static int
cifs_do_rename(const unsigned int xid, struct dentry *from_dentry,
               const char *from_path, struct dentry *to_dentry,
               const char *to_path)
{
...

        /* try path-based rename first */
        rc = server->ops->rename(xid, tcon, from_path, to_path, cifs_sb);
...
        /* open the file to be renamed -- we need DELETE perms */
        oparms.desired_access = DELETE;
        oparms.create_options = CREATE_NOT_DIR;
        oparms.disposition = FILE_OPEN;
        oparms.path = from_path;
        oparms.fid = &fid;
        oparms.reconnect = false;

...
        rc = CIFS_open(xid, &oparms, &oplock, NULL);
        if (rc == 0) {
                rc = CIFSSMBRenameOpenFile(xid, tcon, fid.netfid,
                                (const char *) to_dentry->d_name.name,
                                cifs_sb->local_nls, cifs_remap(cifs_sb));
                CIFSSMBClose(xid, tcon, fid.netfid);

However, CIFS_open, CIFSSMBRenameOpenFile, and CIFSSMBClose are all smb1-specific


in CIFS_open():
        rc = smb_init(SMB_COM_NT_CREATE_ANDX, 24, tcon, (void **)&req,

in CIFSSMBRenameOpenFile():
        rc = smb_init(SMB_COM_TRANSACTION2, 15, pTcon, (void **) &pSMB,

in CIFSSMBClose():
        rc = small_smb_init(SMB_COM_CLOSE, 3, tcon, (void **) &pSMB);


So if the 'path-based rename' fails while using a higher smb version, it falls back to using smb v1 calls, which is clearly wrong.



smb1ops.c:struct smb_version_operations smb1_operations = {
smb1ops.c:	.rename_pending_delete = cifs_rename_pending_delete,
smb1ops.c:	.rename = CIFSSMBRename,
smb1ops.c:	.open = cifs_open_file,
smb1ops.c:	.close = cifs_close_file,
smb1ops.c:	.close_dir = cifs_close_dir,

smb2ops.c:struct smb_version_operations smb20_operations = {
smb2ops.c:	.rename = smb2_rename_path,
smb2ops.c:	.open = smb2_open_file,
smb2ops.c:	.close = smb2_close_file,
smb2ops.c:	.close_dir = smb2_close_dir,

smb2ops.c:struct smb_version_operations smb21_operations = {
smb2ops.c:	.rename = smb2_rename_path,
smb2ops.c:	.open = smb2_open_file,
smb2ops.c:	.close = smb2_close_file,
smb2ops.c:	.close_dir = smb2_close_dir,

smb2ops.c:struct smb_version_operations smb30_operations = {
smb2ops.c:	.rename = smb2_rename_path,
smb2ops.c:	.open = smb2_open_file,
smb2ops.c:	.close = smb2_close_file,
smb2ops.c:	.close_dir = smb2_close_dir,

smb2ops.c:struct smb_version_operations smb311_operations = {
smb2ops.c:	.rename = smb2_rename_path,
smb2ops.c:	.open = smb2_open_file,
smb2ops.c:	.close = smb2_close_file,
smb2ops.c:	.close_dir = smb2_close_dir,


so it seems like the open in cifs_do_rename should really look like this:

    rc = server->ops->open(xid, &oparms, &oplock, NULL);

and the CIFSSMBClose replaced:
    server->ops->close(xid, tcon, fid.netfid);

there is no replacement for CIFSSMBRenameOpenFile ... can this be done using smb2 semantics?

*****

and finally, when trying to rename the open file on an smb1 mount, the operation fails, which seems like it may be the correct response anyway:
11358 07:06:54.545544 rename("/mnt/vm3_a/testfile", "/mnt/vm3_a/testfile.bak") = -1 EBUSY (Device or resource busy) <1.925719>

the 1-2 second delay is expected; in the samba code (for smb1) (I believe there may be 2 waits)

source3/include/local.h:
/* Number of microseconds to wait before a sharing violation. */
#define SHARING_VIOLATION_USEC_WAIT 950000

During the recent big credits cleanup in the smb2 codebase we have recently fixed a handful of similar issues.
But these changes can not easily be backported as they build on and depend on a lot of other unrelated changes.

However, there is a workaround. I suggest for customers that suffer this, then can switch back to vers=1.0
until they get a chance to upgrade to rhel8.

Created attachment 1555290 [details]
patch to prevent falback to smb

A path-based rename returning EBUSY will incorrectly try opening
the file with a cifs (NT Create AndX) operation on an smb2+ mount,
which causes the server to force a session close.

If the mount is smb2+, skip the fallback.

The credits cleanup did not touch this, and does not fix it.

The attached patch fixes this bug, and applies cleanly to both upstream and RHEL7.

How hard would it be to do an xfstest from the reproducer provided?

I think the patch will go upstream just fine, but Steve French rightly suggested the should be something in xfstests

upstream posting for the patch - https://www.spinics.net/lists/linux-cifs/msg16914.html

Steve French responded privately regarding the xfstests

fix is committed upstream and in -stable branches

commit 652727bbe1b17993636346716ae5867627793647
Author: Frank Sorenson <sorenson>
Date:   2019-04-16 08:37:27 -0500

    cifs: do not attempt cifs operation on smb2+ rename error
    
    A path-based rename returning EBUSY will incorrectly try opening
    the file with a cifs (NT Create AndX) operation on an smb2+ mount,
    which causes the server to force a session close.
    
    If the mount is smb2+, skip the fallback.
    
    Signed-off-by: Frank Sorenson <sorenson>
    Signed-off-by: Steve French <stfrench>
    CC: Stable <stable.org>
    Reviewed-by: Ronnie Sahlberg <lsahlber>

Reproducer can be done on a single system, acting as both smb server and cifs client.

specific share, user, credentials, etc. are unimportant.


setup:
    # mount -ocredentials=/root/.user1_smb_creds,vers=2.0 //localhost/user1 /mnt/tmp
    # touch /mnt/tmp/testfile


in terminal 1:
    # smbclient -A /root/.user1_smb_creds //localhost/user1
    Try "help" to get a list of possible commands.
    smb: \> open testfile
    open file \testfile: for read/write fnum 1

in terminal 2:
    # mv /mnt/tmp/testfile /mnt/tmp/testfile.bak


If the bug exists, the 'mv' command will hang, and a packet capture will show the server responding to an open/create call with STATUS_SHARING_VIOLATION.  The client will then attempt to open the file using smb1 semantics, to which the server will close or reset the tcp connection.  The client will delay several seconds, reconnect the smb2 session, and re-attempt the smb1 open/Create.  This then repeats.

If the bug is fixed, the client will simply return an error (EBUSY) to the 'mv' command.

(In reply to Frank Sorenson from comment #12)
> Reproducer can be done on a single system, acting as both smb server and
> cifs client.
> 
> specific share, user, credentials, etc. are unimportant.
> 
> 
> setup:
>     # mount -ocredentials=/root/.user1_smb_creds,vers=2.0 //localhost/user1
> /mnt/tmp
>     # touch /mnt/tmp/testfile
> 
> 
> in terminal 1:
>     # smbclient -A /root/.user1_smb_creds //localhost/user1
>     Try "help" to get a list of possible commands.
>     smb: \> open testfile
>     open file \testfile: for read/write fnum 1
> 
> in terminal 2:
>     # mv /mnt/tmp/testfile /mnt/tmp/testfile.bak
> 
> 
> If the bug exists, the 'mv' command will hang, and a packet capture will
> show the server responding to an open/create call with
> STATUS_SHARING_VIOLATION.  The client will then attempt to open the file
> using smb1 semantics, to which the server will close or reset the tcp
> connection.  The client will delay several seconds, reconnect the smb2
> session, and re-attempt the smb1 open/Create.  This then repeats.
> 
> If the bug is fixed, the client will simply return an error (EBUSY) to the
> 'mv' command.

Thanks Frank very much! I'll try to get it to xfstests.

(In reply to Frank Sorenson from comment #12)
> Reproducer can be done on a single system, acting as both smb server and
> cifs client.
> 
> specific share, user, credentials, etc. are unimportant.

smbclient open is needed, isn't it? I tried opening it in shell and failed to reproduce.

> If the bug exists, the 'mv' command will hang,

In my test, 'mv' hangs for a few minutes and returns EAGAIN eventually:

(smbv1 returns EBUSY.)

[root@ibm-x3850x5-03 ~]# uname -r
3.10.0-1059.el7.x86_64
[root@ibm-x3850x5-03 ~]# mount //localhost/test /cifsmnt -o vers=2.0,credentials=/root/smb1.creds
[root@ibm-x3850x5-03 ~]# mount | grep cifs
//localhost/test on /cifsmnt type cifs (rw,relatime,vers=2.0,cache=strict,username=root,domain=IBM-X3850X5-03,uid=0,noforceuid,gid=0,noforcegid,addr=0000:0000:0000:0000:0000:0000:0000:0001,file_mode=0755,dir_mode=0755,soft,nounix,serverino,mapposix,rsize=65536,wsize=65536,echo_interval=60,actimeo=1)
[root@ibm-x3850x5-03 ~]# time mv /cifsmnt/test /cifsmnt/test1
mv: cannot move ‘/cifsmnt/test’ to ‘/cifsmnt/test1’: Resource temporarily unavailable

real	0m40.115s
user	0m0.002s
sys	0m0.013s
[root@ibm-x3850x5-03 ~]# echo $?
1
[root@ibm-x3850x5-03 ~]# umount /cifsmnt
[root@ibm-x3850x5-03 ~]# 

root@ibm-x3850x5-03 ~]# mount //localhost/test /cifsmnt -o vers=1.0,credentials=/root/smb1.creds
[root@ibm-x3850x5-03 ~]# time mv /cifsmnt/test /cifsmnt/test1
mv: cannot move ‘/cifsmnt/test’ to ‘/cifsmnt/test1’: Device or resource busy

real	0m1.912s
user	0m0.000s
sys	0m0.006s
[root@ibm-x3850x5-03 ~]#

Patch(es) committed on kernel-3.10.0-1063.el7

This seems to hit many different customers based on the number of linked cases
so I think it would justify z-stream.

Since the problem described in this bug report should be
resolved in a recent advisory, it has been closed with a
resolution of ERRATA.

For information on the advisory, and where to find the updated
files, follow the link below.

If the solution does not work for you, open a new bug report.

https://access.redhat.com/errata/RHSA-2020:1016