Fix for the kernel client attached
On Tue, Nov 9, 2021 at 6:54 PM Jeremy Allison <jra@xxxxxxxxx> wrote:
>
> On Tue, Nov 09, 2021 at 10:26:59AM +0100, Julian Sikorski wrote:
> >Am 09.11.21 um 09:10 schrieb Steve French:
> >>Yes - here is a trivial reproducer (excuse the ugly sample cut-n-paste)
> >>
> >>#include <stdio.h>
> >>#include <stdlib.h>
> >>#include <unistd.h>
> >>#include <string.h>
> >>#include <fcntl.h>
> >>#include <sys/types.h>
> >>#include <sys/stat.h>
> >>
> >>int main(int argc, char *argv[]) {
> >>char *str = "Text to be added";
> >>int fd, ret, fsyncrc, fsyncr_rc, openrc, closerc, close2rc;
> >>
> >>fd = creat("test.txt", S_IWUSR | S_IRUSR);
> >>if (fd < 0) {
> >>perror("creat()");
> >>exit(1);
> >>}
> >>ret = write(fd, str, strlen(str));
> >>if (ret < 0) {
> >>perror("write()");
> >>exit(1);
> >>}
> >>openrc = open("test.txt", O_RDONLY);
> >> if (openrc < 0) {
> >> perror("creat()");
> >> exit(1);
> >> }
> >>fsyncr_rc = fsync(openrc);
> >>if (fsyncr_rc < 0)
> >>perror("fsync()");
> >>fsyncrc = fsync(fd);
> >>closerc = close(fd);
> >>close2rc = close(openrc);
> >>printf("read fsync rc=%d, write fsync rc=%d, close rc=%d, RO close
> >>rc=%d\n", fsyncr_rc, fsyncrc, closerc, close2rc);
> >>}
> >>
> >
> >I can confirm this fails on my machine without nostrictsync:
> >
> >$ ./test
> >
> >fsync(): Permission denied
> >
> >read fsync rc=-1, write fsync rc=0, close rc=0, RO close rc=0
> >
> >and works with nostrictsync:
> >
> >$ ./test
> >
> >read fsync rc=0, write fsync rc=0, close rc=0, RO close rc=0
> >
> >So is the bug in the Linux kernel?
>
> Yes, it's in the kernel cifsfs module which is forwarding an SMB_FLUSH request
> (which the spec says must fail on a non-writable handle) to
> a handle opened as non-writable. Steve hopefully will fix :-).
--
Thanks,
Steve
From da939857cdb6e25d0c967547f112eea07d847870 Mon Sep 17 00:00:00 2001
From: Steve French <stfrench@xxxxxxxxxxxxx>
Date: Wed, 10 Nov 2021 01:47:48 -0600
Subject: [PATCH] smb3: do not error on fsync when readonly
Linux allows doing a flush/fsync on a file open for read-only,
but the protocol does not allow that. If the file passed in
on the flush is read-only try to find a writeable handle for
the same inode, if that is not possible skip sending the
fsync call to the server to avoid breaking the apps.
Reported-by: Julian Sikorski <belegdol@xxxxxxxxx>
Suggested-by: Jeremy Allison <jra@xxxxxxxxx>
Signed-off-by: Steve French <stfrench@xxxxxxxxxxxxx>
---
fs/cifs/file.c | 35 +++++++++++++++++++++++++++++------
1 file changed, 29 insertions(+), 6 deletions(-)
diff --git a/fs/cifs/file.c b/fs/cifs/file.c
index 1b855fcb179e..fed30ba56d5f 100644
--- a/fs/cifs/file.c
+++ b/fs/cifs/file.c
@@ -2692,12 +2692,23 @@ int cifs_strict_fsync(struct file *file, loff_t start, loff_t end,
tcon = tlink_tcon(smbfile->tlink);
if (!(cifs_sb->mnt_cifs_flags & CIFS_MOUNT_NOSSYNC)) {
server = tcon->ses->server;
- if (server->ops->flush)
- rc = server->ops->flush(xid, tcon, &smbfile->fid);
- else
+ if (server->ops->flush == NULL) {
rc = -ENOSYS;
+ goto strict_fsync_exit;
+ }
+
+ if ((OPEN_FMODE(smbfile->f_flags) & FMODE_WRITE) == 0) {
+ smbfile = find_writable_file(CIFS_I(inode), FIND_WR_ANY);
+ if (smbfile) {
+ rc = server->ops->flush(xid, tcon, &smbfile->fid);
+ cifsFileInfo_put(smbfile);
+ } else
+ cifs_dbg(FYI, "ignore fsync for file not open for write\n");
+ } else
+ rc = server->ops->flush(xid, tcon, &smbfile->fid);
}
+strict_fsync_exit:
free_xid(xid);
return rc;
}
@@ -2709,6 +2720,7 @@ int cifs_fsync(struct file *file, loff_t start, loff_t end, int datasync)
struct cifs_tcon *tcon;
struct TCP_Server_Info *server;
struct cifsFileInfo *smbfile = file->private_data;
+ struct inode *inode = file_inode(file);
struct cifs_sb_info *cifs_sb = CIFS_FILE_SB(file);
rc = file_write_and_wait_range(file, start, end);
@@ -2725,12 +2737,23 @@ int cifs_fsync(struct file *file, loff_t start, loff_t end, int datasync)
tcon = tlink_tcon(smbfile->tlink);
if (!(cifs_sb->mnt_cifs_flags & CIFS_MOUNT_NOSSYNC)) {
server = tcon->ses->server;
- if (server->ops->flush)
- rc = server->ops->flush(xid, tcon, &smbfile->fid);
- else
+ if (server->ops->flush == NULL) {
rc = -ENOSYS;
+ goto fsync_exit;
+ }
+
+ if ((OPEN_FMODE(smbfile->f_flags) & FMODE_WRITE) == 0) {
+ smbfile = find_writable_file(CIFS_I(inode), FIND_WR_ANY);
+ if (smbfile) {
+ rc = server->ops->flush(xid, tcon, &smbfile->fid);
+ cifsFileInfo_put(smbfile);
+ } else
+ cifs_dbg(FYI, "ignore fsync for file not open for write\n");
+ } else
+ rc = server->ops->flush(xid, tcon, &smbfile->fid);
}
+fsync_exit:
free_xid(xid);
return rc;
}
--
2.32.0
