From 46a7574caf5bc533c24b315800ed323c187614f5 Mon Sep 17 00:00:00 2001 From: Jeff Layton Date: Sun, 24 May 2009 18:45:17 -0400 Subject: [PATCH 01/14] cifs: fix artificial limit on reading symlinks There's no reason to limit the size of a symlink that we can read to 4000 bytes. That may be nowhere near PATH_MAX if the server is sending UCS2 strings. CIFS should be able to read in a symlink up to the size of the buffer. The size of the header has already been accounted for when creating the slabcache, so CIFSMaxBufSize should be the correct size to pass in. Fixes samba bug #6384. Signed-off-by: Jeff Layton Signed-off-by: Steve French --- fs/cifs/cifssmb.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/fs/cifs/cifssmb.c b/fs/cifs/cifssmb.c index d06260251c30..aece2a8c1a7c 100644 --- a/fs/cifs/cifssmb.c +++ b/fs/cifs/cifssmb.c @@ -2427,8 +2427,7 @@ querySymLinkRetry: params = 2 /* level */ + 4 /* rsrvd */ + name_len /* incl null */ ; pSMB->TotalDataCount = 0; pSMB->MaxParameterCount = cpu_to_le16(2); - /* BB find exact max data count below from sess structure BB */ - pSMB->MaxDataCount = cpu_to_le16(4000); + pSMB->MaxDataCount = cpu_to_le16(CIFSMaxBufSize); pSMB->MaxSetupCount = 0; pSMB->Reserved = 0; pSMB->Flags = 0; From f55ed1a83d099f275c9560ad7d4c4700d1e54bdd Mon Sep 17 00:00:00 2001 From: Jeff Layton Date: Tue, 26 May 2009 16:28:11 -0400 Subject: [PATCH 02/14] cifs: tighten up default file_mode/dir_mode The current default file mode is 02767 and dir mode is 0777. This is extremely "loose". Given that CIFS is a single-user protocol, these permissions allow anyone to use the mount -- in effect, giving anyone on the machine access to the credentials used to mount the share. Change this by making the default permissions restrict write access to the default owner of the mount. Give read and execute permissions to everyone else. These are the same permissions that VFAT mounts get by default so there is some precedent here. Note that this patch also removes the mandatory locking flags from the default file_mode. After having looked at how these flags are used by the kernel, I don't think that keeping them as the default offers any real benefit. That flag combination makes it so that the kernel enforces mandatory locking. Since the server is going to do that for us anyway, I don't think we want the client to enforce this by default on applications that just want advisory locks. Anyone that does want this behavior can always enable it by setting the file_mode appropriately. Signed-off-by: Jeff Layton Signed-off-by: Steve French --- fs/cifs/connect.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c index 4aa81a507b74..f32c9036741e 100644 --- a/fs/cifs/connect.c +++ b/fs/cifs/connect.c @@ -827,9 +827,9 @@ cifs_parse_mount_options(char *options, const char *devname, vol->target_rfc1001_name[0] = 0; vol->linux_uid = current_uid(); /* use current_euid() instead? */ vol->linux_gid = current_gid(); - vol->dir_mode = S_IRWXUGO; - /* 2767 perms indicate mandatory locking support */ - vol->file_mode = (S_IRWXUGO | S_ISGID) & (~S_IXGRP); + + /* default to only allowing write access to owner of the mount */ + vol->dir_mode = vol->file_mode = S_IRUGO | S_IXUGO | S_IWUSR; /* vol->retry default is 0 (i.e. "soft" limited retry not hard retry) */ vol->rw = true; From 07119a4df8c8c77d888f2f46964ea9512ea84ff8 Mon Sep 17 00:00:00 2001 From: Jeff Layton Date: Wed, 27 May 2009 09:37:33 -0400 Subject: [PATCH 03/14] cifs: have cifs_NTtimeToUnix take a little-endian arg ...and just have the function call le64_to_cpu. Signed-off-by: Jeff Layton Reviewed-by: Christoph Hellwig Signed-off-by: Steve French --- fs/cifs/cifsproto.h | 2 +- fs/cifs/file.c | 2 +- fs/cifs/inode.c | 15 ++++++--------- fs/cifs/netmisc.c | 12 ++++++------ fs/cifs/readdir.c | 12 ++++++------ 5 files changed, 20 insertions(+), 23 deletions(-) diff --git a/fs/cifs/cifsproto.h b/fs/cifs/cifsproto.h index fae083930eee..8831f649720f 100644 --- a/fs/cifs/cifsproto.h +++ b/fs/cifs/cifsproto.h @@ -90,7 +90,7 @@ extern struct oplock_q_entry *AllocOplockQEntry(struct inode *, u16, struct cifsTconInfo *); extern void DeleteOplockQEntry(struct oplock_q_entry *); extern void DeleteTconOplockQEntries(struct cifsTconInfo *); -extern struct timespec cifs_NTtimeToUnix(u64 utc_nanoseconds_since_1601); +extern struct timespec cifs_NTtimeToUnix(__le64 utc_nanoseconds_since_1601); extern u64 cifs_UnixTimeToNT(struct timespec); extern __le64 cnvrtDosCifsTm(__u16 date, __u16 time); extern struct timespec cnvrtDosUnixTm(__u16 date, __u16 time); diff --git a/fs/cifs/file.c b/fs/cifs/file.c index 302ea15f02e6..06866841b97f 100644 --- a/fs/cifs/file.c +++ b/fs/cifs/file.c @@ -241,7 +241,7 @@ static inline int cifs_open_inode_helper(struct inode *inode, struct file *file, /* BB need same check in cifs_create too? */ /* if not oplocked, invalidate inode pages if mtime or file size changed */ - temp = cifs_NTtimeToUnix(le64_to_cpu(buf->LastWriteTime)); + temp = cifs_NTtimeToUnix(buf->LastWriteTime); if (timespec_equal(&file->f_path.dentry->d_inode->i_mtime, &temp) && (file->f_path.dentry->d_inode->i_size == (loff_t)le64_to_cpu(buf->EndOfFile))) { diff --git a/fs/cifs/inode.c b/fs/cifs/inode.c index 9c869a6dcba1..42d6e0fb6f31 100644 --- a/fs/cifs/inode.c +++ b/fs/cifs/inode.c @@ -85,10 +85,10 @@ static void cifs_unix_info_to_inode(struct inode *inode, __u64 num_of_bytes = le64_to_cpu(info->NumOfBytes); __u64 end_of_file = le64_to_cpu(info->EndOfFile); - inode->i_atime = cifs_NTtimeToUnix(le64_to_cpu(info->LastAccessTime)); + inode->i_atime = cifs_NTtimeToUnix(info->LastAccessTime); inode->i_mtime = - cifs_NTtimeToUnix(le64_to_cpu(info->LastModificationTime)); - inode->i_ctime = cifs_NTtimeToUnix(le64_to_cpu(info->LastStatusChange)); + cifs_NTtimeToUnix(info->LastModificationTime); + inode->i_ctime = cifs_NTtimeToUnix(info->LastStatusChange); inode->i_mode = le64_to_cpu(info->Permissions); /* @@ -554,14 +554,11 @@ int cifs_get_inode_info(struct inode **pinode, /* Linux can not store file creation time so ignore it */ if (pfindData->LastAccessTime) - inode->i_atime = cifs_NTtimeToUnix - (le64_to_cpu(pfindData->LastAccessTime)); + inode->i_atime = cifs_NTtimeToUnix(pfindData->LastAccessTime); else /* do not need to use current_fs_time - time not stored */ inode->i_atime = CURRENT_TIME; - inode->i_mtime = - cifs_NTtimeToUnix(le64_to_cpu(pfindData->LastWriteTime)); - inode->i_ctime = - cifs_NTtimeToUnix(le64_to_cpu(pfindData->ChangeTime)); + inode->i_mtime = cifs_NTtimeToUnix(pfindData->LastWriteTime); + inode->i_ctime = cifs_NTtimeToUnix(pfindData->ChangeTime); cFYI(DBG2, ("Attributes came in as 0x%x", attr)); if (adjustTZ && (pTcon->ses) && (pTcon->ses->server)) { inode->i_ctime.tv_sec += pTcon->ses->server->timeAdj; diff --git a/fs/cifs/netmisc.c b/fs/cifs/netmisc.c index e2fe998989a3..d3ba75ef014f 100644 --- a/fs/cifs/netmisc.c +++ b/fs/cifs/netmisc.c @@ -853,12 +853,12 @@ smbCalcSize_LE(struct smb_hdr *ptr) #define NTFS_TIME_OFFSET ((u64)(369*365 + 89) * 24 * 3600 * 10000000) - /* - * Convert the NT UTC (based 1601-01-01, in hundred nanosecond units) - * into Unix UTC (based 1970-01-01, in seconds). - */ +/* + * Convert the NT UTC (based 1601-01-01, in hundred nanosecond units) + * into Unix UTC (based 1970-01-01, in seconds). + */ struct timespec -cifs_NTtimeToUnix(u64 ntutc) +cifs_NTtimeToUnix(__le64 ntutc) { struct timespec ts; /* BB what about the timezone? BB */ @@ -866,7 +866,7 @@ cifs_NTtimeToUnix(u64 ntutc) /* Subtract the NTFS time offset, then convert to 1s intervals. */ u64 t; - t = ntutc - NTFS_TIME_OFFSET; + t = le64_to_cpu(ntutc) - NTFS_TIME_OFFSET; ts.tv_nsec = do_div(t, 10000000) * 100; ts.tv_sec = t; return ts; diff --git a/fs/cifs/readdir.c b/fs/cifs/readdir.c index 964e097c8203..79c46c2226c5 100644 --- a/fs/cifs/readdir.c +++ b/fs/cifs/readdir.c @@ -150,11 +150,11 @@ static void fill_in_inode(struct inode *tmp_inode, int new_buf_type, allocation_size = le64_to_cpu(pfindData->AllocationSize); end_of_file = le64_to_cpu(pfindData->EndOfFile); tmp_inode->i_atime = - cifs_NTtimeToUnix(le64_to_cpu(pfindData->LastAccessTime)); + cifs_NTtimeToUnix(pfindData->LastAccessTime); tmp_inode->i_mtime = - cifs_NTtimeToUnix(le64_to_cpu(pfindData->LastWriteTime)); + cifs_NTtimeToUnix(pfindData->LastWriteTime); tmp_inode->i_ctime = - cifs_NTtimeToUnix(le64_to_cpu(pfindData->ChangeTime)); + cifs_NTtimeToUnix(pfindData->ChangeTime); } else { /* legacy, OS2 and DOS style */ /* struct timespec ts;*/ FIND_FILE_STANDARD_INFO *pfindData = @@ -331,11 +331,11 @@ static void unix_fill_in_inode(struct inode *tmp_inode, local_size = tmp_inode->i_size; tmp_inode->i_atime = - cifs_NTtimeToUnix(le64_to_cpu(pfindData->LastAccessTime)); + cifs_NTtimeToUnix(pfindData->LastAccessTime); tmp_inode->i_mtime = - cifs_NTtimeToUnix(le64_to_cpu(pfindData->LastModificationTime)); + cifs_NTtimeToUnix(pfindData->LastModificationTime); tmp_inode->i_ctime = - cifs_NTtimeToUnix(le64_to_cpu(pfindData->LastStatusChange)); + cifs_NTtimeToUnix(pfindData->LastStatusChange); tmp_inode->i_mode = le64_to_cpu(pfindData->Permissions); /* since we set the inode type below we need to mask off type From c4a2c08db7d976c2e23a97da5d69ec7c9701034d Mon Sep 17 00:00:00 2001 From: Jeff Layton Date: Wed, 27 May 2009 09:37:33 -0400 Subject: [PATCH 04/14] cifs: make cnvrtDosUnixTm take a little-endian args and an offset The callers primarily end up converting the args from le anyway. Also, most of the callers end up needing to add an offset to the result. The exception to these rules is cnvrtDosCifsTm, but there are no callers of that function, so we might as well remove it. Signed-off-by: Jeff Layton Reviewed-by: Christoph Hellwig Signed-off-by: Steve French --- fs/cifs/cifsproto.h | 4 ++-- fs/cifs/cifssmb.c | 4 ++-- fs/cifs/netmisc.c | 12 ++++-------- fs/cifs/readdir.c | 32 ++++++++++---------------------- 4 files changed, 18 insertions(+), 34 deletions(-) diff --git a/fs/cifs/cifsproto.h b/fs/cifs/cifsproto.h index 8831f649720f..d542cf1f69c3 100644 --- a/fs/cifs/cifsproto.h +++ b/fs/cifs/cifsproto.h @@ -92,8 +92,8 @@ extern void DeleteOplockQEntry(struct oplock_q_entry *); extern void DeleteTconOplockQEntries(struct cifsTconInfo *); extern struct timespec cifs_NTtimeToUnix(__le64 utc_nanoseconds_since_1601); extern u64 cifs_UnixTimeToNT(struct timespec); -extern __le64 cnvrtDosCifsTm(__u16 date, __u16 time); -extern struct timespec cnvrtDosUnixTm(__u16 date, __u16 time); +extern struct timespec cnvrtDosUnixTm(__le16 le_date, __le16 le_time, + int offset); extern int cifs_posix_open(char *full_path, struct inode **pinode, struct super_block *sb, int mode, int oflags, diff --git a/fs/cifs/cifssmb.c b/fs/cifs/cifssmb.c index aece2a8c1a7c..b84c61d5bca4 100644 --- a/fs/cifs/cifssmb.c +++ b/fs/cifs/cifssmb.c @@ -524,8 +524,8 @@ CIFSSMBNegotiate(unsigned int xid, struct cifsSesInfo *ses) int val, seconds, remain, result; struct timespec ts, utc; utc = CURRENT_TIME; - ts = cnvrtDosUnixTm(le16_to_cpu(rsp->SrvTime.Date), - le16_to_cpu(rsp->SrvTime.Time)); + ts = cnvrtDosUnixTm(rsp->SrvTime.Date, + rsp->SrvTime.Time, 0); cFYI(1, ("SrvTime %d sec since 1970 (utc: %d) diff: %d", (int)ts.tv_sec, (int)utc.tv_sec, (int)(utc.tv_sec - ts.tv_sec))); diff --git a/fs/cifs/netmisc.c b/fs/cifs/netmisc.c index d3ba75ef014f..32d6baa0a54f 100644 --- a/fs/cifs/netmisc.c +++ b/fs/cifs/netmisc.c @@ -883,16 +883,12 @@ cifs_UnixTimeToNT(struct timespec t) static int total_days_of_prev_months[] = {0, 31, 59, 90, 120, 151, 181, 212, 243, 273, 304, 334}; - -__le64 cnvrtDosCifsTm(__u16 date, __u16 time) -{ - return cpu_to_le64(cifs_UnixTimeToNT(cnvrtDosUnixTm(date, time))); -} - -struct timespec cnvrtDosUnixTm(__u16 date, __u16 time) +struct timespec cnvrtDosUnixTm(__le16 le_date, __le16 le_time, int offset) { struct timespec ts; int sec, min, days, month, year; + u16 date = le16_to_cpu(le_date); + u16 time = le16_to_cpu(le_time); SMB_TIME *st = (SMB_TIME *)&time; SMB_DATE *sd = (SMB_DATE *)&date; @@ -933,7 +929,7 @@ struct timespec cnvrtDosUnixTm(__u16 date, __u16 time) days -= ((year & 0x03) == 0) && (month < 2 ? 1 : 0); sec += 24 * 60 * 60 * days; - ts.tv_sec = sec; + ts.tv_sec = sec + offset; /* cFYI(1,("sec after cnvrt dos to unix time %d",sec)); */ diff --git a/fs/cifs/readdir.c b/fs/cifs/readdir.c index 79c46c2226c5..86d0055dc529 100644 --- a/fs/cifs/readdir.c +++ b/fs/cifs/readdir.c @@ -115,17 +115,6 @@ construct_dentry(struct qstr *qstring, struct file *file, return rc; } -static void AdjustForTZ(struct cifsTconInfo *tcon, struct inode *inode) -{ - if ((tcon) && (tcon->ses) && (tcon->ses->server)) { - inode->i_ctime.tv_sec += tcon->ses->server->timeAdj; - inode->i_mtime.tv_sec += tcon->ses->server->timeAdj; - inode->i_atime.tv_sec += tcon->ses->server->timeAdj; - } - return; -} - - static void fill_in_inode(struct inode *tmp_inode, int new_buf_type, char *buf, unsigned int *pobject_type, int isNewInode) { @@ -156,20 +145,19 @@ static void fill_in_inode(struct inode *tmp_inode, int new_buf_type, tmp_inode->i_ctime = cifs_NTtimeToUnix(pfindData->ChangeTime); } else { /* legacy, OS2 and DOS style */ -/* struct timespec ts;*/ + int offset = cifs_sb->tcon->ses->server->timeAdj; FIND_FILE_STANDARD_INFO *pfindData = (FIND_FILE_STANDARD_INFO *)buf; - tmp_inode->i_mtime = cnvrtDosUnixTm( - le16_to_cpu(pfindData->LastWriteDate), - le16_to_cpu(pfindData->LastWriteTime)); - tmp_inode->i_atime = cnvrtDosUnixTm( - le16_to_cpu(pfindData->LastAccessDate), - le16_to_cpu(pfindData->LastAccessTime)); - tmp_inode->i_ctime = cnvrtDosUnixTm( - le16_to_cpu(pfindData->LastWriteDate), - le16_to_cpu(pfindData->LastWriteTime)); - AdjustForTZ(cifs_sb->tcon, tmp_inode); + tmp_inode->i_mtime = cnvrtDosUnixTm(pfindData->LastWriteDate, + pfindData->LastWriteTime, + offset); + tmp_inode->i_atime = cnvrtDosUnixTm(pfindData->LastAccessDate, + pfindData->LastAccessTime, + offset); + tmp_inode->i_ctime = cnvrtDosUnixTm(pfindData->LastWriteDate, + pfindData->LastWriteTime, + offset); attr = le16_to_cpu(pfindData->Attributes); allocation_size = le32_to_cpu(pfindData->AllocationSize); end_of_file = le32_to_cpu(pfindData->DataSize); From bd433d4cf4d8593a5f1764776b91f1794fce5a77 Mon Sep 17 00:00:00 2001 From: Jeff Layton Date: Wed, 27 May 2009 09:37:34 -0400 Subject: [PATCH 05/14] cifs: rename cifs_iget to cifs_root_iget The current cifs_iget isn't suitable for anything but the root inode. Rename it with a more appropriate name. Signed-off-by: Jeff Layton Reviewed-by: Christoph Hellwig Signed-off-by: Steve French --- fs/cifs/cifsfs.c | 2 +- fs/cifs/cifsfs.h | 2 +- fs/cifs/inode.c | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/fs/cifs/cifsfs.c b/fs/cifs/cifsfs.c index 5e6d35804d73..0a10a59b6392 100644 --- a/fs/cifs/cifsfs.c +++ b/fs/cifs/cifsfs.c @@ -146,7 +146,7 @@ cifs_read_super(struct super_block *sb, void *data, #endif sb->s_blocksize = CIFS_MAX_MSGSIZE; sb->s_blocksize_bits = 14; /* default 2**14 = CIFS_MAX_MSGSIZE */ - inode = cifs_iget(sb, ROOT_I); + inode = cifs_root_iget(sb, ROOT_I); if (IS_ERR(inode)) { rc = PTR_ERR(inode); diff --git a/fs/cifs/cifsfs.h b/fs/cifs/cifsfs.h index 051b71cfdea9..3b6a85cd484d 100644 --- a/fs/cifs/cifsfs.h +++ b/fs/cifs/cifsfs.h @@ -36,7 +36,7 @@ extern void cifs_read_inode(struct inode *); /* Functions related to inodes */ extern const struct inode_operations cifs_dir_inode_ops; -extern struct inode *cifs_iget(struct super_block *, unsigned long); +extern struct inode *cifs_root_iget(struct super_block *, unsigned long); extern int cifs_create(struct inode *, struct dentry *, int, struct nameidata *); extern struct dentry *cifs_lookup(struct inode *, struct dentry *, diff --git a/fs/cifs/inode.c b/fs/cifs/inode.c index 42d6e0fb6f31..84b7bea73674 100644 --- a/fs/cifs/inode.c +++ b/fs/cifs/inode.c @@ -696,7 +696,7 @@ char *cifs_build_path_to_root(struct cifs_sb_info *cifs_sb) } /* gets root inode */ -struct inode *cifs_iget(struct super_block *sb, unsigned long ino) +struct inode *cifs_root_iget(struct super_block *sb, unsigned long ino) { int xid; struct cifs_sb_info *cifs_sb; From a0c9217f64ee3cd1e534966da8c5f05768e1ab09 Mon Sep 17 00:00:00 2001 From: Jeff Layton Date: Wed, 27 May 2009 15:40:47 -0400 Subject: [PATCH 06/14] cifs: make serverino the default when mounting Signed-off-by: Jeff Layton Signed-off-by: Steve French --- fs/cifs/connect.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c index f32c9036741e..8ae563f028bc 100644 --- a/fs/cifs/connect.c +++ b/fs/cifs/connect.c @@ -835,6 +835,8 @@ cifs_parse_mount_options(char *options, const char *devname, vol->rw = true; /* default is always to request posix paths. */ vol->posix_paths = 1; + /* default to using server inode numbers where available */ + vol->server_ino = 1; if (!options) return 1; From c5077ec42303e07c2c685b0f6cb8eee0f2c7751c Mon Sep 17 00:00:00 2001 From: Steve French Date: Thu, 28 May 2009 15:09:04 +0000 Subject: [PATCH 07/14] [CIFS] Update readme to indicate change to default mount (serverino) Signed-off-by: Steve French --- fs/cifs/CHANGES | 8 ++++++++ fs/cifs/README | 7 ++++++- fs/cifs/cifsfs.h | 2 +- 3 files changed, 15 insertions(+), 2 deletions(-) diff --git a/fs/cifs/CHANGES b/fs/cifs/CHANGES index f20c4069c220..227c681b816d 100644 --- a/fs/cifs/CHANGES +++ b/fs/cifs/CHANGES @@ -1,3 +1,9 @@ +Version 1.59 +------------ +Client uses server inode numbers (which are persistent) rather than +client generated ones by default (mount option "serverino" turned +on by default if server supports it). + Version 1.58 ------------ Guard against buffer overruns in various UCS-2 to UTF-8 string conversions @@ -10,6 +16,8 @@ we converted from). Fix endianness of the vcnum field used during session setup to distinguish multiple mounts to same server from different userids. Raw NTLMSSP fixed (it requires /proc/fs/cifs/experimental flag to be set to 2, and mount must enable krb5 to turn on extended security). +Performance of file create to Samba improved (posix create on lookup +removes 1 of 2 network requests sent on file create) Version 1.57 ------------ diff --git a/fs/cifs/README b/fs/cifs/README index db208ddb9899..6d1608fabde9 100644 --- a/fs/cifs/README +++ b/fs/cifs/README @@ -388,8 +388,13 @@ A partial list of the supported mount options follows: or the CIFS Unix Extensions equivalent and for those this mount option will have no effect. Exporting cifs mounts under nfsd requires this mount option on the cifs mount. + This is now the default if server supports the + required network operation. noserverino Client generates inode numbers (rather than using the actual one - from the server) by default. + from the server). These inode numbers will vary after + unmount or reboot which can confuse some applications, + but not all server filesystems support unique inode + numbers. setuids If the CIFS Unix extensions are negotiated with the server the client will attempt to set the effective uid and gid of the local process on newly created files, directories, and diff --git a/fs/cifs/cifsfs.h b/fs/cifs/cifsfs.h index 3b6a85cd484d..9570a0e8023f 100644 --- a/fs/cifs/cifsfs.h +++ b/fs/cifs/cifsfs.h @@ -100,5 +100,5 @@ extern long cifs_ioctl(struct file *filep, unsigned int cmd, unsigned long arg); extern const struct export_operations cifs_export_ops; #endif /* EXPERIMENTAL */ -#define CIFS_VERSION "1.58" +#define CIFS_VERSION "1.59" #endif /* _CIFSFS_H */ From 1bf4072da67c14d6b02cfeef02212aa5a6211df2 Mon Sep 17 00:00:00 2001 From: Christoph Hellwig Date: Wed, 27 May 2009 09:37:33 -0400 Subject: [PATCH 08/14] cifs: reorganize get_cifs_acl Thus spake Christoph: "But this whole set_cifs_acl function is a real mess anyway and needs some splitting up." With this change too, it's possible to call acl_to_uid_mode() with a NULL inode pointer. That (or something close to it) will eventually be necessary when cifs_get_inode_info is reorganized. Signed-off-by: Christoph Hellwig Reviewed-by: Shirish Pargaonkar Signed-off-by: Jeff Layton Signed-off-by: Steve French --- fs/cifs/cifsacl.c | 102 +++++++++++++++++++++++--------------------- fs/cifs/cifsproto.h | 4 +- fs/cifs/inode.c | 2 +- 3 files changed, 56 insertions(+), 52 deletions(-) diff --git a/fs/cifs/cifsacl.c b/fs/cifs/cifsacl.c index 57ecdc83c26f..7f8e6c46d116 100644 --- a/fs/cifs/cifsacl.c +++ b/fs/cifs/cifsacl.c @@ -552,67 +552,66 @@ static int build_sec_desc(struct cifs_ntsd *pntsd, struct cifs_ntsd *pnntsd, return rc; } - -/* Retrieve an ACL from the server */ -static struct cifs_ntsd *get_cifs_acl(u32 *pacllen, struct inode *inode, - const char *path, const __u16 *pfid) +static struct cifs_ntsd *get_cifs_acl_by_fid(struct cifs_sb_info *cifs_sb, + __u16 fid, u32 *pacllen) { - struct cifsFileInfo *open_file = NULL; - bool unlock_file = false; - int xid; - int rc = -EIO; - __u16 fid; - struct super_block *sb; - struct cifs_sb_info *cifs_sb; struct cifs_ntsd *pntsd = NULL; - - cFYI(1, ("get mode from ACL for %s", path)); - - if (inode == NULL) - return NULL; + int xid, rc; xid = GetXid(); - if (pfid == NULL) - open_file = find_readable_file(CIFS_I(inode)); - else - fid = *pfid; + rc = CIFSSMBGetCIFSACL(xid, cifs_sb->tcon, fid, &pntsd, pacllen); + FreeXid(xid); - sb = inode->i_sb; - if (sb == NULL) { - FreeXid(xid); - return NULL; - } - cifs_sb = CIFS_SB(sb); - if (open_file) { - unlock_file = true; - fid = open_file->netfid; - } else if (pfid == NULL) { - int oplock = 0; - /* open file */ - rc = CIFSSMBOpen(xid, cifs_sb->tcon, path, FILE_OPEN, - READ_CONTROL, 0, &fid, &oplock, NULL, - cifs_sb->local_nls, cifs_sb->mnt_cifs_flags & - CIFS_MOUNT_MAP_SPECIAL_CHR); - if (rc != 0) { - cERROR(1, ("Unable to open file to get ACL")); - FreeXid(xid); - return NULL; - } + cFYI(1, ("GetCIFSACL rc = %d ACL len %d", rc, *pacllen)); + return pntsd; +} + +static struct cifs_ntsd *get_cifs_acl_by_path(struct cifs_sb_info *cifs_sb, + const char *path, u32 *pacllen) +{ + struct cifs_ntsd *pntsd = NULL; + int oplock = 0; + int xid, rc; + __u16 fid; + + xid = GetXid(); + + rc = CIFSSMBOpen(xid, cifs_sb->tcon, path, FILE_OPEN, READ_CONTROL, 0, + &fid, &oplock, NULL, cifs_sb->local_nls, + cifs_sb->mnt_cifs_flags & CIFS_MOUNT_MAP_SPECIAL_CHR); + if (rc) { + cERROR(1, ("Unable to open file to get ACL")); + goto out; } rc = CIFSSMBGetCIFSACL(xid, cifs_sb->tcon, fid, &pntsd, pacllen); cFYI(1, ("GetCIFSACL rc = %d ACL len %d", rc, *pacllen)); - if (unlock_file == true) /* find_readable_file increments ref count */ - atomic_dec(&open_file->wrtPending); - else if (pfid == NULL) /* if opened above we have to close the handle */ - CIFSSMBClose(xid, cifs_sb->tcon, fid); - /* else handle was passed in by caller */ + CIFSSMBClose(xid, cifs_sb->tcon, fid); + out: FreeXid(xid); return pntsd; } +/* Retrieve an ACL from the server */ +static struct cifs_ntsd *get_cifs_acl(struct cifs_sb_info *cifs_sb, + struct inode *inode, const char *path, + u32 *pacllen) +{ + struct cifs_ntsd *pntsd = NULL; + struct cifsFileInfo *open_file = NULL; + + if (inode) + open_file = find_readable_file(CIFS_I(inode)); + if (!open_file) + return get_cifs_acl_by_path(cifs_sb, path, pacllen); + + pntsd = get_cifs_acl_by_fid(cifs_sb, open_file->netfid, pacllen); + atomic_dec(&open_file->wrtPending); + return pntsd; +} + /* Set an ACL on the server */ static int set_cifs_acl(struct cifs_ntsd *pnntsd, __u32 acllen, struct inode *inode, const char *path) @@ -668,14 +667,19 @@ static int set_cifs_acl(struct cifs_ntsd *pnntsd, __u32 acllen, } /* Translate the CIFS ACL (simlar to NTFS ACL) for a file into mode bits */ -void acl_to_uid_mode(struct inode *inode, const char *path, const __u16 *pfid) +void acl_to_uid_mode(struct cifs_sb_info *cifs_sb, struct inode *inode, + const char *path, const __u16 *pfid) { struct cifs_ntsd *pntsd = NULL; u32 acllen = 0; int rc = 0; cFYI(DBG2, ("converting ACL to mode for %s", path)); - pntsd = get_cifs_acl(&acllen, inode, path, pfid); + + if (pfid) + pntsd = get_cifs_acl_by_fid(cifs_sb, *pfid, &acllen); + else + pntsd = get_cifs_acl(cifs_sb, inode, path, &acllen); /* if we can retrieve the ACL, now parse Access Control Entries, ACEs */ if (pntsd) @@ -698,7 +702,7 @@ int mode_to_acl(struct inode *inode, const char *path, __u64 nmode) cFYI(DBG2, ("set ACL from mode for %s", path)); /* Get the security descriptor */ - pntsd = get_cifs_acl(&secdesclen, inode, path, NULL); + pntsd = get_cifs_acl(CIFS_SB(inode->i_sb), inode, path, &secdesclen); /* Add three ACEs for owner, group, everyone getting rid of other ACEs as chmod disables ACEs and set the security descriptor */ diff --git a/fs/cifs/cifsproto.h b/fs/cifs/cifsproto.h index d542cf1f69c3..f9452329bcce 100644 --- a/fs/cifs/cifsproto.h +++ b/fs/cifs/cifsproto.h @@ -108,8 +108,8 @@ extern int cifs_get_inode_info(struct inode **pinode, extern int cifs_get_inode_info_unix(struct inode **pinode, const unsigned char *search_path, struct super_block *sb, int xid); -extern void acl_to_uid_mode(struct inode *inode, const char *path, - const __u16 *pfid); +extern void acl_to_uid_mode(struct cifs_sb_info *cifs_sb, struct inode *inode, + const char *path, const __u16 *pfid); extern int mode_to_acl(struct inode *inode, const char *path, __u64); extern int cifs_mount(struct super_block *, struct cifs_sb_info *, char *, diff --git a/fs/cifs/inode.c b/fs/cifs/inode.c index 84b7bea73674..fad882b075ba 100644 --- a/fs/cifs/inode.c +++ b/fs/cifs/inode.c @@ -626,7 +626,7 @@ int cifs_get_inode_info(struct inode **pinode, /* fill in 0777 bits from ACL */ if (cifs_sb->mnt_cifs_flags & CIFS_MOUNT_CIFS_ACL) { cFYI(1, ("Getting mode bits from ACL")); - acl_to_uid_mode(inode, full_path, pfid); + acl_to_uid_mode(cifs_sb, inode, full_path, pfid); } #endif if (cifs_sb->mnt_cifs_flags & CIFS_MOUNT_UNX_EMUL) { From b96d31a62f714566fa6420851b3bb3615c796322 Mon Sep 17 00:00:00 2001 From: Christoph Hellwig Date: Wed, 27 May 2009 09:37:33 -0400 Subject: [PATCH 09/14] cifs: clean up set_cifs_acl interfaces Signed-off-by: Christoph Hellwig Reviewed-by: Shirish Pargaonkar Signed-off-by: Jeff Layton Signed-off-by: Steve French --- fs/cifs/cifsacl.c | 82 +++++++++++++++++++++++++---------------------- 1 file changed, 43 insertions(+), 39 deletions(-) diff --git a/fs/cifs/cifsacl.c b/fs/cifs/cifsacl.c index 7f8e6c46d116..1403b5d86a73 100644 --- a/fs/cifs/cifsacl.c +++ b/fs/cifs/cifsacl.c @@ -612,57 +612,61 @@ static struct cifs_ntsd *get_cifs_acl(struct cifs_sb_info *cifs_sb, return pntsd; } -/* Set an ACL on the server */ -static int set_cifs_acl(struct cifs_ntsd *pnntsd, __u32 acllen, - struct inode *inode, const char *path) +static int set_cifs_acl_by_fid(struct cifs_sb_info *cifs_sb, __u16 fid, + struct cifs_ntsd *pnntsd, u32 acllen) { - struct cifsFileInfo *open_file; - bool unlock_file = false; - int xid; - int rc = -EIO; + int xid, rc; + + xid = GetXid(); + rc = CIFSSMBSetCIFSACL(xid, cifs_sb->tcon, fid, pnntsd, acllen); + FreeXid(xid); + + cFYI(DBG2, ("SetCIFSACL rc = %d", rc)); + return rc; +} + +static int set_cifs_acl_by_path(struct cifs_sb_info *cifs_sb, const char *path, + struct cifs_ntsd *pnntsd, u32 acllen) +{ + int oplock = 0; + int xid, rc; __u16 fid; - struct super_block *sb; - struct cifs_sb_info *cifs_sb; - cFYI(DBG2, ("set ACL for %s from mode 0x%x", path, inode->i_mode)); - - if (!inode) - return rc; - - sb = inode->i_sb; - if (sb == NULL) - return rc; - - cifs_sb = CIFS_SB(sb); xid = GetXid(); - open_file = find_readable_file(CIFS_I(inode)); - if (open_file) { - unlock_file = true; - fid = open_file->netfid; - } else { - int oplock = 0; - /* open file */ - rc = CIFSSMBOpen(xid, cifs_sb->tcon, path, FILE_OPEN, - WRITE_DAC, 0, &fid, &oplock, NULL, - cifs_sb->local_nls, cifs_sb->mnt_cifs_flags & - CIFS_MOUNT_MAP_SPECIAL_CHR); - if (rc != 0) { - cERROR(1, ("Unable to open file to set ACL")); - FreeXid(xid); - return rc; - } + rc = CIFSSMBOpen(xid, cifs_sb->tcon, path, FILE_OPEN, WRITE_DAC, 0, + &fid, &oplock, NULL, cifs_sb->local_nls, + cifs_sb->mnt_cifs_flags & CIFS_MOUNT_MAP_SPECIAL_CHR); + if (rc) { + cERROR(1, ("Unable to open file to set ACL")); + goto out; } rc = CIFSSMBSetCIFSACL(xid, cifs_sb->tcon, fid, pnntsd, acllen); cFYI(DBG2, ("SetCIFSACL rc = %d", rc)); - if (unlock_file) - atomic_dec(&open_file->wrtPending); - else - CIFSSMBClose(xid, cifs_sb->tcon, fid); + CIFSSMBClose(xid, cifs_sb->tcon, fid); + out: FreeXid(xid); + return rc; +} +/* Set an ACL on the server */ +static int set_cifs_acl(struct cifs_ntsd *pnntsd, __u32 acllen, + struct inode *inode, const char *path) +{ + struct cifs_sb_info *cifs_sb = CIFS_SB(inode->i_sb); + struct cifsFileInfo *open_file; + int rc; + + cFYI(DBG2, ("set ACL for %s from mode 0x%x", path, inode->i_mode)); + + open_file = find_readable_file(CIFS_I(inode)); + if (!open_file) + return set_cifs_acl_by_path(cifs_sb, path, pnntsd, acllen); + + rc = set_cifs_acl_by_fid(cifs_sb, open_file->netfid, pnntsd, acllen); + atomic_dec(&open_file->wrtPending); return rc; } From 50b64e3b77d569c217a48e078cd565dbd6462ad0 Mon Sep 17 00:00:00 2001 From: Jeff Layton Date: Tue, 2 Jun 2009 06:55:20 -0400 Subject: [PATCH 10/14] cifs: fix IPv6 address length check For IPv6 the userspace mount helper sends an address in the "ip=" option. This check fails if the length is > 35 characters. I have no idea where the magic 35 character limit came from, but it's clearly not enough for IPv6. Fix it by making it use the INET6_ADDRSTRLEN #define. While we're at it, use the same #define for the address length in SPNEGO upcalls. Reported-by: Charles R. Anderson Signed-off-by: Jeff Layton Signed-off-by: Steve French --- fs/cifs/cifs_spnego.c | 6 ++---- fs/cifs/connect.c | 4 +++- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/fs/cifs/cifs_spnego.c b/fs/cifs/cifs_spnego.c index 67bf93a40d2e..4a4581cb2b5e 100644 --- a/fs/cifs/cifs_spnego.c +++ b/fs/cifs/cifs_spnego.c @@ -23,6 +23,7 @@ #include #include #include +#include #include "cifsglob.h" #include "cifs_spnego.h" #include "cifs_debug.h" @@ -73,9 +74,6 @@ struct key_type cifs_spnego_key_type = { * strlen(";sec=ntlmsspi") */ #define MAX_MECH_STR_LEN 13 -/* max possible addr len eg FEDC:BA98:7654:3210:FEDC:BA98:7654:3210/128 */ -#define MAX_IPV6_ADDR_LEN 43 - /* strlen of "host=" */ #define HOST_KEY_LEN 5 @@ -102,7 +100,7 @@ cifs_get_spnego_key(struct cifsSesInfo *sesInfo) host=hostname sec=mechanism uid=0xFF user=username */ desc_len = MAX_VER_STR_LEN + HOST_KEY_LEN + strlen(hostname) + - IP_KEY_LEN + MAX_IPV6_ADDR_LEN + + IP_KEY_LEN + INET6_ADDRSTRLEN + MAX_MECH_STR_LEN + UID_KEY_LEN + (sizeof(uid_t) * 2) + USER_KEY_LEN + strlen(sesInfo->userName) + 1; diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c index 8ae563f028bc..74b5a87e9195 100644 --- a/fs/cifs/connect.c +++ b/fs/cifs/connect.c @@ -35,6 +35,7 @@ #include #include #include +#include #include #include "cifspdu.h" #include "cifsglob.h" @@ -960,7 +961,8 @@ cifs_parse_mount_options(char *options, const char *devname, } else if (strnicmp(data, "ip", 2) == 0) { if (!value || !*value) { vol->UNCip = NULL; - } else if (strnlen(value, 35) < 35) { + } else if (strnlen(value, INET6_ADDRSTRLEN) < + INET6_ADDRSTRLEN) { vol->UNCip = value; } else { printk(KERN_WARNING "CIFS: ip address " From 4ae1507f6d266d0cc3dd36e474d83aad70fec9e4 Mon Sep 17 00:00:00 2001 From: Jeff Layton Date: Sun, 24 May 2009 18:45:15 -0400 Subject: [PATCH 11/14] cifs: make overriding of ownership conditional on new mount options We have a bit of a problem with the uid= option. The basic issue is that it means too many things and has too many side-effects. It's possible to allow an unprivileged user to mount a filesystem if the user owns the mountpoint, /bin/mount is setuid root, and the mount is set up in /etc/fstab with the "user" option. When doing this though, /bin/mount automatically adds the "uid=" and "gid=" options to the share. This is fortunate since the correct uid= option is needed in order to tell the upcall what user's credcache to use when generating the SPNEGO blob. On a mount without unix extensions this is fine -- you generally will want the files to be owned by the "owner" of the mount. The problem comes in on a mount with unix extensions. With those enabled, the uid/gid options cause the ownership of files to be overriden even though the server is sending along the ownership info. This means that it's not possible to have a mount by an unprivileged user that shows the server's file ownership info. The result is also inode permissions that have no reflection at all on the server. You simply cannot separate ownership from the mode in this fashion. This behavior also makes MultiuserMount option less usable. Once you pass in the uid= option for a mount, then you can't use unix ownership info and allow someone to share the mount. While I'm not thrilled with it, the only solution I can see is to stop making uid=/gid= force the overriding of ownership on mounts, and to add new mount options that turn this behavior on. Signed-off-by: Jeff Layton Signed-off-by: Steve French --- fs/cifs/connect.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c index 74b5a87e9195..10151f8d8495 100644 --- a/fs/cifs/connect.c +++ b/fs/cifs/connect.c @@ -1096,17 +1096,17 @@ cifs_parse_mount_options(char *options, const char *devname, return 1; } } else if (strnicmp(data, "uid", 3) == 0) { - if (value && *value) { + if (value && *value) vol->linux_uid = simple_strtoul(value, &value, 0); + } else if (strnicmp(data, "forceuid", 8) == 0) { vol->override_uid = 1; - } } else if (strnicmp(data, "gid", 3) == 0) { - if (value && *value) { + if (value && *value) vol->linux_gid = simple_strtoul(value, &value, 0); + } else if (strnicmp(data, "forcegid", 8) == 0) { vol->override_gid = 1; - } } else if (strnicmp(data, "file_mode", 4) == 0) { if (value && *value) { vol->file_mode = From f0472d0ec89bef2ea4432828c3daa1b26ef569aa Mon Sep 17 00:00:00 2001 From: Steve French Date: Sat, 6 Jun 2009 21:09:39 +0000 Subject: [PATCH 12/14] [CIFS] Add mention of new mount parm (forceuid) to cifs readme Also update fs/cifs/CHANGES Signed-off-by: Steve French --- fs/cifs/CHANGES | 5 ++++- fs/cifs/README | 9 ++++++++- 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/fs/cifs/CHANGES b/fs/cifs/CHANGES index 227c681b816d..b48689839428 100644 --- a/fs/cifs/CHANGES +++ b/fs/cifs/CHANGES @@ -2,7 +2,10 @@ Version 1.59 ------------ Client uses server inode numbers (which are persistent) rather than client generated ones by default (mount option "serverino" turned -on by default if server supports it). +on by default if server supports it). Add forceuid and forcegid +mount options (so that when negotiating unix extensions specifying +which uid mounted does not immediately force the server's reported +uids to be overridden). Version 1.58 ------------ diff --git a/fs/cifs/README b/fs/cifs/README index 6d1608fabde9..ad92921dbde4 100644 --- a/fs/cifs/README +++ b/fs/cifs/README @@ -262,7 +262,8 @@ A partial list of the supported mount options follows: mount. domain Set the SMB/CIFS workgroup name prepended to the username during CIFS session establishment - uid Set the default uid for inodes. For mounts to servers + forceuid Set the default uid for inodes based on the uid + passed in. For mounts to servers which do support the CIFS Unix extensions, such as a properly configured Samba server, the server provides the uid, gid and mode so this parameter should not be @@ -292,6 +293,12 @@ A partial list of the supported mount options follows: the client. Note that the mount.cifs helper must be at version 1.10 or higher to support specifying the uid (or gid) in non-numeric form. + forcegid (similar to above but for the groupid instead of uid) + uid Set the default uid for inodes, and indicate to the + cifs kernel driver which local user mounted . If the server + supports the unix extensions the default uid is + not used to fill in the owner fields of inodes (files) + unless the "forceuid" parameter is specified. gid Set the default gid for inodes (similar to above). file_mode If CIFS Unix extensions are not supported by the server this overrides the default mode for file inodes. From 58f7f68f228c3aba2ba4468d92e2cec35724ba2e Mon Sep 17 00:00:00 2001 From: Jeff Layton Date: Wed, 10 Jun 2009 09:57:55 -0400 Subject: [PATCH 13/14] cifs: add addr= mount option alias for ip= When you look in /proc/mounts, the address of the server gets displayed as "addr=". That's really a better option to use anyway since it's more generic. What if we eventually want to support non-IP transports? It also makes CIFS option consistent with the NFS option of the same name. Begin the migration to that option name by adding an alias for ip= called addr=. Signed-off-by: Jeff Layton Signed-off-by: Steve French --- fs/cifs/connect.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c index 10151f8d8495..6298dc32adeb 100644 --- a/fs/cifs/connect.c +++ b/fs/cifs/connect.c @@ -958,7 +958,8 @@ cifs_parse_mount_options(char *options, const char *devname, } strcpy(vol->password, value); } - } else if (strnicmp(data, "ip", 2) == 0) { + } else if (!strnicmp(data, "ip", 2) || + !strnicmp(data, "addr", 4)) { if (!value || !*value) { vol->UNCip = NULL; } else if (strnlen(value, INET6_ADDRSTRLEN) < From 61b6bc525a34931bb73e4c95bfe009cd9572a288 Mon Sep 17 00:00:00 2001 From: Jeff Layton Date: Wed, 10 Jun 2009 10:04:58 -0400 Subject: [PATCH 14/14] cifs: remove never-used in6_addr option This option was never used to my knowledge. Remove it before someone does... Signed-off-by: Jeff Layton Signed-off-by: Steve French --- fs/cifs/connect.c | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c index 6298dc32adeb..97f4311b9a8e 100644 --- a/fs/cifs/connect.c +++ b/fs/cifs/connect.c @@ -62,7 +62,6 @@ struct smb_vol { char *domainname; char *UNC; char *UNCip; - char *in6_addr; /* ipv6 address as human readable form of in6_addr */ char *iocharset; /* local code page for mapping to and from Unicode */ char source_rfc1001_name[16]; /* netbios name of client */ char target_rfc1001_name[16]; /* netbios name of server for Win9x/ME */ @@ -1320,16 +1319,6 @@ cifs_parse_mount_options(char *options, const char *devname, vol->direct_io = 1; } else if (strnicmp(data, "forcedirectio", 13) == 0) { vol->direct_io = 1; - } else if (strnicmp(data, "in6_addr", 8) == 0) { - if (!value || !*value) { - vol->in6_addr = NULL; - } else if (strnlen(value, 49) == 48) { - vol->in6_addr = value; - } else { - printk(KERN_WARNING "CIFS: ip v6 address not " - "48 characters long\n"); - return 1; - } } else if (strnicmp(data, "noac", 4) == 0) { printk(KERN_WARNING "CIFS: Mount option noac not " "supported. Instead set "