Re: [PATCH] cifs: Close deferred files that may be open via hard links

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 5/16/2023 11:01 AM, Ross Lagerwall wrote:
Windows Server (tested with 2016) disallows opening the same inode via
two different hardlinks at the same time. With deferred closes, this can
result in unexpected behaviour as the following Python snippet shows:

     # Create file
     fd = os.open('test', os.O_WRONLY|os.O_CREAT)
     os.write(fd, b'foo')
     os.close(fd)

     # Open and close the file to leave a pending deferred close
     fd = os.open('test', os.O_RDONLY|os.O_DIRECT)
     os.close(fd)

     # Try to open the file via a hard link
     os.link('test', 'new')
     newfd = os.open('new', os.O_RDONLY|os.O_DIRECT)

The final open returns EINVAL due to the server returning
STATUS_INVALID_PARAMETER.

Good sleuthing. The hardlink behavior is slightly surprising, but
it is certainly the case that certain incompatible combinations of
open/create flags can conflict with other handles, including
cached opens.

Fix this by closing any deferred closes that may be open via other hard
links if we haven't successfully reused a cached handle.

Fixes: c3f207ab29f7 ("cifs: Deferred close for files")
Signed-off-by: Ross Lagerwall <ross.lagerwall@xxxxxxxxxx>
---

This is kind of an RFC. Is the server doing the wrong thing? Is it
correct to work around this in the client?

By definition the server is doing what it does, and it's a moot
question whether it's appropriate to work around it. However, I don't
see this behavior explicitly stated in MS-FSA. And INVALID_PARAMETER
is a strange error code to return for this. Do you have a packet
trace? We should ask Microsoft.

  fs/cifs/file.c | 1 +
  1 file changed, 1 insertion(+)

diff --git a/fs/cifs/file.c b/fs/cifs/file.c
index c5fcefdfd797..723cbc060f57 100644
--- a/fs/cifs/file.c
+++ b/fs/cifs/file.c
@@ -749,6 +749,7 @@ int cifs_open(struct inode *inode, struct file *file)
  			_cifsFileInfo_put(cfile, true, false);
  		}
  	}
+	cifs_close_deferred_file(CIFS_I(inode));

But, is this correct? I don't believe this function blocks. And I'm
not sure this triggers the close only in the case you've identified.

Tom.

  	if (server->oplocks)
  		oplock = REQ_OPLOCK;



[Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux