[PATCH 0/3] cifs: add a SET_FILE_INFO variant that uses FILE_UNIX_BASIC_INFO infolevel

[Jeff Layton-2]

Jeremy and Kukks pinged me on IRC last night about this problem:

https://bugzilla.samba.org/show_bug.cgi?id=6520

...apparently it was a samba bug, but Jeremy pointed out that CIFS
should really be using SET_FILE_INFO rather than SET_PATH_INFO when it
has an open filehandle.

This set adds a new SET_FILE_INFO variant that uses a
FILE_UNIX_BASIC_INFO infolevel and changes cifs_setattr_unix to use it.
It's a pretty straightforward set, but has only been lightly tested so
far.

This is probably 2.6.32 material.

Jeff Layton (3):
  cifs: rename CIFSSMBUnixSetInfo to CIFSSMBUnixSetPathInfo
  cifs: make a separate function for filling out FILE_UNIX_BASIC_INFO
  cifs: add and use CIFSSMBUnixSetFileInfo for setattr calls

 fs/cifs/cifsproto.h |    6 ++-
 fs/cifs/cifssmb.c   |  143 ++++++++++++++++++++++++++++++++++++++------------
 fs/cifs/dir.c       |   15 +++---
 fs/cifs/file.c      |    6 +-
 fs/cifs/inode.c     |   25 ++++++---
 5 files changed, 141 insertions(+), 54 deletions(-)

_______________________________________________
linux-cifs-client mailing list
linux-cifs-client@...
https://lists.samba.org/mailman/listinfo/linux-cifs-client

[Jeff Layton-2]

...in preparation of adding a SET_FILE_INFO variant.

Signed-off-by: Jeff Layton <jlayton@...>
---
 fs/cifs/cifsproto.h |    2 +-
 fs/cifs/cifssmb.c   |    6 +++---
 fs/cifs/dir.c       |   15 ++++++++-------
 fs/cifs/file.c      |    6 +++---
 fs/cifs/inode.c     |   16 ++++++++--------
 5 files changed, 23 insertions(+), 22 deletions(-)

diff --git a/fs/cifs/cifsproto.h b/fs/cifs/cifsproto.h
index c55e023..8085335 100644
--- a/fs/cifs/cifsproto.h
+++ b/fs/cifs/cifsproto.h
@@ -220,7 +220,7 @@ struct cifs_unix_set_info_args {
  dev_t device;
 };
 
-extern int CIFSSMBUnixSetInfo(const int xid, struct cifsTconInfo *pTcon,
+extern int CIFSSMBUnixSetPathInfo(const int xid, struct cifsTconInfo *pTcon,
  char *fileName,
  const struct cifs_unix_set_info_args *args,
  const struct nls_table *nls_codepage,
diff --git a/fs/cifs/cifssmb.c b/fs/cifs/cifssmb.c
index 61007c6..1cd01ba 100644
--- a/fs/cifs/cifssmb.c
+++ b/fs/cifs/cifssmb.c
@@ -5075,9 +5075,9 @@ SetAttrLgcyRetry:
 #endif /* temporarily unneeded SetAttr legacy function */
 
 int
-CIFSSMBUnixSetInfo(const int xid, struct cifsTconInfo *tcon, char *fileName,
-   const struct cifs_unix_set_info_args *args,
-   const struct nls_table *nls_codepage, int remap)
+CIFSSMBUnixSetPathInfo(const int xid, struct cifsTconInfo *tcon, char *fileName,
+       const struct cifs_unix_set_info_args *args,
+       const struct nls_table *nls_codepage, int remap)
 {
  TRANSACTION2_SPI_REQ *pSMB = NULL;
  TRANSACTION2_SPI_RSP *pSMBr = NULL;
diff --git a/fs/cifs/dir.c b/fs/cifs/dir.c
index ff55fc6..4326ffd 100644
--- a/fs/cifs/dir.c
+++ b/fs/cifs/dir.c
@@ -425,9 +425,10 @@ cifs_create(struct inode *inode, struct dentry *direntry, int mode,
  args.uid = NO_CHANGE_64;
  args.gid = NO_CHANGE_64;
  }
- CIFSSMBUnixSetInfo(xid, tcon, full_path, &args,
- cifs_sb->local_nls,
- cifs_sb->mnt_cifs_flags & CIFS_MOUNT_MAP_SPECIAL_CHR);
+ CIFSSMBUnixSetPathInfo(xid, tcon, full_path, &args,
+ cifs_sb->local_nls,
+ cifs_sb->mnt_cifs_flags &
+ CIFS_MOUNT_MAP_SPECIAL_CHR);
  } else {
  /* BB implement mode setting via Windows security
    descriptors e.g. */
@@ -515,10 +516,10 @@ int cifs_mknod(struct inode *inode, struct dentry *direntry, int mode,
  args.uid = NO_CHANGE_64;
  args.gid = NO_CHANGE_64;
  }
- rc = CIFSSMBUnixSetInfo(xid, pTcon, full_path,
- &args, cifs_sb->local_nls,
- cifs_sb->mnt_cifs_flags &
- CIFS_MOUNT_MAP_SPECIAL_CHR);
+ rc = CIFSSMBUnixSetPathInfo(xid, pTcon, full_path, &args,
+    cifs_sb->local_nls,
+    cifs_sb->mnt_cifs_flags &
+ CIFS_MOUNT_MAP_SPECIAL_CHR);
 
  if (!rc) {
  rc = cifs_get_inode_info_unix(&newinode, full_path,
diff --git a/fs/cifs/file.c b/fs/cifs/file.c
index 97ce4bf..c34b7f8 100644
--- a/fs/cifs/file.c
+++ b/fs/cifs/file.c
@@ -448,9 +448,9 @@ int cifs_open(struct inode *inode, struct file *file)
  .mtime = NO_CHANGE_64,
  .device = 0,
  };
- CIFSSMBUnixSetInfo(xid, tcon, full_path, &args,
-    cifs_sb->local_nls,
-    cifs_sb->mnt_cifs_flags &
+ CIFSSMBUnixSetPathInfo(xid, tcon, full_path, &args,
+       cifs_sb->local_nls,
+       cifs_sb->mnt_cifs_flags &
  CIFS_MOUNT_MAP_SPECIAL_CHR);
  }
  }
diff --git a/fs/cifs/inode.c b/fs/cifs/inode.c
index 080f291..96ab05e 100644
--- a/fs/cifs/inode.c
+++ b/fs/cifs/inode.c
@@ -1146,10 +1146,10 @@ mkdir_get_info:
  args.uid = NO_CHANGE_64;
  args.gid = NO_CHANGE_64;
  }
- CIFSSMBUnixSetInfo(xid, pTcon, full_path, &args,
-    cifs_sb->local_nls,
-    cifs_sb->mnt_cifs_flags &
-    CIFS_MOUNT_MAP_SPECIAL_CHR);
+ CIFSSMBUnixSetPathInfo(xid, pTcon, full_path, &args,
+       cifs_sb->local_nls,
+       cifs_sb->mnt_cifs_flags &
+ CIFS_MOUNT_MAP_SPECIAL_CHR);
  } else {
  if (!(cifs_sb->mnt_cifs_flags & CIFS_MOUNT_CIFS_ACL) &&
     (mode & S_IWUGO) == 0) {
@@ -1723,10 +1723,10 @@ cifs_setattr_unix(struct dentry *direntry, struct iattr *attrs)
  args->ctime = NO_CHANGE_64;
 
  args->device = 0;
- rc = CIFSSMBUnixSetInfo(xid, pTcon, full_path, args,
- cifs_sb->local_nls,
- cifs_sb->mnt_cifs_flags &
- CIFS_MOUNT_MAP_SPECIAL_CHR);
+ rc = CIFSSMBUnixSetPathInfo(xid, pTcon, full_path, args,
+    cifs_sb->local_nls,
+    cifs_sb->mnt_cifs_flags &
+ CIFS_MOUNT_MAP_SPECIAL_CHR);
 
  if (!rc)
  rc = inode_setattr(inode, attrs);
--
1.6.0.6

_______________________________________________
linux-cifs-client mailing list
linux-cifs-client@...
https://lists.samba.org/mailman/listinfo/linux-cifs-client

[Jeff Layton-2]

The SET_FILE_INFO variant will need to do the same thing here. Break
this code out into a separate function that both variants can call.

Signed-off-by: Jeff Layton <jlayton@...>
---
 fs/cifs/cifssmb.c |   74 ++++++++++++++++++++++++++++++-----------------------
 1 files changed, 42 insertions(+), 32 deletions(-)

diff --git a/fs/cifs/cifssmb.c b/fs/cifs/cifssmb.c
index 1cd01ba..1f3c8a4 100644
--- a/fs/cifs/cifssmb.c
+++ b/fs/cifs/cifssmb.c
@@ -5074,6 +5074,47 @@ SetAttrLgcyRetry:
 }
 #endif /* temporarily unneeded SetAttr legacy function */
 
+static void
+cifs_fill_unix_set_info(FILE_UNIX_BASIC_INFO *data_offset,
+ const struct cifs_unix_set_info_args *args)
+{
+ u64 mode = args->mode;
+
+ /*
+ * Samba server ignores set of file size to zero due to bugs in some
+ * older clients, but we should be precise - we use SetFileSize to
+ * set file size and do not want to truncate file size to zero
+ * accidently as happened on one Samba server beta by putting
+ * zero instead of -1 here
+ */
+ data_offset->EndOfFile = cpu_to_le64(NO_CHANGE_64);
+ data_offset->NumOfBytes = cpu_to_le64(NO_CHANGE_64);
+ data_offset->LastStatusChange = cpu_to_le64(args->ctime);
+ data_offset->LastAccessTime = cpu_to_le64(args->atime);
+ data_offset->LastModificationTime = cpu_to_le64(args->mtime);
+ data_offset->Uid = cpu_to_le64(args->uid);
+ data_offset->Gid = cpu_to_le64(args->gid);
+ /* better to leave device as zero when it is  */
+ data_offset->DevMajor = cpu_to_le64(MAJOR(args->device));
+ data_offset->DevMinor = cpu_to_le64(MINOR(args->device));
+ data_offset->Permissions = cpu_to_le64(mode);
+
+ if (S_ISREG(mode))
+ data_offset->Type = cpu_to_le32(UNIX_FILE);
+ else if (S_ISDIR(mode))
+ data_offset->Type = cpu_to_le32(UNIX_DIR);
+ else if (S_ISLNK(mode))
+ data_offset->Type = cpu_to_le32(UNIX_SYMLINK);
+ else if (S_ISCHR(mode))
+ data_offset->Type = cpu_to_le32(UNIX_CHARDEV);
+ else if (S_ISBLK(mode))
+ data_offset->Type = cpu_to_le32(UNIX_BLOCKDEV);
+ else if (S_ISFIFO(mode))
+ data_offset->Type = cpu_to_le32(UNIX_FIFO);
+ else if (S_ISSOCK(mode))
+ data_offset->Type = cpu_to_le32(UNIX_SOCKET);
+}
+
 int
 CIFSSMBUnixSetPathInfo(const int xid, struct cifsTconInfo *tcon, char *fileName,
        const struct cifs_unix_set_info_args *args,
@@ -5086,7 +5127,6 @@ CIFSSMBUnixSetPathInfo(const int xid, struct cifsTconInfo *tcon, char *fileName,
  int bytes_returned = 0;
  FILE_UNIX_BASIC_INFO *data_offset;
  __u16 params, param_offset, offset, count, byte_count;
- __u64 mode = args->mode;
 
  cFYI(1, ("In SetUID/GID/Mode"));
 setPermsRetry:
@@ -5137,38 +5177,8 @@ setPermsRetry:
  pSMB->InformationLevel = cpu_to_le16(SMB_SET_FILE_UNIX_BASIC);
  pSMB->Reserved4 = 0;
  pSMB->hdr.smb_buf_length += byte_count;
- /* Samba server ignores set of file size to zero due to bugs in some
- older clients, but we should be precise - we use SetFileSize to
- set file size and do not want to truncate file size to zero
- accidently as happened on one Samba server beta by putting
- zero instead of -1 here */
- data_offset->EndOfFile = cpu_to_le64(NO_CHANGE_64);
- data_offset->NumOfBytes = cpu_to_le64(NO_CHANGE_64);
- data_offset->LastStatusChange = cpu_to_le64(args->ctime);
- data_offset->LastAccessTime = cpu_to_le64(args->atime);
- data_offset->LastModificationTime = cpu_to_le64(args->mtime);
- data_offset->Uid = cpu_to_le64(args->uid);
- data_offset->Gid = cpu_to_le64(args->gid);
- /* better to leave device as zero when it is  */
- data_offset->DevMajor = cpu_to_le64(MAJOR(args->device));
- data_offset->DevMinor = cpu_to_le64(MINOR(args->device));
- data_offset->Permissions = cpu_to_le64(mode);
-
- if (S_ISREG(mode))
- data_offset->Type = cpu_to_le32(UNIX_FILE);
- else if (S_ISDIR(mode))
- data_offset->Type = cpu_to_le32(UNIX_DIR);
- else if (S_ISLNK(mode))
- data_offset->Type = cpu_to_le32(UNIX_SYMLINK);
- else if (S_ISCHR(mode))
- data_offset->Type = cpu_to_le32(UNIX_CHARDEV);
- else if (S_ISBLK(mode))
- data_offset->Type = cpu_to_le32(UNIX_BLOCKDEV);
- else if (S_ISFIFO(mode))
- data_offset->Type = cpu_to_le32(UNIX_FIFO);
- else if (S_ISSOCK(mode))
- data_offset->Type = cpu_to_le32(UNIX_SOCKET);
 
+ cifs_fill_unix_set_info(data_offset, args);
 
  pSMB->ByteCount = cpu_to_le16(byte_count);
  rc = SendReceive(xid, tcon->ses, (struct smb_hdr *) pSMB,
--
1.6.0.6

_______________________________________________
linux-cifs-client mailing list
linux-cifs-client@...
https://lists.samba.org/mailman/listinfo/linux-cifs-client

[Jeff Layton-2]

When there's an open filehandle, SET_FILE_INFO is apparently preferred
over SET_PATH_INFO. Add a new variant that sets a FILE_UNIX_INFO_BASIC
infolevel via SET_FILE_INFO and switch cifs_setattr_unix to use the
new call when there's an open filehandle available.

Signed-off-by: Jeff Layton <jlayton@...>
---
 fs/cifs/cifsproto.h |    4 +++
 fs/cifs/cifssmb.c   |   63 +++++++++++++++++++++++++++++++++++++++++++++++++++
 fs/cifs/inode.c     |   11 ++++++++-
 3 files changed, 77 insertions(+), 1 deletions(-)

diff --git a/fs/cifs/cifsproto.h b/fs/cifs/cifsproto.h
index 8085335..da8fbf5 100644
--- a/fs/cifs/cifsproto.h
+++ b/fs/cifs/cifsproto.h
@@ -220,6 +220,10 @@ struct cifs_unix_set_info_args {
  dev_t device;
 };
 
+extern int CIFSSMBUnixSetFileInfo(const int xid, struct cifsTconInfo *tcon,
+  const struct cifs_unix_set_info_args *args,
+  u16 fid, u32 pid_of_opener);
+
 extern int CIFSSMBUnixSetPathInfo(const int xid, struct cifsTconInfo *pTcon,
  char *fileName,
  const struct cifs_unix_set_info_args *args,
diff --git a/fs/cifs/cifssmb.c b/fs/cifs/cifssmb.c
index 1f3c8a4..922f5fe 100644
--- a/fs/cifs/cifssmb.c
+++ b/fs/cifs/cifssmb.c
@@ -5116,6 +5116,69 @@ cifs_fill_unix_set_info(FILE_UNIX_BASIC_INFO *data_offset,
 }
 
 int
+CIFSSMBUnixSetFileInfo(const int xid, struct cifsTconInfo *tcon,
+       const struct cifs_unix_set_info_args *args,
+       u16 fid, u32 pid_of_opener)
+{
+ struct smb_com_transaction2_sfi_req *pSMB  = NULL;
+ FILE_UNIX_BASIC_INFO *data_offset;
+ int rc = 0;
+ u16 params, param_offset, offset, byte_count, count;
+
+ cFYI(1, ("Set Unix Info (via SetFileInfo)"));
+ rc = small_smb_init(SMB_COM_TRANSACTION2, 15, tcon, (void **) &pSMB);
+
+ if (rc)
+ return rc;
+
+ pSMB->hdr.Pid = cpu_to_le16((__u16)pid_of_opener);
+ pSMB->hdr.PidHigh = cpu_to_le16((__u16)(pid_of_opener >> 16));
+
+ params = 6;
+ pSMB->MaxSetupCount = 0;
+ pSMB->Reserved = 0;
+ pSMB->Flags = 0;
+ pSMB->Timeout = 0;
+ pSMB->Reserved2 = 0;
+ param_offset = offsetof(struct smb_com_transaction2_sfi_req, Fid) - 4;
+ offset = param_offset + params;
+
+ data_offset = (FILE_UNIX_BASIC_INFO *)
+ ((char *)(&pSMB->hdr.Protocol) + offset);
+ count = sizeof(FILE_UNIX_BASIC_INFO);
+
+ pSMB->MaxParameterCount = cpu_to_le16(2);
+ /* BB find max SMB PDU from sess */
+ pSMB->MaxDataCount = cpu_to_le16(1000);
+ pSMB->SetupCount = 1;
+ pSMB->Reserved3 = 0;
+ pSMB->SubCommand = cpu_to_le16(TRANS2_SET_FILE_INFORMATION);
+ byte_count = 3 /* pad */  + params + count;
+ pSMB->DataCount = cpu_to_le16(count);
+ pSMB->ParameterCount = cpu_to_le16(params);
+ pSMB->TotalDataCount = pSMB->DataCount;
+ pSMB->TotalParameterCount = pSMB->ParameterCount;
+ pSMB->ParameterOffset = cpu_to_le16(param_offset);
+ pSMB->DataOffset = cpu_to_le16(offset);
+ pSMB->Fid = fid;
+ pSMB->InformationLevel = cpu_to_le16(SMB_SET_FILE_UNIX_BASIC);
+ pSMB->Reserved4 = 0;
+ pSMB->hdr.smb_buf_length += byte_count;
+ pSMB->ByteCount = cpu_to_le16(byte_count);
+
+ cifs_fill_unix_set_info(data_offset, args);
+
+ rc = SendReceiveNoRsp(xid, tcon->ses, (struct smb_hdr *) pSMB, 0);
+ if (rc)
+ cFYI(1, ("Send error in Set Time (SetFileInfo) = %d", rc));
+
+ /* Note: On -EAGAIN error only caller can retry on handle based calls
+ since file handle passed in no longer valid */
+
+ return rc;
+}
+
+int
 CIFSSMBUnixSetPathInfo(const int xid, struct cifsTconInfo *tcon, char *fileName,
        const struct cifs_unix_set_info_args *args,
        const struct nls_table *nls_codepage, int remap)
diff --git a/fs/cifs/inode.c b/fs/cifs/inode.c
index 96ab05e..2680a5b 100644
--- a/fs/cifs/inode.c
+++ b/fs/cifs/inode.c
@@ -1637,6 +1637,7 @@ cifs_setattr_unix(struct dentry *direntry, struct iattr *attrs)
  struct cifs_sb_info *cifs_sb = CIFS_SB(inode->i_sb);
  struct cifsTconInfo *pTcon = cifs_sb->tcon;
  struct cifs_unix_set_info_args *args = NULL;
+ struct cifsFileInfo *open_file;
 
  cFYI(1, ("setattr_unix on file %s attrs->ia_valid=0x%x",
  direntry->d_name.name, attrs->ia_valid));
@@ -1723,10 +1724,18 @@ cifs_setattr_unix(struct dentry *direntry, struct iattr *attrs)
  args->ctime = NO_CHANGE_64;
 
  args->device = 0;
- rc = CIFSSMBUnixSetPathInfo(xid, pTcon, full_path, args,
+ open_file = find_writable_file(cifsInode);
+ if (open_file) {
+ u16 nfid = open_file->netfid;
+ u32 npid = open_file->pid;
+ rc = CIFSSMBUnixSetFileInfo(xid, pTcon, args, nfid, npid);
+ atomic_dec(&open_file->wrtPending);
+ } else {
+ rc = CIFSSMBUnixSetPathInfo(xid, pTcon, full_path, args,
     cifs_sb->local_nls,
     cifs_sb->mnt_cifs_flags &
  CIFS_MOUNT_MAP_SPECIAL_CHR);
+ }
 
  if (!rc)
  rc = inode_setattr(inode, attrs);
--
1.6.0.6

_______________________________________________
linux-cifs-client mailing list
linux-cifs-client@...
https://lists.samba.org/mailman/listinfo/linux-cifs-client