From 852e22950dc47e774bb602b16f55fed42afac5fb Mon Sep 17 00:00:00 2001 From: Jeff Layton Date: Sun, 25 Nov 2012 08:00:36 -0500 Subject: cifs: use the NUM_AUTHS and NUM_SUBAUTHS constants in cifsacl code ...instead of hardcoding in '5' and '6' all over the place. Reviewed-by: Shirish Pargaonkar Signed-off-by: Jeff Layton Signed-off-by: Steve French --- fs/cifs/cifsacl.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'fs/cifs/cifsacl.h') diff --git a/fs/cifs/cifsacl.h b/fs/cifs/cifsacl.h index 5c902c7ce52..80e0d66a403 100644 --- a/fs/cifs/cifsacl.h +++ b/fs/cifs/cifsacl.h @@ -60,8 +60,8 @@ struct cifs_ntsd { struct cifs_sid { __u8 revision; /* revision level */ __u8 num_subauth; - __u8 authority[6]; - __le32 sub_auth[5]; /* sub_auth[num_subauth] */ + __u8 authority[NUM_AUTHS]; + __le32 sub_auth[NUM_SUBAUTHS]; /* sub_auth[num_subauth] */ } __attribute__((packed)); struct cifs_acl { -- cgit v1.2.3 From 436bb435fcbe2d52678ec7e2abc45fd1938601ce Mon Sep 17 00:00:00 2001 From: Jeff Layton Date: Sun, 25 Nov 2012 08:00:36 -0500 Subject: cifs: make compare_sids static ..nothing outside of cifsacl.c calls it. Also fix the incorrect comment on the function. It returns 0 when they match. Reviewed-by: Shirish Pargaonkar Signed-off-by: Jeff Layton Signed-off-by: Steve French --- fs/cifs/cifsacl.h | 2 -- 1 file changed, 2 deletions(-) (limited to 'fs/cifs/cifsacl.h') diff --git a/fs/cifs/cifsacl.h b/fs/cifs/cifsacl.h index 80e0d66a403..18c7521273a 100644 --- a/fs/cifs/cifsacl.h +++ b/fs/cifs/cifsacl.h @@ -98,6 +98,4 @@ extern struct key_type cifs_idmap_key_type; extern const struct cred *root_cred; #endif /* KERNEL */ -extern int compare_sids(const struct cifs_sid *, const struct cifs_sid *); - #endif /* _CIFSACL_H */ -- cgit v1.2.3 From 36f87ee70f754d04e55518853e6fb30ed4732dda Mon Sep 17 00:00:00 2001 From: Jeff Layton Date: Sun, 25 Nov 2012 08:00:37 -0500 Subject: cifs: make cifs_copy_sid handle a source sid with variable size subauth arrays ...and lift the restriction in id_to_sid upcall that the size must be at least as big as a full cifs_sid. Reviewed-by: Shirish Pargaonkar Signed-off-by: Jeff Layton Signed-off-by: Steve French --- fs/cifs/cifsacl.h | 3 +++ 1 file changed, 3 insertions(+) (limited to 'fs/cifs/cifsacl.h') diff --git a/fs/cifs/cifsacl.h b/fs/cifs/cifsacl.h index 18c7521273a..7e52f19f996 100644 --- a/fs/cifs/cifsacl.h +++ b/fs/cifs/cifsacl.h @@ -64,6 +64,9 @@ struct cifs_sid { __le32 sub_auth[NUM_SUBAUTHS]; /* sub_auth[num_subauth] */ } __attribute__((packed)); +/* size of a struct cifs_sid, sans sub_auth array */ +#define CIFS_SID_BASE_SIZE (1 + 1 + NUM_AUTHS) + struct cifs_acl { __le16 revision; /* revision level */ __le16 size; -- cgit v1.2.3 From 30c9d6cca526243abe6c08eb6fa03db9d2b1a630 Mon Sep 17 00:00:00 2001 From: Jeff Layton Date: Sun, 25 Nov 2012 08:00:37 -0500 Subject: cifs: redefine NUM_SUBAUTH constant from 5 to 15 According to several places on the Internet and the samba winbind code, this is hard limited to 15 in windows, not 5. This does balloon out the allocation of each by 40 bytes, but I don't see any alternative. Also, rename it to SID_MAX_SUB_AUTHORITIES to match the alleged name of this constant in the windows header files Finally, rename SIDLEN to SID_STRING_MAX, fix the value to reflect the change to SID_MAX_SUB_AUTHORITIES and document how it was determined. Reviewed-by: Shirish Pargaonkar Signed-off-by: Jeff Layton Signed-off-by: Steve French --- fs/cifs/cifsacl.h | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-) (limited to 'fs/cifs/cifsacl.h') diff --git a/fs/cifs/cifsacl.h b/fs/cifs/cifsacl.h index 7e52f19f996..8b980cd445c 100644 --- a/fs/cifs/cifsacl.h +++ b/fs/cifs/cifsacl.h @@ -24,7 +24,7 @@ #define NUM_AUTHS 6 /* number of authority fields */ -#define NUM_SUBAUTHS 5 /* number of sub authority fields */ +#define SID_MAX_SUB_AUTHORITIES (15) /* max number of sub authority fields */ #define NUM_WK_SIDS 7 /* number of well known sids */ #define SIDNAMELENGTH 20 /* long enough for the ones we care about */ #define DEFSECDESCLEN 192 /* sec desc len contaiting a dacl with three aces */ @@ -41,7 +41,20 @@ #define SIDOWNER 1 #define SIDGROUP 2 -#define SIDLEN 150 /* S- 1 revision- 6 authorities- max 5 sub authorities */ + +/* + * Maximum size of a string representation of a SID: + * + * The fields are unsigned values in decimal. So: + * + * u8: max 3 bytes in decimal + * u32: max 10 bytes in decimal + * + * "S-" + 3 bytes for version field + 4 bytes for each authority field (3 bytes + * per number + 1 for '-') + 11 bytes for each subauthority field (10 bytes + * per number + 1 for '-') + NULL terminator. + */ +#define SID_STRING_MAX (195) #define SID_ID_MAPPED 0 #define SID_ID_PENDING 1 @@ -61,7 +74,7 @@ struct cifs_sid { __u8 revision; /* revision level */ __u8 num_subauth; __u8 authority[NUM_AUTHS]; - __le32 sub_auth[NUM_SUBAUTHS]; /* sub_auth[num_subauth] */ + __le32 sub_auth[SID_MAX_SUB_AUTHORITIES]; /* sub_auth[num_subauth] */ } __attribute__((packed)); /* size of a struct cifs_sid, sans sub_auth array */ -- cgit v1.2.3 From b1a6dc21d1a731fdb71fcd683ef856c6af0b3f23 Mon Sep 17 00:00:00 2001 From: Jeff Layton Date: Sun, 25 Nov 2012 08:00:38 -0500 Subject: cifs: remove uneeded __KERNEL__ block from cifsacl.h ...and make those symbols static in cifsacl.c. Nothing outside of that file refers to them. Reviewed-by: Shirish Pargaonkar Signed-off-by: Jeff Layton Signed-off-by: Steve French --- fs/cifs/cifsacl.h | 5 ----- 1 file changed, 5 deletions(-) (limited to 'fs/cifs/cifsacl.h') diff --git a/fs/cifs/cifsacl.h b/fs/cifs/cifsacl.h index 8b980cd445c..249c94f3963 100644 --- a/fs/cifs/cifsacl.h +++ b/fs/cifs/cifsacl.h @@ -109,9 +109,4 @@ struct cifs_sid_id { struct cifs_sid sid; }; -#ifdef __KERNEL__ -extern struct key_type cifs_idmap_key_type; -extern const struct cred *root_cred; -#endif /* KERNEL */ - #endif /* _CIFSACL_H */ -- cgit v1.2.3 From faa65f07d21e7d37190c91fdcf9f940d733ae3cc Mon Sep 17 00:00:00 2001 From: Jeff Layton Date: Mon, 3 Dec 2012 06:05:29 -0500 Subject: cifs: simplify id_to_sid and sid_to_id mapping code The cifs.idmap handling code currently causes the kernel to cache the data from userspace twice. It first looks in a rbtree to see if there is a matching entry for the given id. If there isn't then it calls request_key which then checks its cache and then calls out to userland if it doesn't have one. If the userland program establishes a mapping and downcalls with that info, it then gets cached in the keyring and in this rbtree. Aside from the double memory usage and the performance penalty in doing all of these extra copies, there are some nasty bugs in here too. The code declares four rbtrees and spinlocks to protect them, but only seems to use two of them. The upshot is that the same tree is used to hold (eg) uid:sid and sid:uid mappings. The comparitors aren't equipped to deal with that. I think we'd be best off to remove a layer of caching in this code. If this was originally done for performance reasons, then that really seems like a premature optimization. This patch does that -- it removes the rbtrees and the locks that protect them and simply has the code do a request_key call on each call into sid_to_id and id_to_sid. This greatly simplifies this code and should roughly halve the memory utilization from using the idmapping code. Reviewed-by: Shirish Pargaonkar Signed-off-by: Jeff Layton Signed-off-by: Steve French --- fs/cifs/cifsacl.h | 28 +++++----------------------- 1 file changed, 5 insertions(+), 23 deletions(-) (limited to 'fs/cifs/cifsacl.h') diff --git a/fs/cifs/cifsacl.h b/fs/cifs/cifsacl.h index 249c94f3963..46cd444ea2f 100644 --- a/fs/cifs/cifsacl.h +++ b/fs/cifs/cifsacl.h @@ -23,7 +23,7 @@ #define _CIFSACL_H -#define NUM_AUTHS 6 /* number of authority fields */ +#define NUM_AUTHS (6) /* number of authority fields */ #define SID_MAX_SUB_AUTHORITIES (15) /* max number of sub authority fields */ #define NUM_WK_SIDS 7 /* number of well known sids */ #define SIDNAMELENGTH 20 /* long enough for the ones we care about */ @@ -51,15 +51,12 @@ * u32: max 10 bytes in decimal * * "S-" + 3 bytes for version field + 4 bytes for each authority field (3 bytes - * per number + 1 for '-') + 11 bytes for each subauthority field (10 bytes * per number + 1 for '-') + NULL terminator. + * + * Add 11 bytes for each subauthority field (10 bytes each + 1 for '-') */ -#define SID_STRING_MAX (195) - -#define SID_ID_MAPPED 0 -#define SID_ID_PENDING 1 -#define SID_MAP_EXPIRE (3600 * HZ) /* map entry expires after one hour */ -#define SID_MAP_RETRY (300 * HZ) /* wait 5 minutes for next attempt to map */ +#define SID_STRING_BASE_SIZE (2 + 3 + (4 * NUM_AUTHS) + 1) +#define SID_STRING_SUBAUTH_SIZE (11) /* size of a single subauth string */ struct cifs_ntsd { __le16 revision; /* revision level */ @@ -94,19 +91,4 @@ struct cifs_ace { struct cifs_sid sid; /* ie UUID of user or group who gets these perms */ } __attribute__((packed)); -struct cifs_wksid { - struct cifs_sid cifssid; - char sidname[SIDNAMELENGTH]; -} __attribute__((packed)); - -struct cifs_sid_id { - unsigned int refcount; /* increment with spinlock, decrement without */ - unsigned long id; - unsigned long time; - unsigned long state; - char *sidstr; - struct rb_node rbnode; - struct cifs_sid sid; -}; - #endif /* _CIFSACL_H */ -- cgit v1.2.3 From 7ee0b4c635c091eb3c805977ba886bae2fd33f0c Mon Sep 17 00:00:00 2001 From: Jeff Layton Date: Mon, 3 Dec 2012 06:05:31 -0500 Subject: cifs: fix hardcoded default security descriptor length It was hardcoded to 192 bytes, which was not enough when the max number of subauthorities went to 15. Redefine this constant in terms of sizeof the structs involved, and rename it for better clarity. While we're at it, remove a couple more unused constants from cifsacl.h. Reviewed-by: Shirish Pargaonkar Signed-off-by: Jeff Layton Signed-off-by: Steve French --- fs/cifs/cifsacl.h | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) (limited to 'fs/cifs/cifsacl.h') diff --git a/fs/cifs/cifsacl.h b/fs/cifs/cifsacl.h index 46cd444ea2f..a445405f80d 100644 --- a/fs/cifs/cifsacl.h +++ b/fs/cifs/cifsacl.h @@ -25,9 +25,6 @@ #define NUM_AUTHS (6) /* number of authority fields */ #define SID_MAX_SUB_AUTHORITIES (15) /* max number of sub authority fields */ -#define NUM_WK_SIDS 7 /* number of well known sids */ -#define SIDNAMELENGTH 20 /* long enough for the ones we care about */ -#define DEFSECDESCLEN 192 /* sec desc len contaiting a dacl with three aces */ #define READ_BIT 0x4 #define WRITE_BIT 0x2 @@ -42,6 +39,14 @@ #define SIDOWNER 1 #define SIDGROUP 2 +/* + * Security Descriptor length containing DACL with 3 ACEs (one each for + * owner, group and world). + */ +#define DEFAULT_SEC_DESC_LEN (sizeof(struct cifs_ntsd) + \ + sizeof(struct cifs_acl) + \ + (sizeof(struct cifs_ace) * 3)) + /* * Maximum size of a string representation of a SID: * -- cgit v1.2.3 From 193cdd8a293007d1a1ad252cf66b2dc5b793d2d0 Mon Sep 17 00:00:00 2001 From: Jeff Layton Date: Mon, 10 Dec 2012 06:10:44 -0500 Subject: cifs: fix SID binary to string conversion The authority fields are supposed to be represented by a single 48-bit value. It's also supposed to represent the value as hex if it's equal to or greater than 2^32. This is documented in MS-DTYP, section 2.4.2.1. Also, fix up the max string length to account for this fix. Acked-by: Pavel Shilovsky Signed-off-by: Jeff Layton Signed-off-by: Steve French --- fs/cifs/cifsacl.h | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) (limited to 'fs/cifs/cifsacl.h') diff --git a/fs/cifs/cifsacl.h b/fs/cifs/cifsacl.h index a445405f80d..4f388483526 100644 --- a/fs/cifs/cifsacl.h +++ b/fs/cifs/cifsacl.h @@ -55,12 +55,14 @@ * u8: max 3 bytes in decimal * u32: max 10 bytes in decimal * - * "S-" + 3 bytes for version field + 4 bytes for each authority field (3 bytes - * per number + 1 for '-') + NULL terminator. + * "S-" + 3 bytes for version field + 15 for authority field + NULL terminator + * + * For authority field, max is when all 6 values are non-zero and it must be + * represented in hex. So "-0x" + 12 hex digits. * * Add 11 bytes for each subauthority field (10 bytes each + 1 for '-') */ -#define SID_STRING_BASE_SIZE (2 + 3 + (4 * NUM_AUTHS) + 1) +#define SID_STRING_BASE_SIZE (2 + 3 + 15 + 1) #define SID_STRING_SUBAUTH_SIZE (11) /* size of a single subauth string */ struct cifs_ntsd { -- cgit v1.2.3