I tried out the patch and it does fix getting STATUS_INVALID_PARAMETER
error from the server, but doesn't fix the issue of getting EINVAL
Traceback (most recent call last):
File "/root/hl-test.py", line 15, in <module>
newfd = os.open('new', os.O_RDONLY|os.O_DIRECT)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
OSError: [Errno 22] Invalid argument: 'new'
It is fixed by running with leases disable (via mount parm), but
wouldn't it be better to fix the error so apps don't break. Ideas?
On Mon, Mar 17, 2025 at 3:41 AM Chunjie Zhu <chunjie.zhu@xxxxxxxxx> wrote:
>
> The following Python script results in unexpected behaviour when run on
> a CIFS filesystem against a Windows Server:
>
> # 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. The root cause of this is that the client
> caches lease keys per inode, but the spec requires them to be related to
> the filename which causes problems when hard links are involved:
>
> From MS-SMB2 section 3.3.5.9.11:
>
> "The server MUST attempt to locate a Lease by performing a lookup in the
> LeaseTable.LeaseList using the LeaseKey in the
> SMB2_CREATE_REQUEST_LEASE_V2 as the lookup key. If a lease is found,
> Lease.FileDeleteOnClose is FALSE, and Lease.Filename does not match the
> file name for the incoming request, the request MUST be failed with
> STATUS_INVALID_PARAMETER"
>
> The client side can return EINVAL directly without invoking server
> operations. This reduces client server network communication overhead.
>
> Signed-off-by: Chunjie Zhu <chunjie.zhu@xxxxxxxxx>
> ---
> fs/smb/client/cifsproto.h | 2 ++
> fs/smb/client/file.c | 29 +++++++++++++++++++++++++++++
> 2 files changed, 31 insertions(+)
>
> diff --git a/fs/smb/client/cifsproto.h b/fs/smb/client/cifsproto.h
> index 260a6299bddb..b563c227792e 100644
> --- a/fs/smb/client/cifsproto.h
> +++ b/fs/smb/client/cifsproto.h
> @@ -157,6 +157,8 @@ extern int cifs_get_writable_path(struct cifs_tcon *tcon, const char *name,
> extern struct cifsFileInfo *find_readable_file(struct cifsInodeInfo *, bool);
> extern int cifs_get_readable_path(struct cifs_tcon *tcon, const char *name,
> struct cifsFileInfo **ret_file);
> +extern int cifs_get_hardlink_path(struct cifs_tcon *tcon, struct inode *inode,
> + struct file *file);
> extern unsigned int smbCalcSize(void *buf);
> extern int decode_negTokenInit(unsigned char *security_blob, int length,
> struct TCP_Server_Info *server);
> diff --git a/fs/smb/client/file.c b/fs/smb/client/file.c
> index 4cbb5487bd8d..0a66cce6e0ff 100644
> --- a/fs/smb/client/file.c
> +++ b/fs/smb/client/file.c
> @@ -751,6 +751,12 @@ int cifs_open(struct inode *inode, struct file *file)
> } else {
> _cifsFileInfo_put(cfile, true, false);
> }
> + } else {
> + /* hard link on the defeered close file */
> + rc = cifs_get_hardlink_path(tcon, inode, file);
> + if (rc) {
> + goto out;
> + }
> }
>
> if (server->oplocks)
> @@ -2413,6 +2419,29 @@ cifs_get_readable_path(struct cifs_tcon *tcon, const char *name,
> return -ENOENT;
> }
>
> +int
> +cifs_get_hardlink_path(struct cifs_tcon *tcon, struct inode *inode,
> + struct file *file)
> +{
> + struct cifsFileInfo *open_file = NULL;
> + struct cifsInodeInfo *cinode = CIFS_I(inode);
> + int rc = 0;
> +
> + spin_lock(&tcon->open_file_lock);
> + spin_lock(&cinode->open_file_lock);
> +
> + list_for_each_entry(open_file, &cinode->openFileList, flist) {
> + if (file->f_flags == open_file->f_flags) {
> + rc = -EINVAL;
> + break;
> + }
> + }
> +
> + spin_unlock(&cinode->open_file_lock);
> + spin_unlock(&tcon->open_file_lock);
> + return rc;
> +}
> +
> void
> cifs_writedata_release(struct kref *refcount)
> {
> --
> 2.34.1
>
>
--
Thanks,
Steve
