From 380cf5ba6b0a0b307f4afb62b186ca801defb203 Mon Sep 17 00:00:00 2001 From: Andy Lutomirski Date: Thu, 23 Jun 2016 16:41:05 -0500 Subject: fs: Treat foreign mounts as nosuid If a process gets access to a mount from a different user namespace, that process should not be able to take advantage of setuid files or selinux entrypoints from that filesystem. Prevent this by treating mounts from other mount namespaces and those not owned by current_user_ns() or an ancestor as nosuid. This will make it safer to allow more complex filesystems to be mounted in non-root user namespaces. This does not remove the need for MNT_LOCK_NOSUID. The setuid, setgid, and file capability bits can no longer be abused if code in a user namespace were to clear nosuid on an untrusted filesystem, but this patch, by itself, is insufficient to protect the system from abuse of files that, when execed, would increase MAC privilege. As a more concrete explanation, any task that can manipulate a vfsmount associated with a given user namespace already has capabilities in that namespace and all of its descendents. If they can cause a malicious setuid, setgid, or file-caps executable to appear in that mount, then that executable will only allow them to elevate privileges in exactly the set of namespaces in which they are already privileges. On the other hand, if they can cause a malicious executable to appear with a dangerous MAC label, running it could change the caller's security context in a way that should not have been possible, even inside the namespace in which the task is confined. As a hardening measure, this would have made CVE-2014-5207 much more difficult to exploit. Signed-off-by: Andy Lutomirski Signed-off-by: Seth Forshee Acked-by: James Morris Acked-by: Serge Hallyn Signed-off-by: Eric W. Biederman --- security/selinux/hooks.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'security/selinux/hooks.c') diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index a86d537eb79b..15541756eb07 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -2259,7 +2259,7 @@ static int check_nnp_nosuid(const struct linux_binprm *bprm, const struct task_security_struct *new_tsec) { int nnp = (bprm->unsafe & LSM_UNSAFE_NO_NEW_PRIVS); - int nosuid = (bprm->file->f_path.mnt->mnt_flags & MNT_NOSUID); + int nosuid = !mnt_may_suid(bprm->file->f_path.mnt); int rc; if (!nnp && !nosuid) -- cgit From aad82892af261b9903cc11c55be3ecf5f0b0b4f8 Mon Sep 17 00:00:00 2001 From: Seth Forshee Date: Tue, 26 Apr 2016 14:36:20 -0500 Subject: selinux: Add support for unprivileged mounts from user namespaces Security labels from unprivileged mounts in user namespaces must be ignored. Force superblocks from user namespaces whose labeling behavior is to use xattrs to use mountpoint labeling instead. For the mountpoint label, default to converting the current task context into a form suitable for file objects, but also allow the policy writer to specify a different label through policy transition rules. Pieced together from code snippets provided by Stephen Smalley. Signed-off-by: Seth Forshee Acked-by: Stephen Smalley Acked-by: James Morris Signed-off-by: Eric W. Biederman --- security/selinux/hooks.c | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) (limited to 'security/selinux/hooks.c') diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index 15541756eb07..19be9d39c742 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -830,6 +830,28 @@ static int selinux_set_mnt_opts(struct super_block *sb, goto out; } } + + /* + * If this is a user namespace mount, no contexts are allowed + * on the command line and security labels must be ignored. + */ + if (sb->s_user_ns != &init_user_ns) { + if (context_sid || fscontext_sid || rootcontext_sid || + defcontext_sid) { + rc = -EACCES; + goto out; + } + if (sbsec->behavior == SECURITY_FS_USE_XATTR) { + sbsec->behavior = SECURITY_FS_USE_MNTPOINT; + rc = security_transition_sid(current_sid(), current_sid(), + SECCLASS_FILE, NULL, + &sbsec->mntpoint_sid); + if (rc) + goto out; + } + goto out_set_opts; + } + /* sets the context of the superblock for the fs being mounted. */ if (fscontext_sid) { rc = may_context_mount_sb_relabel(fscontext_sid, sbsec, cred); @@ -898,6 +920,7 @@ static int selinux_set_mnt_opts(struct super_block *sb, sbsec->def_sid = defcontext_sid; } +out_set_opts: rc = sb_finish_set_opts(sb); out: mutex_unlock(&sbsec->lock); -- cgit From 2917f57b6bc15cc6787496ee5f2fdf17f0e9b7d3 Mon Sep 17 00:00:00 2001 From: Huw Davies Date: Mon, 27 Jun 2016 15:06:15 -0400 Subject: calipso: Allow the lsm to label the skbuff directly. In some cases, the lsm needs to add the label to the skbuff directly. A NF_INET_LOCAL_OUT IPv6 hook is added to selinux to match the IPv4 behaviour. This allows selinux to label the skbuffs that it requires. Signed-off-by: Huw Davies Signed-off-by: Paul Moore --- include/net/netlabel.h | 11 +++ net/ipv6/calipso.c | 165 ++++++++++++++++++++++++++++++++++++++++ net/netlabel/netlabel_calipso.c | 82 ++++++++++++++++++++ net/netlabel/netlabel_calipso.h | 7 ++ net/netlabel/netlabel_kapi.c | 32 +++++++- security/selinux/hooks.c | 15 ++++ 6 files changed, 308 insertions(+), 4 deletions(-) (limited to 'security/selinux/hooks.c') diff --git a/include/net/netlabel.h b/include/net/netlabel.h index a2408c30a7f7..e0e4ce8f22af 100644 --- a/include/net/netlabel.h +++ b/include/net/netlabel.h @@ -231,6 +231,10 @@ struct netlbl_lsm_secattr { * @sock_delattr: remove the socket's attr * @req_setattr: set the req socket's attr * @req_delattr: remove the req socket's attr + * @opt_getattr: retrieve attr from memory block + * @skbuff_optptr: find option in packet + * @skbuff_setattr: set the skbuff's attr + * @skbuff_delattr: remove the skbuff's attr * * Description: * This structure is filled out by the CALIPSO engine and passed @@ -258,6 +262,13 @@ struct netlbl_calipso_ops { const struct calipso_doi *doi_def, const struct netlbl_lsm_secattr *secattr); void (*req_delattr)(struct request_sock *req); + int (*opt_getattr)(const unsigned char *calipso, + struct netlbl_lsm_secattr *secattr); + unsigned char *(*skbuff_optptr)(const struct sk_buff *skb); + int (*skbuff_setattr)(struct sk_buff *skb, + const struct calipso_doi *doi_def, + const struct netlbl_lsm_secattr *secattr); + int (*skbuff_delattr)(struct sk_buff *skb); }; /* diff --git a/net/ipv6/calipso.c b/net/ipv6/calipso.c index 067a5640ef17..fa371a8827cf 100644 --- a/net/ipv6/calipso.c +++ b/net/ipv6/calipso.c @@ -62,6 +62,11 @@ */ #define CALIPSO_OPT_LEN_MAX_WITH_PAD (3 + CALIPSO_OPT_LEN_MAX + 7) + /* Maximium size of u32 aligned buffer required to hold calipso + * option. Max of 3 initial pad bytes starting from buffer + 3. + * i.e. the worst case is when the previous tlv finishes on 4n + 3. + */ +#define CALIPSO_MAX_BUFFER (6 + CALIPSO_OPT_LEN_MAX) /* List of available DOI definitions */ static DEFINE_SPINLOCK(calipso_doi_list_lock); @@ -973,6 +978,162 @@ static void calipso_req_delattr(struct request_sock *req) kfree(new); } +/* skbuff functions. + */ + +/** + * calipso_skbuff_optptr - Find the CALIPSO option in the packet + * @skb: the packet + * + * Description: + * Parse the packet's IP header looking for a CALIPSO option. Returns a pointer + * to the start of the CALIPSO option on success, NULL if one if not found. + * + */ +static unsigned char *calipso_skbuff_optptr(const struct sk_buff *skb) +{ + const struct ipv6hdr *ip6_hdr = ipv6_hdr(skb); + int offset; + + if (ip6_hdr->nexthdr != NEXTHDR_HOP) + return NULL; + + offset = ipv6_find_tlv(skb, sizeof(*ip6_hdr), IPV6_TLV_CALIPSO); + if (offset >= 0) + return (unsigned char *)ip6_hdr + offset; + + return NULL; +} + +/** + * calipso_skbuff_setattr - Set the CALIPSO option on a packet + * @skb: the packet + * @doi_def: the CALIPSO DOI to use + * @secattr: the security attributes + * + * Description: + * Set the CALIPSO option on the given packet based on the security attributes. + * Returns a pointer to the IP header on success and NULL on failure. + * + */ +static int calipso_skbuff_setattr(struct sk_buff *skb, + const struct calipso_doi *doi_def, + const struct netlbl_lsm_secattr *secattr) +{ + int ret_val; + struct ipv6hdr *ip6_hdr; + struct ipv6_opt_hdr *hop; + unsigned char buf[CALIPSO_MAX_BUFFER]; + int len_delta, new_end, pad; + unsigned int start, end; + + ip6_hdr = ipv6_hdr(skb); + if (ip6_hdr->nexthdr == NEXTHDR_HOP) { + hop = (struct ipv6_opt_hdr *)(ip6_hdr + 1); + ret_val = calipso_opt_find(hop, &start, &end); + if (ret_val && ret_val != -ENOENT) + return ret_val; + } else { + start = 0; + end = 0; + } + + memset(buf, 0, sizeof(buf)); + ret_val = calipso_genopt(buf, start & 3, sizeof(buf), doi_def, secattr); + if (ret_val < 0) + return ret_val; + + new_end = start + ret_val; + /* At this point new_end aligns to 4n, so (new_end & 4) pads to 8n */ + pad = ((new_end & 4) + (end & 7)) & 7; + len_delta = new_end - (int)end + pad; + ret_val = skb_cow(skb, skb_headroom(skb) + len_delta); + if (ret_val < 0) + return ret_val; + + if (len_delta) { + if (len_delta > 0) + skb_push(skb, len_delta); + else + skb_pull(skb, -len_delta); + memmove((char *)ip6_hdr - len_delta, ip6_hdr, + sizeof(*ip6_hdr) + start); + skb_reset_network_header(skb); + ip6_hdr = ipv6_hdr(skb); + } + + hop = (struct ipv6_opt_hdr *)(ip6_hdr + 1); + if (start == 0) { + struct ipv6_opt_hdr *new_hop = (struct ipv6_opt_hdr *)buf; + + new_hop->nexthdr = ip6_hdr->nexthdr; + new_hop->hdrlen = len_delta / 8 - 1; + ip6_hdr->nexthdr = NEXTHDR_HOP; + } else { + hop->hdrlen += len_delta / 8; + } + memcpy((char *)hop + start, buf + (start & 3), new_end - start); + calipso_pad_write((unsigned char *)hop, new_end, pad); + + return 0; +} + +/** + * calipso_skbuff_delattr - Delete any CALIPSO options from a packet + * @skb: the packet + * + * Description: + * Removes any and all CALIPSO options from the given packet. Returns zero on + * success, negative values on failure. + * + */ +static int calipso_skbuff_delattr(struct sk_buff *skb) +{ + int ret_val; + struct ipv6hdr *ip6_hdr; + struct ipv6_opt_hdr *old_hop; + u32 old_hop_len, start = 0, end = 0, delta, size, pad; + + if (!calipso_skbuff_optptr(skb)) + return 0; + + /* since we are changing the packet we should make a copy */ + ret_val = skb_cow(skb, skb_headroom(skb)); + if (ret_val < 0) + return ret_val; + + ip6_hdr = ipv6_hdr(skb); + old_hop = (struct ipv6_opt_hdr *)(ip6_hdr + 1); + old_hop_len = ipv6_optlen(old_hop); + + ret_val = calipso_opt_find(old_hop, &start, &end); + if (ret_val) + return ret_val; + + if (start == sizeof(*old_hop) && end == old_hop_len) { + /* There's no other option in the header so we delete + * the whole thing. */ + delta = old_hop_len; + size = sizeof(*ip6_hdr); + ip6_hdr->nexthdr = old_hop->nexthdr; + } else { + delta = (end - start) & ~7; + if (delta) + old_hop->hdrlen -= delta / 8; + pad = (end - start) & 7; + size = sizeof(*ip6_hdr) + start + pad; + calipso_pad_write((unsigned char *)old_hop, start, pad); + } + + if (delta) { + skb_pull(skb, delta); + memmove((char *)ip6_hdr + delta, ip6_hdr, size); + skb_reset_network_header(skb); + } + + return 0; +} + static const struct netlbl_calipso_ops ops = { .doi_add = calipso_doi_add, .doi_free = calipso_doi_free, @@ -985,6 +1146,10 @@ static const struct netlbl_calipso_ops ops = { .sock_delattr = calipso_sock_delattr, .req_setattr = calipso_req_setattr, .req_delattr = calipso_req_delattr, + .opt_getattr = calipso_opt_getattr, + .skbuff_optptr = calipso_skbuff_optptr, + .skbuff_setattr = calipso_skbuff_setattr, + .skbuff_delattr = calipso_skbuff_delattr, }; /** diff --git a/net/netlabel/netlabel_calipso.c b/net/netlabel/netlabel_calipso.c index 79519e3a6a93..0d02c262dbf6 100644 --- a/net/netlabel/netlabel_calipso.c +++ b/net/netlabel/netlabel_calipso.c @@ -618,3 +618,85 @@ void calipso_req_delattr(struct request_sock *req) if (ops) ops->req_delattr(req); } + +/** + * calipso_optptr - Find the CALIPSO option in the packet + * @skb: the packet + * + * Description: + * Parse the packet's IP header looking for a CALIPSO option. Returns a pointer + * to the start of the CALIPSO option on success, NULL if one if not found. + * + */ +unsigned char *calipso_optptr(const struct sk_buff *skb) +{ + unsigned char *ret_val = NULL; + const struct netlbl_calipso_ops *ops = netlbl_calipso_ops_get(); + + if (ops) + ret_val = ops->skbuff_optptr(skb); + return ret_val; +} + +/** + * calipso_getattr - Get the security attributes from a memory block. + * @calipso: the CALIPSO option + * @secattr: the security attributes + * + * Description: + * Inspect @calipso and return the security attributes in @secattr. + * Returns zero on success and negative values on failure. + * + */ +int calipso_getattr(const unsigned char *calipso, + struct netlbl_lsm_secattr *secattr) +{ + int ret_val = -ENOMSG; + const struct netlbl_calipso_ops *ops = netlbl_calipso_ops_get(); + + if (ops) + ret_val = ops->opt_getattr(calipso, secattr); + return ret_val; +} + +/** + * calipso_skbuff_setattr - Set the CALIPSO option on a packet + * @skb: the packet + * @doi_def: the CALIPSO DOI to use + * @secattr: the security attributes + * + * Description: + * Set the CALIPSO option on the given packet based on the security attributes. + * Returns a pointer to the IP header on success and NULL on failure. + * + */ +int calipso_skbuff_setattr(struct sk_buff *skb, + const struct calipso_doi *doi_def, + const struct netlbl_lsm_secattr *secattr) +{ + int ret_val = -ENOMSG; + const struct netlbl_calipso_ops *ops = netlbl_calipso_ops_get(); + + if (ops) + ret_val = ops->skbuff_setattr(skb, doi_def, secattr); + return ret_val; +} + +/** + * calipso_skbuff_delattr - Delete any CALIPSO options from a packet + * @skb: the packet + * + * Description: + * Removes any and all CALIPSO options from the given packet. Returns zero on + * success, negative values on failure. + * + */ +int calipso_skbuff_delattr(struct sk_buff *skb) +{ + int ret_val = -ENOMSG; + const struct netlbl_calipso_ops *ops = netlbl_calipso_ops_get(); + + if (ops) + ret_val = ops->skbuff_delattr(skb); + return ret_val; +} diff --git a/net/netlabel/netlabel_calipso.h b/net/netlabel/netlabel_calipso.h index 1372fdd86588..66ba92e9289f 100644 --- a/net/netlabel/netlabel_calipso.h +++ b/net/netlabel/netlabel_calipso.h @@ -137,5 +137,12 @@ int calipso_req_setattr(struct request_sock *req, const struct calipso_doi *doi_def, const struct netlbl_lsm_secattr *secattr); void calipso_req_delattr(struct request_sock *req); +unsigned char *calipso_optptr(const struct sk_buff *skb); +int calipso_getattr(const unsigned char *calipso, + struct netlbl_lsm_secattr *secattr); +int calipso_skbuff_setattr(struct sk_buff *skb, + const struct calipso_doi *doi_def, + const struct netlbl_lsm_secattr *secattr); +int calipso_skbuff_delattr(struct sk_buff *skb); #endif diff --git a/net/netlabel/netlabel_kapi.c b/net/netlabel/netlabel_kapi.c index 9b725f75c750..c50a1c5d1e1a 100644 --- a/net/netlabel/netlabel_kapi.c +++ b/net/netlabel/netlabel_kapi.c @@ -1147,13 +1147,17 @@ int netlbl_skbuff_setattr(struct sk_buff *skb, { int ret_val; struct iphdr *hdr4; +#if IS_ENABLED(CONFIG_IPV6) + struct ipv6hdr *hdr6; +#endif struct netlbl_dommap_def *entry; rcu_read_lock(); switch (family) { case AF_INET: hdr4 = ip_hdr(skb); - entry = netlbl_domhsh_getentry_af4(secattr->domain,hdr4->daddr); + entry = netlbl_domhsh_getentry_af4(secattr->domain, + hdr4->daddr); if (entry == NULL) { ret_val = -ENOENT; goto skbuff_setattr_return; @@ -1174,9 +1178,26 @@ int netlbl_skbuff_setattr(struct sk_buff *skb, break; #if IS_ENABLED(CONFIG_IPV6) case AF_INET6: - /* since we don't support any IPv6 labeling protocols right - * now we can optimize everything away until we do */ - ret_val = 0; + hdr6 = ipv6_hdr(skb); + entry = netlbl_domhsh_getentry_af6(secattr->domain, + &hdr6->daddr); + if (entry == NULL) { + ret_val = -ENOENT; + goto skbuff_setattr_return; + } + switch (entry->type) { + case NETLBL_NLTYPE_CALIPSO: + ret_val = calipso_skbuff_setattr(skb, entry->calipso, + secattr); + break; + case NETLBL_NLTYPE_UNLABELED: + /* just delete the protocols we support for right now + * but we could remove other protocols if needed */ + ret_val = calipso_skbuff_delattr(skb); + break; + default: + ret_val = -ENOENT; + } break; #endif /* IPv6 */ default: @@ -1215,6 +1236,9 @@ int netlbl_skbuff_getattr(const struct sk_buff *skb, break; #if IS_ENABLED(CONFIG_IPV6) case AF_INET6: + ptr = calipso_optptr(skb); + if (ptr && calipso_getattr(ptr, secattr) == 0) + return 0; break; #endif /* IPv6 */ } diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index a00ab81ab719..cb7c5c8028e7 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -5063,6 +5063,15 @@ static unsigned int selinux_ipv4_output(void *priv, return selinux_ip_output(skb, PF_INET); } +#if defined(CONFIG_IPV6) || defined(CONFIG_IPV6_MODULE) +static unsigned int selinux_ipv6_output(void *priv, + struct sk_buff *skb, + const struct nf_hook_state *state) +{ + return selinux_ip_output(skb, PF_INET6); +} +#endif /* IPV6 */ + static unsigned int selinux_ip_postroute_compat(struct sk_buff *skb, int ifindex, u16 family) @@ -6297,6 +6306,12 @@ static struct nf_hook_ops selinux_nf_ops[] = { .hooknum = NF_INET_FORWARD, .priority = NF_IP6_PRI_SELINUX_FIRST, }, + { + .hook = selinux_ipv6_output, + .pf = NFPROTO_IPV6, + .hooknum = NF_INET_LOCAL_OUT, + .priority = NF_IP6_PRI_SELINUX_FIRST, + }, #endif /* IPV6 */ }; -- cgit From a04e71f631fa3d2fd2aa0404c11484739d1e9073 Mon Sep 17 00:00:00 2001 From: Huw Davies Date: Mon, 27 Jun 2016 15:06:16 -0400 Subject: netlabel: Pass a family parameter to netlbl_skbuff_err(). This makes it possible to route the error to the appropriate labelling engine. CALIPSO is far less verbose than CIPSO when encountering a bogus packet, so there is no need for a CALIPSO error handler. Signed-off-by: Huw Davies Signed-off-by: Paul Moore --- include/net/netlabel.h | 2 +- net/netlabel/netlabel_kapi.c | 11 ++++++++--- security/selinux/hooks.c | 6 +++--- security/selinux/include/netlabel.h | 4 +++- security/selinux/netlabel.c | 6 +++--- security/smack/smack_lsm.c | 2 +- 6 files changed, 19 insertions(+), 12 deletions(-) (limited to 'security/selinux/hooks.c') diff --git a/include/net/netlabel.h b/include/net/netlabel.h index e0e4ce8f22af..d8a46a8ed512 100644 --- a/include/net/netlabel.h +++ b/include/net/netlabel.h @@ -488,7 +488,7 @@ int netlbl_skbuff_setattr(struct sk_buff *skb, int netlbl_skbuff_getattr(const struct sk_buff *skb, u16 family, struct netlbl_lsm_secattr *secattr); -void netlbl_skbuff_err(struct sk_buff *skb, int error, int gateway); +void netlbl_skbuff_err(struct sk_buff *skb, u16 family, int error, int gateway); /* * LSM label mapping cache operations diff --git a/net/netlabel/netlabel_kapi.c b/net/netlabel/netlabel_kapi.c index c50a1c5d1e1a..a42ce3c15d70 100644 --- a/net/netlabel/netlabel_kapi.c +++ b/net/netlabel/netlabel_kapi.c @@ -1249,6 +1249,7 @@ int netlbl_skbuff_getattr(const struct sk_buff *skb, /** * netlbl_skbuff_err - Handle a LSM error on a sk_buff * @skb: the packet + * @family: the family * @error: the error code * @gateway: true if host is acting as a gateway, false otherwise * @@ -1258,10 +1259,14 @@ int netlbl_skbuff_getattr(const struct sk_buff *skb, * according to the packet's labeling protocol. * */ -void netlbl_skbuff_err(struct sk_buff *skb, int error, int gateway) +void netlbl_skbuff_err(struct sk_buff *skb, u16 family, int error, int gateway) { - if (cipso_v4_optptr(skb)) - cipso_v4_error(skb, error, gateway); + switch (family) { + case AF_INET: + if (cipso_v4_optptr(skb)) + cipso_v4_error(skb, error, gateway); + break; + } } /** diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index cb7c5c8028e7..51eafe5d3bf4 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -4603,13 +4603,13 @@ static int selinux_socket_sock_rcv_skb(struct sock *sk, struct sk_buff *skb) err = selinux_inet_sys_rcv_skb(sock_net(sk), skb->skb_iif, addrp, family, peer_sid, &ad); if (err) { - selinux_netlbl_err(skb, err, 0); + selinux_netlbl_err(skb, family, err, 0); return err; } err = avc_has_perm(sk_sid, peer_sid, SECCLASS_PEER, PEER__RECV, &ad); if (err) { - selinux_netlbl_err(skb, err, 0); + selinux_netlbl_err(skb, family, err, 0); return err; } } @@ -4977,7 +4977,7 @@ static unsigned int selinux_ip_forward(struct sk_buff *skb, err = selinux_inet_sys_rcv_skb(dev_net(indev), indev->ifindex, addrp, family, peer_sid, &ad); if (err) { - selinux_netlbl_err(skb, err, 1); + selinux_netlbl_err(skb, family, err, 1); return NF_DROP; } } diff --git a/security/selinux/include/netlabel.h b/security/selinux/include/netlabel.h index 8c59b8f150e8..75686d53df07 100644 --- a/security/selinux/include/netlabel.h +++ b/security/selinux/include/netlabel.h @@ -40,7 +40,8 @@ #ifdef CONFIG_NETLABEL void selinux_netlbl_cache_invalidate(void); -void selinux_netlbl_err(struct sk_buff *skb, int error, int gateway); +void selinux_netlbl_err(struct sk_buff *skb, u16 family, int error, + int gateway); void selinux_netlbl_sk_security_free(struct sk_security_struct *sksec); void selinux_netlbl_sk_security_reset(struct sk_security_struct *sksec); @@ -72,6 +73,7 @@ static inline void selinux_netlbl_cache_invalidate(void) } static inline void selinux_netlbl_err(struct sk_buff *skb, + u16 family, int error, int gateway) { diff --git a/security/selinux/netlabel.c b/security/selinux/netlabel.c index ca220c3fbcf9..dfca50dc292a 100644 --- a/security/selinux/netlabel.c +++ b/security/selinux/netlabel.c @@ -151,9 +151,9 @@ void selinux_netlbl_cache_invalidate(void) * present on the packet, NetLabel is smart enough to only act when it should. * */ -void selinux_netlbl_err(struct sk_buff *skb, int error, int gateway) +void selinux_netlbl_err(struct sk_buff *skb, u16 family, int error, int gateway) { - netlbl_skbuff_err(skb, error, gateway); + netlbl_skbuff_err(skb, family, error, gateway); } /** @@ -405,7 +405,7 @@ int selinux_netlbl_sock_rcv_skb(struct sk_security_struct *sksec, return 0; if (nlbl_sid != SECINITSID_UNLABELED) - netlbl_skbuff_err(skb, rc, 0); + netlbl_skbuff_err(skb, family, rc, 0); return rc; } diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c index 11f79013ae1f..292fdeaaec7e 100644 --- a/security/smack/smack_lsm.c +++ b/security/smack/smack_lsm.c @@ -3992,7 +3992,7 @@ access_check: rc = smk_bu_note("IPv4 delivery", skp, ssp->smk_in, MAY_WRITE, rc); if (rc != 0) - netlbl_skbuff_err(skb, rc, 0); + netlbl_skbuff_err(skb, sk->sk_family, rc, 0); break; #if IS_ENABLED(CONFIG_IPV6) case PF_INET6: -- cgit From 4f3ccd76572a83278166c12f3e351e74cc03578c Mon Sep 17 00:00:00 2001 From: Al Viro Date: Wed, 20 Jul 2016 16:06:15 -0400 Subject: qstr: constify dentry_init_security Signed-off-by: Al Viro --- include/linux/lsm_hooks.h | 2 +- include/linux/security.h | 4 ++-- security/security.c | 2 +- security/selinux/hooks.c | 2 +- 4 files changed, 5 insertions(+), 5 deletions(-) (limited to 'security/selinux/hooks.c') diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h index 7ae397669d8b..101bf19c0f41 100644 --- a/include/linux/lsm_hooks.h +++ b/include/linux/lsm_hooks.h @@ -1356,7 +1356,7 @@ union security_list_options { struct super_block *newsb); int (*sb_parse_opts_str)(char *options, struct security_mnt_opts *opts); int (*dentry_init_security)(struct dentry *dentry, int mode, - struct qstr *name, void **ctx, + const struct qstr *name, void **ctx, u32 *ctxlen); diff --git a/include/linux/security.h b/include/linux/security.h index 14df373ff2ca..7831cd57bcf7 100644 --- a/include/linux/security.h +++ b/include/linux/security.h @@ -240,7 +240,7 @@ int security_sb_clone_mnt_opts(const struct super_block *oldsb, struct super_block *newsb); int security_sb_parse_opts_str(char *options, struct security_mnt_opts *opts); int security_dentry_init_security(struct dentry *dentry, int mode, - struct qstr *name, void **ctx, + const struct qstr *name, void **ctx, u32 *ctxlen); int security_inode_alloc(struct inode *inode); @@ -591,7 +591,7 @@ static inline void security_inode_free(struct inode *inode) static inline int security_dentry_init_security(struct dentry *dentry, int mode, - struct qstr *name, + const struct qstr *name, void **ctx, u32 *ctxlen) { diff --git a/security/security.c b/security/security.c index 709569305d32..d441f45943e1 100644 --- a/security/security.c +++ b/security/security.c @@ -356,7 +356,7 @@ void security_inode_free(struct inode *inode) } int security_dentry_init_security(struct dentry *dentry, int mode, - struct qstr *name, void **ctx, + const struct qstr *name, void **ctx, u32 *ctxlen) { return call_int_hook(dentry_init_security, -EOPNOTSUPP, dentry, mode, diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index a86d537eb79b..22b6628041ef 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -2809,7 +2809,7 @@ static void selinux_inode_free_security(struct inode *inode) } static int selinux_dentry_init_security(struct dentry *dentry, int mode, - struct qstr *name, void **ctx, + const struct qstr *name, void **ctx, u32 *ctxlen) { u32 newsid; -- cgit From 1a93a6eac32a2853177f10e274b9b761b42356eb Mon Sep 17 00:00:00 2001 From: Javier Martinez Canillas Date: Mon, 8 Aug 2016 13:08:25 -0400 Subject: security: Use IS_ENABLED() instead of checking for built-in or module The IS_ENABLED() macro checks if a Kconfig symbol has been enabled either built-in or as a module, use that macro instead of open coding the same. Signed-off-by: Javier Martinez Canillas Acked-by: Casey Schaufler Signed-off-by: Paul Moore --- security/lsm_audit.c | 2 +- security/selinux/hooks.c | 12 ++++++------ security/smack/smack_netfilter.c | 4 ++-- 3 files changed, 9 insertions(+), 9 deletions(-) (limited to 'security/selinux/hooks.c') diff --git a/security/lsm_audit.c b/security/lsm_audit.c index cccbf3068cdc..5369036cf905 100644 --- a/security/lsm_audit.c +++ b/security/lsm_audit.c @@ -99,7 +99,7 @@ int ipv4_skb_to_auditdata(struct sk_buff *skb, } return ret; } -#if defined(CONFIG_IPV6) || defined(CONFIG_IPV6_MODULE) +#if IS_ENABLED(CONFIG_IPV6) /** * ipv6_skb_to_auditdata : fill auditdata from skb * @skb : the skb diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index 13185a6c266a..880f9533863f 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -3984,7 +3984,7 @@ out: return ret; } -#if defined(CONFIG_IPV6) || defined(CONFIG_IPV6_MODULE) +#if IS_ENABLED(CONFIG_IPV6) /* Returns error only if unable to parse addresses */ static int selinux_parse_skb_ipv6(struct sk_buff *skb, @@ -4075,7 +4075,7 @@ static int selinux_parse_skb(struct sk_buff *skb, struct common_audit_data *ad, &ad->u.net->v4info.daddr); goto okay; -#if defined(CONFIG_IPV6) || defined(CONFIG_IPV6_MODULE) +#if IS_ENABLED(CONFIG_IPV6) case PF_INET6: ret = selinux_parse_skb_ipv6(skb, ad, proto); if (ret) @@ -5029,7 +5029,7 @@ static unsigned int selinux_ipv4_forward(void *priv, return selinux_ip_forward(skb, state->in, PF_INET); } -#if defined(CONFIG_IPV6) || defined(CONFIG_IPV6_MODULE) +#if IS_ENABLED(CONFIG_IPV6) static unsigned int selinux_ipv6_forward(void *priv, struct sk_buff *skb, const struct nf_hook_state *state) @@ -5087,7 +5087,7 @@ static unsigned int selinux_ipv4_output(void *priv, return selinux_ip_output(skb, PF_INET); } -#if defined(CONFIG_IPV6) || defined(CONFIG_IPV6_MODULE) +#if IS_ENABLED(CONFIG_IPV6) static unsigned int selinux_ipv6_output(void *priv, struct sk_buff *skb, const struct nf_hook_state *state) @@ -5273,7 +5273,7 @@ static unsigned int selinux_ipv4_postroute(void *priv, return selinux_ip_postroute(skb, state->out, PF_INET); } -#if defined(CONFIG_IPV6) || defined(CONFIG_IPV6_MODULE) +#if IS_ENABLED(CONFIG_IPV6) static unsigned int selinux_ipv6_postroute(void *priv, struct sk_buff *skb, const struct nf_hook_state *state) @@ -6317,7 +6317,7 @@ static struct nf_hook_ops selinux_nf_ops[] = { .hooknum = NF_INET_LOCAL_OUT, .priority = NF_IP_PRI_SELINUX_FIRST, }, -#if defined(CONFIG_IPV6) || defined(CONFIG_IPV6_MODULE) +#if IS_ENABLED(CONFIG_IPV6) { .hook = selinux_ipv6_postroute, .pf = NFPROTO_IPV6, diff --git a/security/smack/smack_netfilter.c b/security/smack/smack_netfilter.c index aa6bf1b22ec5..205b785fb400 100644 --- a/security/smack/smack_netfilter.c +++ b/security/smack/smack_netfilter.c @@ -20,7 +20,7 @@ #include #include "smack.h" -#if defined(CONFIG_IPV6) || defined(CONFIG_IPV6_MODULE) +#if IS_ENABLED(CONFIG_IPV6) static unsigned int smack_ipv6_output(void *priv, struct sk_buff *skb, @@ -64,7 +64,7 @@ static struct nf_hook_ops smack_nf_ops[] = { .hooknum = NF_INET_LOCAL_OUT, .priority = NF_IP_PRI_SELINUX_FIRST, }, -#if defined(CONFIG_IPV6) || defined(CONFIG_IPV6_MODULE) +#if IS_ENABLED(CONFIG_IPV6) { .hook = smack_ipv6_output, .pf = NFPROTO_IPV6, -- cgit From 56909eb3f559103196ecbf2c08c923e0804980fb Mon Sep 17 00:00:00 2001 From: Vivek Goyal Date: Wed, 13 Jul 2016 10:44:48 -0400 Subject: selinux: Implementation for inode_copy_up() hook A file is being copied up for overlay file system. Prepare a new set of creds and set create_sid appropriately so that new file is created with appropriate label. Overlay inode has right label for both context and non-context mount cases. In case of non-context mount, overlay inode will have the label of lower file and in case of context mount, overlay inode will have the label from context= mount option. Signed-off-by: Vivek Goyal Acked-by: Stephen Smalley Signed-off-by: Paul Moore --- security/selinux/hooks.c | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) (limited to 'security/selinux/hooks.c') diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index 880f9533863f..40597ed00ba9 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -3293,6 +3293,26 @@ static void selinux_inode_getsecid(struct inode *inode, u32 *secid) *secid = isec->sid; } +static int selinux_inode_copy_up(struct dentry *src, struct cred **new) +{ + u32 sid; + struct task_security_struct *tsec; + struct cred *new_creds = *new; + + if (new_creds == NULL) { + new_creds = prepare_creds(); + if (!new_creds) + return -ENOMEM; + } + + tsec = new_creds->security; + /* Get label from overlay inode and set it in create_sid */ + selinux_inode_getsecid(d_inode(src), &sid); + tsec->create_sid = sid; + *new = new_creds; + return 0; +} + /* file security operations */ static int selinux_revalidate_file_permission(struct file *file, int mask) @@ -6088,6 +6108,7 @@ static struct security_hook_list selinux_hooks[] = { LSM_HOOK_INIT(inode_setsecurity, selinux_inode_setsecurity), LSM_HOOK_INIT(inode_listsecurity, selinux_inode_listsecurity), LSM_HOOK_INIT(inode_getsecid, selinux_inode_getsecid), + LSM_HOOK_INIT(inode_copy_up, selinux_inode_copy_up), LSM_HOOK_INIT(file_permission, selinux_file_permission), LSM_HOOK_INIT(file_alloc_security, selinux_file_alloc_security), -- cgit From 19472b69d639d58415866bf127d5f9005038c105 Mon Sep 17 00:00:00 2001 From: Vivek Goyal Date: Wed, 13 Jul 2016 10:44:50 -0400 Subject: selinux: Implementation for inode_copy_up_xattr() hook When a file is copied up in overlay, we have already created file on upper/ with right label and there is no need to copy up selinux label/xattr from lower file to upper file. In fact in case of context mount, we don't want to copy up label as newly created file got its label from context= option. Signed-off-by: Vivek Goyal Acked-by: Stephen Smalley Signed-off-by: Paul Moore --- security/selinux/hooks.c | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) (limited to 'security/selinux/hooks.c') diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index 40597ed00ba9..a2d510895ff3 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -3313,6 +3313,21 @@ static int selinux_inode_copy_up(struct dentry *src, struct cred **new) return 0; } +static int selinux_inode_copy_up_xattr(const char *name) +{ + /* The copy_up hook above sets the initial context on an inode, but we + * don't then want to overwrite it by blindly copying all the lower + * xattrs up. Instead, we have to filter out SELinux-related xattrs. + */ + if (strcmp(name, XATTR_NAME_SELINUX) == 0) + return 1; /* Discard */ + /* + * Any other attribute apart from SELINUX is not claimed, supported + * by selinux. + */ + return -EOPNOTSUPP; +} + /* file security operations */ static int selinux_revalidate_file_permission(struct file *file, int mask) @@ -6109,6 +6124,7 @@ static struct security_hook_list selinux_hooks[] = { LSM_HOOK_INIT(inode_listsecurity, selinux_inode_listsecurity), LSM_HOOK_INIT(inode_getsecid, selinux_inode_getsecid), LSM_HOOK_INIT(inode_copy_up, selinux_inode_copy_up), + LSM_HOOK_INIT(inode_copy_up_xattr, selinux_inode_copy_up_xattr), LSM_HOOK_INIT(file_permission, selinux_file_permission), LSM_HOOK_INIT(file_alloc_security, selinux_file_alloc_security), -- cgit From c957f6df52c509ccfbb96659fd1a0f7812de333f Mon Sep 17 00:00:00 2001 From: Vivek Goyal Date: Wed, 13 Jul 2016 10:44:51 -0400 Subject: selinux: Pass security pointer to determine_inode_label() Right now selinux_determine_inode_label() works on security pointer of current task. Soon I need this to work on a security pointer retrieved from a set of creds. So start passing in a pointer and caller can decide where to fetch security pointer from. Signed-off-by: Vivek Goyal Acked-by: Stephen Smalley Signed-off-by: Paul Moore --- security/selinux/hooks.c | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) (limited to 'security/selinux/hooks.c') diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index a2d510895ff3..f9d398bc9dcd 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -1808,13 +1808,13 @@ out: /* * Determine the label for an inode that might be unioned. */ -static int selinux_determine_inode_label(struct inode *dir, - const struct qstr *name, - u16 tclass, - u32 *_new_isid) +static int +selinux_determine_inode_label(const struct task_security_struct *tsec, + struct inode *dir, + const struct qstr *name, u16 tclass, + u32 *_new_isid) { const struct superblock_security_struct *sbsec = dir->i_sb->s_security; - const struct task_security_struct *tsec = current_security(); if ((sbsec->flags & SE_SBINITIALIZED) && (sbsec->behavior == SECURITY_FS_USE_MNTPOINT)) { @@ -1857,8 +1857,8 @@ static int may_create(struct inode *dir, if (rc) return rc; - rc = selinux_determine_inode_label(dir, &dentry->d_name, tclass, - &newsid); + rc = selinux_determine_inode_label(current_security(), dir, + &dentry->d_name, tclass, &newsid); if (rc) return rc; @@ -2838,7 +2838,8 @@ static int selinux_dentry_init_security(struct dentry *dentry, int mode, u32 newsid; int rc; - rc = selinux_determine_inode_label(d_inode(dentry->d_parent), name, + rc = selinux_determine_inode_label(current_security(), + d_inode(dentry->d_parent), name, inode_mode_to_security_class(mode), &newsid); if (rc) @@ -2863,7 +2864,7 @@ static int selinux_inode_init_security(struct inode *inode, struct inode *dir, sid = tsec->sid; newsid = tsec->create_sid; - rc = selinux_determine_inode_label( + rc = selinux_determine_inode_label(current_security(), dir, qstr, inode_mode_to_security_class(inode->i_mode), &newsid); -- cgit From a518b0a5b0d7f3397e065acb956bca9635aa892d Mon Sep 17 00:00:00 2001 From: Vivek Goyal Date: Wed, 13 Jul 2016 10:44:53 -0400 Subject: selinux: Implement dentry_create_files_as() hook Calculate what would be the label of newly created file and set that secid in the passed creds. Context of the task which is actually creating file is retrieved from set of creds passed in. (old->security). Signed-off-by: Vivek Goyal Acked-by: Stephen Smalley Signed-off-by: Paul Moore --- security/selinux/hooks.c | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) (limited to 'security/selinux/hooks.c') diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index f9d398bc9dcd..e15e56081c0c 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -2848,6 +2848,27 @@ static int selinux_dentry_init_security(struct dentry *dentry, int mode, return security_sid_to_context(newsid, (char **)ctx, ctxlen); } +static int selinux_dentry_create_files_as(struct dentry *dentry, int mode, + struct qstr *name, + const struct cred *old, + struct cred *new) +{ + u32 newsid; + int rc; + struct task_security_struct *tsec; + + rc = selinux_determine_inode_label(old->security, + d_inode(dentry->d_parent), name, + inode_mode_to_security_class(mode), + &newsid); + if (rc) + return rc; + + tsec = new->security; + tsec->create_sid = newsid; + return 0; +} + static int selinux_inode_init_security(struct inode *inode, struct inode *dir, const struct qstr *qstr, const char **name, @@ -6098,6 +6119,7 @@ static struct security_hook_list selinux_hooks[] = { LSM_HOOK_INIT(sb_parse_opts_str, selinux_parse_opts_str), LSM_HOOK_INIT(dentry_init_security, selinux_dentry_init_security), + LSM_HOOK_INIT(dentry_create_files_as, selinux_dentry_create_files_as), LSM_HOOK_INIT(inode_alloc_security, selinux_inode_alloc_security), LSM_HOOK_INIT(inode_free_security, selinux_inode_free_security), -- cgit From 43af5de74288a7cdc3684902c5259346ae67adf8 Mon Sep 17 00:00:00 2001 From: Vivek Goyal Date: Fri, 9 Sep 2016 11:37:49 -0400 Subject: lsm,audit,selinux: Introduce a new audit data type LSM_AUDIT_DATA_FILE Right now LSM_AUDIT_DATA_PATH type contains "struct path" in union "u" of common_audit_data. This information is used to print path of file at the same time it is also used to get to dentry and inode. And this inode information is used to get to superblock and device and print device information. This does not work well for layered filesystems like overlay where dentry contained in path is overlay dentry and not the real dentry of underlying file system. That means inode retrieved from dentry is also overlay inode and not the real inode. SELinux helpers like file_path_has_perm() are doing checks on inode retrieved from file_inode(). This returns the real inode and not the overlay inode. That means we are doing check on real inode but for audit purposes we are printing details of overlay inode and that can be confusing while debugging. Hence, introduce a new type LSM_AUDIT_DATA_FILE which carries file information and inode retrieved is real inode using file_inode(). That way right avc denied information is given to user. For example, following is one example avc before the patch. type=AVC msg=audit(1473360868.399:214): avc: denied { read open } for pid=1765 comm="cat" path="/root/.../overlay/container1/merged/readfile" dev="overlay" ino=21443 scontext=unconfined_u:unconfined_r:test_overlay_client_t:s0:c10,c20 tcontext=unconfined_u:object_r:test_overlay_files_ro_t:s0 tclass=file permissive=0 It looks as follows after the patch. type=AVC msg=audit(1473360017.388:282): avc: denied { read open } for pid=2530 comm="cat" path="/root/.../overlay/container1/merged/readfile" dev="dm-0" ino=2377915 scontext=unconfined_u:unconfined_r:test_overlay_client_t:s0:c10,c20 tcontext=unconfined_u:object_r:test_overlay_files_ro_t:s0 tclass=file permissive=0 Notice that now dev information points to "dm-0" device instead of "overlay" device. This makes it clear that check failed on underlying inode and not on the overlay inode. Signed-off-by: Vivek Goyal [PM: slight tweaks to the description to make checkpatch.pl happy] Signed-off-by: Paul Moore --- include/linux/lsm_audit.h | 2 ++ security/lsm_audit.c | 13 +++++++++++++ security/selinux/hooks.c | 16 ++++++++-------- 3 files changed, 23 insertions(+), 8 deletions(-) (limited to 'security/selinux/hooks.c') diff --git a/include/linux/lsm_audit.h b/include/linux/lsm_audit.h index ffb9c9da4f39..e58e577117b6 100644 --- a/include/linux/lsm_audit.h +++ b/include/linux/lsm_audit.h @@ -59,6 +59,7 @@ struct common_audit_data { #define LSM_AUDIT_DATA_INODE 9 #define LSM_AUDIT_DATA_DENTRY 10 #define LSM_AUDIT_DATA_IOCTL_OP 11 +#define LSM_AUDIT_DATA_FILE 12 union { struct path path; struct dentry *dentry; @@ -75,6 +76,7 @@ struct common_audit_data { #endif char *kmod_name; struct lsm_ioctlop_audit *op; + struct file *file; } u; /* this union contains LSM specific data */ union { diff --git a/security/lsm_audit.c b/security/lsm_audit.c index 9bf851884800..8bfa3147fd54 100644 --- a/security/lsm_audit.c +++ b/security/lsm_audit.c @@ -245,6 +245,19 @@ static void dump_common_audit_data(struct audit_buffer *ab, } break; } + case LSM_AUDIT_DATA_FILE: { + struct inode *inode; + + audit_log_d_path(ab, " path=", &a->u.file->f_path); + + inode = file_inode(a->u.file); + if (inode) { + audit_log_format(ab, " dev="); + audit_log_untrustedstring(ab, inode->i_sb->s_id); + audit_log_format(ab, " ino=%lu", inode->i_ino); + } + break; + } case LSM_AUDIT_DATA_IOCTL_OP: { struct inode *inode; diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index e15e56081c0c..2205ea27aa0a 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -1761,8 +1761,8 @@ static inline int file_path_has_perm(const struct cred *cred, { struct common_audit_data ad; - ad.type = LSM_AUDIT_DATA_PATH; - ad.u.path = file->f_path; + ad.type = LSM_AUDIT_DATA_FILE; + ad.u.file = file; return inode_has_perm(cred, file_inode(file), av, &ad); } @@ -1784,8 +1784,8 @@ static int file_has_perm(const struct cred *cred, u32 sid = cred_sid(cred); int rc; - ad.type = LSM_AUDIT_DATA_PATH; - ad.u.path = file->f_path; + ad.type = LSM_AUDIT_DATA_FILE; + ad.u.file = file; if (sid != fsec->sid) { rc = avc_has_perm(sid, fsec->sid, @@ -2365,8 +2365,8 @@ static int selinux_bprm_set_creds(struct linux_binprm *bprm) new_tsec->sid = old_tsec->sid; } - ad.type = LSM_AUDIT_DATA_PATH; - ad.u.path = bprm->file->f_path; + ad.type = LSM_AUDIT_DATA_FILE; + ad.u.file = bprm->file; if (new_tsec->sid == old_tsec->sid) { rc = avc_has_perm(old_tsec->sid, isec->sid, @@ -3833,8 +3833,8 @@ static int selinux_kernel_module_from_file(struct file *file) /* finit_module */ - ad.type = LSM_AUDIT_DATA_PATH; - ad.u.path = file->f_path; + ad.type = LSM_AUDIT_DATA_FILE; + ad.u.file = file; fsec = file->f_security; if (sid != fsec->sid) { -- cgit From 5d6c31910bc0713e37628dc0ce677dcb13c8ccf4 Mon Sep 17 00:00:00 2001 From: Andreas Gruenbacher Date: Thu, 29 Sep 2016 17:48:42 +0200 Subject: xattr: Add __vfs_{get,set,remove}xattr helpers Right now, various places in the kernel check for the existence of getxattr, setxattr, and removexattr inode operations and directly call those operations. Switch to helper functions and test for the IOP_XATTR flag instead. Signed-off-by: Andreas Gruenbacher Acked-by: James Morris Signed-off-by: Al Viro --- fs/cachefiles/bind.c | 4 ++-- fs/cachefiles/namei.c | 4 ++-- fs/ecryptfs/inode.c | 18 +++++++------- fs/ecryptfs/mmap.c | 13 +++++------ fs/overlayfs/copy_up.c | 4 ++-- fs/overlayfs/super.c | 4 ++-- fs/xattr.c | 44 ++++++++++++++++++++++++++--------- include/linux/xattr.h | 3 +++ security/commoncap.c | 25 ++++++++------------ security/integrity/evm/evm_crypto.c | 7 +++--- security/integrity/evm/evm_main.c | 4 ++-- security/integrity/ima/ima_appraise.c | 21 ++++++++--------- security/selinux/hooks.c | 19 ++++++--------- security/smack/smack_lsm.c | 12 +++++----- 14 files changed, 97 insertions(+), 85 deletions(-) (limited to 'security/selinux/hooks.c') diff --git a/fs/cachefiles/bind.c b/fs/cachefiles/bind.c index 6af790fc3df8..3ff867f87d73 100644 --- a/fs/cachefiles/bind.c +++ b/fs/cachefiles/bind.c @@ -20,6 +20,7 @@ #include #include #include +#include #include "internal.h" static int cachefiles_daemon_add_cache(struct cachefiles_cache *caches); @@ -126,8 +127,7 @@ static int cachefiles_daemon_add_cache(struct cachefiles_cache *cache) if (d_is_negative(root) || !d_backing_inode(root)->i_op->lookup || !d_backing_inode(root)->i_op->mkdir || - !d_backing_inode(root)->i_op->setxattr || - !d_backing_inode(root)->i_op->getxattr || + !(d_backing_inode(root)->i_opflags & IOP_XATTR) || !root->d_sb->s_op->statfs || !root->d_sb->s_op->sync_fs) goto error_unsupported; diff --git a/fs/cachefiles/namei.c b/fs/cachefiles/namei.c index 3f7c2cd41f8f..6eb3dec2adbb 100644 --- a/fs/cachefiles/namei.c +++ b/fs/cachefiles/namei.c @@ -20,6 +20,7 @@ #include #include #include +#include #include "internal.h" #define CACHEFILES_KEYBUF_SIZE 512 @@ -799,8 +800,7 @@ struct dentry *cachefiles_get_directory(struct cachefiles_cache *cache, } ret = -EPERM; - if (!d_backing_inode(subdir)->i_op->setxattr || - !d_backing_inode(subdir)->i_op->getxattr || + if (!(d_backing_inode(subdir)->i_opflags & IOP_XATTR) || !d_backing_inode(subdir)->i_op->lookup || !d_backing_inode(subdir)->i_op->mkdir || !d_backing_inode(subdir)->i_op->create || diff --git a/fs/ecryptfs/inode.c b/fs/ecryptfs/inode.c index cef32ccc3fcb..32fee255d7b5 100644 --- a/fs/ecryptfs/inode.c +++ b/fs/ecryptfs/inode.c @@ -1005,15 +1005,14 @@ ecryptfs_setxattr(struct dentry *dentry, struct inode *inode, const char *name, const void *value, size_t size, int flags) { - int rc = 0; + int rc; struct dentry *lower_dentry; lower_dentry = ecryptfs_dentry_to_lower(dentry); - if (!d_inode(lower_dentry)->i_op->setxattr) { + if (!(d_inode(lower_dentry)->i_opflags & IOP_XATTR)) { rc = -EOPNOTSUPP; goto out; } - rc = vfs_setxattr(lower_dentry, name, value, size, flags); if (!rc && inode) fsstack_copy_attr_all(inode, d_inode(lower_dentry)); @@ -1025,15 +1024,14 @@ ssize_t ecryptfs_getxattr_lower(struct dentry *lower_dentry, struct inode *lower_inode, const char *name, void *value, size_t size) { - int rc = 0; + int rc; - if (!lower_inode->i_op->getxattr) { + if (!(lower_inode->i_opflags & IOP_XATTR)) { rc = -EOPNOTSUPP; goto out; } inode_lock(lower_inode); - rc = lower_inode->i_op->getxattr(lower_dentry, lower_inode, - name, value, size); + rc = __vfs_getxattr(lower_dentry, lower_inode, name, value, size); inode_unlock(lower_inode); out: return rc; @@ -1069,18 +1067,18 @@ out: static int ecryptfs_removexattr(struct dentry *dentry, struct inode *inode, const char *name) { - int rc = 0; + int rc; struct dentry *lower_dentry; struct inode *lower_inode; lower_dentry = ecryptfs_dentry_to_lower(dentry); lower_inode = ecryptfs_inode_to_lower(inode); - if (!lower_inode->i_op->removexattr) { + if (!(lower_inode->i_opflags & IOP_XATTR)) { rc = -EOPNOTSUPP; goto out; } inode_lock(lower_inode); - rc = lower_inode->i_op->removexattr(lower_dentry, name); + rc = __vfs_removexattr(lower_dentry, name); inode_unlock(lower_inode); out: return rc; diff --git a/fs/ecryptfs/mmap.c b/fs/ecryptfs/mmap.c index 9c3437c8a5b1..1f0c471b4ba3 100644 --- a/fs/ecryptfs/mmap.c +++ b/fs/ecryptfs/mmap.c @@ -32,6 +32,7 @@ #include #include #include +#include #include #include "ecryptfs_kernel.h" @@ -422,7 +423,7 @@ static int ecryptfs_write_inode_size_to_xattr(struct inode *ecryptfs_inode) struct inode *lower_inode = d_inode(lower_dentry); int rc; - if (!lower_inode->i_op->getxattr || !lower_inode->i_op->setxattr) { + if (!(lower_inode->i_opflags & IOP_XATTR)) { printk(KERN_WARNING "No support for setting xattr in lower filesystem\n"); rc = -ENOSYS; @@ -436,15 +437,13 @@ static int ecryptfs_write_inode_size_to_xattr(struct inode *ecryptfs_inode) goto out; } inode_lock(lower_inode); - size = lower_inode->i_op->getxattr(lower_dentry, lower_inode, - ECRYPTFS_XATTR_NAME, - xattr_virt, PAGE_SIZE); + size = __vfs_getxattr(lower_dentry, lower_inode, ECRYPTFS_XATTR_NAME, + xattr_virt, PAGE_SIZE); if (size < 0) size = 8; put_unaligned_be64(i_size_read(ecryptfs_inode), xattr_virt); - rc = lower_inode->i_op->setxattr(lower_dentry, lower_inode, - ECRYPTFS_XATTR_NAME, - xattr_virt, size, 0); + rc = __vfs_setxattr(lower_dentry, lower_inode, ECRYPTFS_XATTR_NAME, + xattr_virt, size, 0); inode_unlock(lower_inode); if (rc) printk(KERN_ERR "Error whilst attempting to write inode size " diff --git a/fs/overlayfs/copy_up.c b/fs/overlayfs/copy_up.c index 43fdc2765aea..807951cb438c 100644 --- a/fs/overlayfs/copy_up.c +++ b/fs/overlayfs/copy_up.c @@ -58,8 +58,8 @@ int ovl_copy_xattr(struct dentry *old, struct dentry *new) char *buf, *name, *value = NULL; int uninitialized_var(error); - if (!old->d_inode->i_op->getxattr || - !new->d_inode->i_op->getxattr) + if (!(old->d_inode->i_opflags & IOP_XATTR) || + !(new->d_inode->i_opflags & IOP_XATTR)) return 0; list_size = vfs_listxattr(old, NULL, 0); diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c index e2a94a26767b..f170114481f7 100644 --- a/fs/overlayfs/super.c +++ b/fs/overlayfs/super.c @@ -275,10 +275,10 @@ static bool ovl_is_opaquedir(struct dentry *dentry) char val; struct inode *inode = dentry->d_inode; - if (!S_ISDIR(inode->i_mode) || !inode->i_op->getxattr) + if (!S_ISDIR(inode->i_mode) || !(inode->i_opflags & IOP_XATTR)) return false; - res = inode->i_op->getxattr(dentry, inode, OVL_XATTR_OPAQUE, &val, 1); + res = __vfs_getxattr(dentry, inode, OVL_XATTR_OPAQUE, &val, 1); if (res == 1 && val == 'y') return true; diff --git a/fs/xattr.c b/fs/xattr.c index 1f5d0b48422a..54a411519127 100644 --- a/fs/xattr.c +++ b/fs/xattr.c @@ -136,6 +136,16 @@ xattr_permission(struct inode *inode, const char *name, int mask) return inode_permission(inode, mask); } +int +__vfs_setxattr(struct dentry *dentry, struct inode *inode, const char *name, + const void *value, size_t size, int flags) +{ + if (!inode->i_op->setxattr) + return -EOPNOTSUPP; + return inode->i_op->setxattr(dentry, inode, name, value, size, flags); +} +EXPORT_SYMBOL(__vfs_setxattr); + /** * __vfs_setxattr_noperm - perform setxattr operation without performing * permission checks. @@ -163,7 +173,7 @@ int __vfs_setxattr_noperm(struct dentry *dentry, const char *name, if (issec) inode->i_flags &= ~S_NOSEC; if (inode->i_op->setxattr) { - error = inode->i_op->setxattr(dentry, inode, name, value, size, flags); + error = __vfs_setxattr(dentry, inode, name, value, size, flags); if (!error) { fsnotify_xattr(dentry); security_inode_post_setxattr(dentry, name, value, @@ -274,6 +284,16 @@ vfs_getxattr_alloc(struct dentry *dentry, const char *name, char **xattr_value, return error; } +ssize_t +__vfs_getxattr(struct dentry *dentry, struct inode *inode, const char *name, + void *value, size_t size) +{ + if (!inode->i_op->getxattr) + return -EOPNOTSUPP; + return inode->i_op->getxattr(dentry, inode, name, value, size); +} +EXPORT_SYMBOL(__vfs_getxattr); + ssize_t vfs_getxattr(struct dentry *dentry, const char *name, void *value, size_t size) { @@ -301,13 +321,7 @@ vfs_getxattr(struct dentry *dentry, const char *name, void *value, size_t size) return ret; } nolsm: - if (inode->i_op->getxattr) - error = inode->i_op->getxattr(dentry, inode, name, value, size); - else - error = -EOPNOTSUPP; - - return error; - + return __vfs_getxattr(dentry, inode, name, value, size); } EXPORT_SYMBOL_GPL(vfs_getxattr); @@ -332,13 +346,21 @@ vfs_listxattr(struct dentry *d, char *list, size_t size) EXPORT_SYMBOL_GPL(vfs_listxattr); int -vfs_removexattr(struct dentry *dentry, const char *name) +__vfs_removexattr(struct dentry *dentry, const char *name) { struct inode *inode = dentry->d_inode; - int error; if (!inode->i_op->removexattr) return -EOPNOTSUPP; + return inode->i_op->removexattr(dentry, name); +} +EXPORT_SYMBOL(__vfs_removexattr); + +int +vfs_removexattr(struct dentry *dentry, const char *name) +{ + struct inode *inode = dentry->d_inode; + int error; error = xattr_permission(inode, name, MAY_WRITE); if (error) @@ -349,7 +371,7 @@ vfs_removexattr(struct dentry *dentry, const char *name) if (error) goto out; - error = inode->i_op->removexattr(dentry, name); + error = __vfs_removexattr(dentry, name); if (!error) { fsnotify_xattr(dentry); diff --git a/include/linux/xattr.h b/include/linux/xattr.h index 94079bab9243..6ae6b2e68efb 100644 --- a/include/linux/xattr.h +++ b/include/linux/xattr.h @@ -46,10 +46,13 @@ struct xattr { }; ssize_t xattr_getsecurity(struct inode *, const char *, void *, size_t); +ssize_t __vfs_getxattr(struct dentry *, struct inode *, const char *, void *, size_t); ssize_t vfs_getxattr(struct dentry *, const char *, void *, size_t); ssize_t vfs_listxattr(struct dentry *d, char *list, size_t size); +int __vfs_setxattr(struct dentry *, struct inode *, const char *, const void *, size_t, int); int __vfs_setxattr_noperm(struct dentry *, const char *, const void *, size_t, int); int vfs_setxattr(struct dentry *, const char *, const void *, size_t, int); +int __vfs_removexattr(struct dentry *, const char *); int vfs_removexattr(struct dentry *, const char *); ssize_t generic_getxattr(struct dentry *dentry, struct inode *inode, const char *name, void *buffer, size_t size); diff --git a/security/commoncap.c b/security/commoncap.c index 14540bd78561..8df676fbd393 100644 --- a/security/commoncap.c +++ b/security/commoncap.c @@ -310,13 +310,8 @@ int cap_inode_need_killpriv(struct dentry *dentry) struct inode *inode = d_backing_inode(dentry); int error; - if (!inode->i_op->getxattr) - return 0; - - error = inode->i_op->getxattr(dentry, inode, XATTR_NAME_CAPS, NULL, 0); - if (error <= 0) - return 0; - return 1; + error = __vfs_getxattr(dentry, inode, XATTR_NAME_CAPS, NULL, 0); + return error > 0; } /** @@ -329,12 +324,12 @@ int cap_inode_need_killpriv(struct dentry *dentry) */ int cap_inode_killpriv(struct dentry *dentry) { - struct inode *inode = d_backing_inode(dentry); - - if (!inode->i_op->removexattr) - return 0; + int error; - return inode->i_op->removexattr(dentry, XATTR_NAME_CAPS); + error = __vfs_removexattr(dentry, XATTR_NAME_CAPS); + if (error == -EOPNOTSUPP) + error = 0; + return error; } /* @@ -394,11 +389,11 @@ int get_vfs_caps_from_disk(const struct dentry *dentry, struct cpu_vfs_cap_data memset(cpu_caps, 0, sizeof(struct cpu_vfs_cap_data)); - if (!inode || !inode->i_op->getxattr) + if (!inode) return -ENODATA; - size = inode->i_op->getxattr((struct dentry *)dentry, inode, - XATTR_NAME_CAPS, &caps, XATTR_CAPS_SZ); + size = __vfs_getxattr((struct dentry *)dentry, inode, + XATTR_NAME_CAPS, &caps, XATTR_CAPS_SZ); if (size == -ENODATA || size == -EOPNOTSUPP) /* no data, that's ok */ return -ENODATA; diff --git a/security/integrity/evm/evm_crypto.c b/security/integrity/evm/evm_crypto.c index 11c1d30bd705..bf663915412e 100644 --- a/security/integrity/evm/evm_crypto.c +++ b/security/integrity/evm/evm_crypto.c @@ -182,8 +182,9 @@ static int evm_calc_hmac_or_hash(struct dentry *dentry, int error; int size; - if (!inode->i_op->getxattr) + if (!(inode->i_opflags & IOP_XATTR)) return -EOPNOTSUPP; + desc = init_desc(type); if (IS_ERR(desc)) return PTR_ERR(desc); @@ -253,8 +254,8 @@ int evm_update_evmxattr(struct dentry *dentry, const char *xattr_name, rc = __vfs_setxattr_noperm(dentry, XATTR_NAME_EVM, &xattr_data, sizeof(xattr_data), 0); - } else if (rc == -ENODATA && inode->i_op->removexattr) { - rc = inode->i_op->removexattr(dentry, XATTR_NAME_EVM); + } else if (rc == -ENODATA && (inode->i_opflags & IOP_XATTR)) { + rc = __vfs_removexattr(dentry, XATTR_NAME_EVM); } return rc; } diff --git a/security/integrity/evm/evm_main.c b/security/integrity/evm/evm_main.c index b9e26288d30c..ba8615576d4d 100644 --- a/security/integrity/evm/evm_main.c +++ b/security/integrity/evm/evm_main.c @@ -78,11 +78,11 @@ static int evm_find_protected_xattrs(struct dentry *dentry) int error; int count = 0; - if (!inode->i_op->getxattr) + if (!(inode->i_opflags & IOP_XATTR)) return -EOPNOTSUPP; for (xattr = evm_config_xattrnames; *xattr != NULL; xattr++) { - error = inode->i_op->getxattr(dentry, inode, *xattr, NULL, 0); + error = __vfs_getxattr(dentry, inode, *xattr, NULL, 0); if (error < 0) { if (error == -ENODATA) continue; diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c index 4b9b4a4e1b89..0cc40af9c218 100644 --- a/security/integrity/ima/ima_appraise.c +++ b/security/integrity/ima/ima_appraise.c @@ -165,13 +165,13 @@ enum hash_algo ima_get_hash_algo(struct evm_ima_xattr_data *xattr_value, int ima_read_xattr(struct dentry *dentry, struct evm_ima_xattr_data **xattr_value) { - struct inode *inode = d_backing_inode(dentry); - - if (!inode->i_op->getxattr) - return 0; + ssize_t ret; - return vfs_getxattr_alloc(dentry, XATTR_NAME_IMA, (char **)xattr_value, - 0, GFP_NOFS); + ret = vfs_getxattr_alloc(dentry, XATTR_NAME_IMA, (char **)xattr_value, + 0, GFP_NOFS); + if (ret == -EOPNOTSUPP) + ret = 0; + return ret; } /* @@ -195,7 +195,7 @@ int ima_appraise_measurement(enum ima_hooks func, enum integrity_status status = INTEGRITY_UNKNOWN; int rc = xattr_len, hash_start = 0; - if (!inode->i_op->getxattr) + if (!(inode->i_opflags & IOP_XATTR)) return INTEGRITY_UNKNOWN; if (rc <= 0) { @@ -322,10 +322,10 @@ void ima_inode_post_setattr(struct dentry *dentry) { struct inode *inode = d_backing_inode(dentry); struct integrity_iint_cache *iint; - int must_appraise, rc; + int must_appraise; if (!(ima_policy_flag & IMA_APPRAISE) || !S_ISREG(inode->i_mode) - || !inode->i_op->removexattr) + || !(inode->i_opflags & IOP_XATTR)) return; must_appraise = ima_must_appraise(inode, MAY_ACCESS, POST_SETATTR); @@ -338,8 +338,7 @@ void ima_inode_post_setattr(struct dentry *dentry) iint->flags |= IMA_APPRAISE; } if (!must_appraise) - rc = inode->i_op->removexattr(dentry, XATTR_NAME_IMA); - return; + __vfs_removexattr(dentry, XATTR_NAME_IMA); } /* diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index 13185a6c266a..3db31ac7986b 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -507,14 +507,14 @@ static int sb_finish_set_opts(struct super_block *sb) the root directory. -ENODATA is ok, as this may be the first boot of the SELinux kernel before we have assigned xattr values to the filesystem. */ - if (!root_inode->i_op->getxattr) { + if (!(root_inode->i_opflags & IOP_XATTR)) { printk(KERN_WARNING "SELinux: (dev %s, type %s) has no " "xattr support\n", sb->s_id, sb->s_type->name); rc = -EOPNOTSUPP; goto out; } - rc = root_inode->i_op->getxattr(root, root_inode, - XATTR_NAME_SELINUX, NULL, 0); + + rc = __vfs_getxattr(root, root_inode, XATTR_NAME_SELINUX, NULL, 0); if (rc < 0 && rc != -ENODATA) { if (rc == -EOPNOTSUPP) printk(KERN_WARNING "SELinux: (dev %s, type " @@ -1410,11 +1410,10 @@ static int inode_doinit_with_dentry(struct inode *inode, struct dentry *opt_dent case SECURITY_FS_USE_NATIVE: break; case SECURITY_FS_USE_XATTR: - if (!inode->i_op->getxattr) { + if (!(inode->i_opflags & IOP_XATTR)) { isec->sid = sbsec->def_sid; break; } - /* Need a dentry, since the xattr API requires one. Life would be simpler if we could just pass the inode. */ if (opt_dentry) { @@ -1445,14 +1444,12 @@ static int inode_doinit_with_dentry(struct inode *inode, struct dentry *opt_dent goto out_unlock; } context[len] = '\0'; - rc = inode->i_op->getxattr(dentry, inode, XATTR_NAME_SELINUX, - context, len); + rc = __vfs_getxattr(dentry, inode, XATTR_NAME_SELINUX, context, len); if (rc == -ERANGE) { kfree(context); /* Need a larger buffer. Query for the right size. */ - rc = inode->i_op->getxattr(dentry, inode, XATTR_NAME_SELINUX, - NULL, 0); + rc = __vfs_getxattr(dentry, inode, XATTR_NAME_SELINUX, NULL, 0); if (rc < 0) { dput(dentry); goto out_unlock; @@ -1465,9 +1462,7 @@ static int inode_doinit_with_dentry(struct inode *inode, struct dentry *opt_dent goto out_unlock; } context[len] = '\0'; - rc = inode->i_op->getxattr(dentry, inode, - XATTR_NAME_SELINUX, - context, len); + rc = __vfs_getxattr(dentry, inode, XATTR_NAME_SELINUX, context, len); } dput(dentry); if (rc < 0) { diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c index 87a9741b0d02..516b3f50f23d 100644 --- a/security/smack/smack_lsm.c +++ b/security/smack/smack_lsm.c @@ -265,14 +265,14 @@ static struct smack_known *smk_fetch(const char *name, struct inode *ip, char *buffer; struct smack_known *skp = NULL; - if (ip->i_op->getxattr == NULL) + if (!(ip->i_opflags & IOP_XATTR)) return ERR_PTR(-EOPNOTSUPP); buffer = kzalloc(SMK_LONGLABEL, GFP_KERNEL); if (buffer == NULL) return ERR_PTR(-ENOMEM); - rc = ip->i_op->getxattr(dp, ip, name, buffer, SMK_LONGLABEL); + rc = __vfs_getxattr(dp, ip, name, buffer, SMK_LONGLABEL); if (rc < 0) skp = ERR_PTR(rc); else if (rc == 0) @@ -3520,8 +3520,8 @@ static void smack_d_instantiate(struct dentry *opt_dentry, struct inode *inode) * It would be curious if the label of the task * does not match that assigned. */ - if (inode->i_op->getxattr == NULL) - break; + if (!(inode->i_opflags & IOP_XATTR)) + break; /* * Get the dentry for xattr. */ @@ -3545,12 +3545,12 @@ static void smack_d_instantiate(struct dentry *opt_dentry, struct inode *inode) */ if (isp->smk_flags & SMK_INODE_CHANGED) { isp->smk_flags &= ~SMK_INODE_CHANGED; - rc = inode->i_op->setxattr(dp, inode, + rc = __vfs_setxattr(dp, inode, XATTR_NAME_SMACKTRANSMUTE, TRANS_TRUE, TRANS_TRUE_SIZE, 0); } else { - rc = inode->i_op->getxattr(dp, inode, + rc = __vfs_getxattr(dp, inode, XATTR_NAME_SMACKTRANSMUTE, trattr, TRANS_TRUE_SIZE); if (rc >= 0 && strncmp(trattr, TRANS_TRUE, -- cgit