* [PATCH v18 18/23] NET: Store LSM netlabel data in a lsmblob
From: Casey Schaufler @ 2020-07-09 0:12 UTC (permalink / raw)
To: casey.schaufler, jmorris, linux-security-module, selinux
Cc: casey, keescook, john.johansen, penguin-kernel, paul, sds, netdev
In-Reply-To: <20200709001234.9719-1-casey@schaufler-ca.com>
Netlabel uses LSM interfaces requiring an lsmblob and
the internal storage is used to pass information between
these interfaces, so change the internal data from a secid
to a lsmblob. Update the netlabel interfaces and their
callers to accommodate the change. This requires that the
modules using netlabel use the lsm_id.slot to access the
correct secid when using netlabel.
Reviewed-by: Kees Cook <keescook@chromium.org>
Reviewed-by: John Johansen <john.johansen@canonical.com>
Acked-by: Stephen Smalley <sds@tycho.nsa.gov>
Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
Cc: netdev@vger.kernel.org
---
include/net/netlabel.h | 8 +--
net/ipv4/cipso_ipv4.c | 27 ++++++----
net/netlabel/netlabel_kapi.c | 6 +--
net/netlabel/netlabel_unlabeled.c | 79 +++++++++--------------------
net/netlabel/netlabel_unlabeled.h | 2 +-
security/selinux/hooks.c | 2 +-
security/selinux/include/security.h | 1 +
security/selinux/netlabel.c | 2 +-
security/selinux/ss/services.c | 4 +-
security/smack/smack.h | 1 +
security/smack/smack_lsm.c | 5 +-
security/smack/smackfs.c | 10 ++--
12 files changed, 65 insertions(+), 82 deletions(-)
diff --git a/include/net/netlabel.h b/include/net/netlabel.h
index 43ae50337685..73fc25b4042b 100644
--- a/include/net/netlabel.h
+++ b/include/net/netlabel.h
@@ -166,7 +166,7 @@ struct netlbl_lsm_catmap {
* @attr.mls: MLS sensitivity label
* @attr.mls.cat: MLS category bitmap
* @attr.mls.lvl: MLS sensitivity level
- * @attr.secid: LSM specific secid token
+ * @attr.lsmblob: LSM specific data
*
* Description:
* This structure is used to pass security attributes between NetLabel and the
@@ -201,7 +201,7 @@ struct netlbl_lsm_secattr {
struct netlbl_lsm_catmap *cat;
u32 lvl;
} mls;
- u32 secid;
+ struct lsmblob lsmblob;
} attr;
};
@@ -415,7 +415,7 @@ int netlbl_cfg_unlbl_static_add(struct net *net,
const void *addr,
const void *mask,
u16 family,
- u32 secid,
+ struct lsmblob *lsmblob,
struct netlbl_audit *audit_info);
int netlbl_cfg_unlbl_static_del(struct net *net,
const char *dev_name,
@@ -523,7 +523,7 @@ static inline int netlbl_cfg_unlbl_static_add(struct net *net,
const void *addr,
const void *mask,
u16 family,
- u32 secid,
+ struct lsmblob *lsmblob,
struct netlbl_audit *audit_info)
{
return -ENOSYS;
diff --git a/net/ipv4/cipso_ipv4.c b/net/ipv4/cipso_ipv4.c
index a23094b050f8..469baf6704f5 100644
--- a/net/ipv4/cipso_ipv4.c
+++ b/net/ipv4/cipso_ipv4.c
@@ -106,15 +106,17 @@ int cipso_v4_rbm_strictvalid = 1;
/* Base length of the local tag (non-standard tag).
* Tag definition (may change between kernel versions)
*
- * 0 8 16 24 32
- * +----------+----------+----------+----------+
- * | 10000000 | 00000110 | 32-bit secid value |
- * +----------+----------+----------+----------+
- * | in (host byte order)|
- * +----------+----------+
- *
+ * 0 8 16 16 + sizeof(struct lsmblob)
+ * +----------+----------+---------------------+
+ * | 10000000 | 00000110 | LSM blob data |
+ * +----------+----------+---------------------+
+ *
+ * All secid and flag fields are in host byte order.
+ * The lsmblob structure size varies depending on which
+ * Linux security modules are built in the kernel.
+ * The data is opaque.
*/
-#define CIPSO_V4_TAG_LOC_BLEN 6
+#define CIPSO_V4_TAG_LOC_BLEN (2 + sizeof(struct lsmblob))
/*
* Helper Functions
@@ -1469,7 +1471,12 @@ static int cipso_v4_gentag_loc(const struct cipso_v4_doi *doi_def,
buffer[0] = CIPSO_V4_TAG_LOCAL;
buffer[1] = CIPSO_V4_TAG_LOC_BLEN;
- *(u32 *)&buffer[2] = secattr->attr.secid;
+ /* Ensure that there is sufficient space in the CIPSO header
+ * for the LSM data. This should never become an issue.
+ * The check is made from an abundance of caution. */
+ BUILD_BUG_ON(CIPSO_V4_TAG_LOC_BLEN > CIPSO_V4_OPT_LEN_MAX);
+ memcpy(&buffer[2], &secattr->attr.lsmblob,
+ sizeof(secattr->attr.lsmblob));
return CIPSO_V4_TAG_LOC_BLEN;
}
@@ -1489,7 +1496,7 @@ static int cipso_v4_parsetag_loc(const struct cipso_v4_doi *doi_def,
const unsigned char *tag,
struct netlbl_lsm_secattr *secattr)
{
- secattr->attr.secid = *(u32 *)&tag[2];
+ memcpy(&secattr->attr.lsmblob, &tag[2], sizeof(secattr->attr.lsmblob));
secattr->flags |= NETLBL_SECATTR_SECID;
return 0;
diff --git a/net/netlabel/netlabel_kapi.c b/net/netlabel/netlabel_kapi.c
index 5e1239cef000..bbfaff539416 100644
--- a/net/netlabel/netlabel_kapi.c
+++ b/net/netlabel/netlabel_kapi.c
@@ -196,7 +196,7 @@ int netlbl_cfg_unlbl_map_add(const char *domain,
* @addr: IP address in network byte order (struct in[6]_addr)
* @mask: address mask in network byte order (struct in[6]_addr)
* @family: address family
- * @secid: LSM secid value for the entry
+ * @lsmblob: LSM data value for the entry
* @audit_info: NetLabel audit information
*
* Description:
@@ -210,7 +210,7 @@ int netlbl_cfg_unlbl_static_add(struct net *net,
const void *addr,
const void *mask,
u16 family,
- u32 secid,
+ struct lsmblob *lsmblob,
struct netlbl_audit *audit_info)
{
u32 addr_len;
@@ -230,7 +230,7 @@ int netlbl_cfg_unlbl_static_add(struct net *net,
return netlbl_unlhsh_add(net,
dev_name, addr, mask, addr_len,
- secid, audit_info);
+ lsmblob, audit_info);
}
/**
diff --git a/net/netlabel/netlabel_unlabeled.c b/net/netlabel/netlabel_unlabeled.c
index cf4c56beb3ec..c14a485ff045 100644
--- a/net/netlabel/netlabel_unlabeled.c
+++ b/net/netlabel/netlabel_unlabeled.c
@@ -66,7 +66,7 @@ struct netlbl_unlhsh_tbl {
#define netlbl_unlhsh_addr4_entry(iter) \
container_of(iter, struct netlbl_unlhsh_addr4, list)
struct netlbl_unlhsh_addr4 {
- u32 secid;
+ struct lsmblob lsmblob;
struct netlbl_af4list list;
struct rcu_head rcu;
@@ -74,7 +74,7 @@ struct netlbl_unlhsh_addr4 {
#define netlbl_unlhsh_addr6_entry(iter) \
container_of(iter, struct netlbl_unlhsh_addr6, list)
struct netlbl_unlhsh_addr6 {
- u32 secid;
+ struct lsmblob lsmblob;
struct netlbl_af6list list;
struct rcu_head rcu;
@@ -220,7 +220,7 @@ static struct netlbl_unlhsh_iface *netlbl_unlhsh_search_iface(int ifindex)
* @iface: the associated interface entry
* @addr: IPv4 address in network byte order
* @mask: IPv4 address mask in network byte order
- * @secid: LSM secid value for entry
+ * @lsmblob: LSM data value for entry
*
* Description:
* Add a new address entry into the unlabeled connection hash table using the
@@ -231,7 +231,7 @@ static struct netlbl_unlhsh_iface *netlbl_unlhsh_search_iface(int ifindex)
static int netlbl_unlhsh_add_addr4(struct netlbl_unlhsh_iface *iface,
const struct in_addr *addr,
const struct in_addr *mask,
- u32 secid)
+ struct lsmblob *lsmblob)
{
int ret_val;
struct netlbl_unlhsh_addr4 *entry;
@@ -243,7 +243,7 @@ static int netlbl_unlhsh_add_addr4(struct netlbl_unlhsh_iface *iface,
entry->list.addr = addr->s_addr & mask->s_addr;
entry->list.mask = mask->s_addr;
entry->list.valid = 1;
- entry->secid = secid;
+ entry->lsmblob = *lsmblob;
spin_lock(&netlbl_unlhsh_lock);
ret_val = netlbl_af4list_add(&entry->list, &iface->addr4_list);
@@ -260,7 +260,7 @@ static int netlbl_unlhsh_add_addr4(struct netlbl_unlhsh_iface *iface,
* @iface: the associated interface entry
* @addr: IPv6 address in network byte order
* @mask: IPv6 address mask in network byte order
- * @secid: LSM secid value for entry
+ * @lsmblob: LSM data value for entry
*
* Description:
* Add a new address entry into the unlabeled connection hash table using the
@@ -271,7 +271,7 @@ static int netlbl_unlhsh_add_addr4(struct netlbl_unlhsh_iface *iface,
static int netlbl_unlhsh_add_addr6(struct netlbl_unlhsh_iface *iface,
const struct in6_addr *addr,
const struct in6_addr *mask,
- u32 secid)
+ struct lsmblob *lsmblob)
{
int ret_val;
struct netlbl_unlhsh_addr6 *entry;
@@ -287,7 +287,7 @@ static int netlbl_unlhsh_add_addr6(struct netlbl_unlhsh_iface *iface,
entry->list.addr.s6_addr32[3] &= mask->s6_addr32[3];
entry->list.mask = *mask;
entry->list.valid = 1;
- entry->secid = secid;
+ entry->lsmblob = *lsmblob;
spin_lock(&netlbl_unlhsh_lock);
ret_val = netlbl_af6list_add(&entry->list, &iface->addr6_list);
@@ -366,7 +366,7 @@ int netlbl_unlhsh_add(struct net *net,
const void *addr,
const void *mask,
u32 addr_len,
- u32 secid,
+ struct lsmblob *lsmblob,
struct netlbl_audit *audit_info)
{
int ret_val;
@@ -375,7 +375,6 @@ int netlbl_unlhsh_add(struct net *net,
struct netlbl_unlhsh_iface *iface;
struct audit_buffer *audit_buf = NULL;
struct lsmcontext context;
- struct lsmblob blob;
if (addr_len != sizeof(struct in_addr) &&
addr_len != sizeof(struct in6_addr))
@@ -408,7 +407,7 @@ int netlbl_unlhsh_add(struct net *net,
const struct in_addr *addr4 = addr;
const struct in_addr *mask4 = mask;
- ret_val = netlbl_unlhsh_add_addr4(iface, addr4, mask4, secid);
+ ret_val = netlbl_unlhsh_add_addr4(iface, addr4, mask4, lsmblob);
if (audit_buf != NULL)
netlbl_af4list_audit_addr(audit_buf, 1,
dev_name,
@@ -421,7 +420,7 @@ int netlbl_unlhsh_add(struct net *net,
const struct in6_addr *addr6 = addr;
const struct in6_addr *mask6 = mask;
- ret_val = netlbl_unlhsh_add_addr6(iface, addr6, mask6, secid);
+ ret_val = netlbl_unlhsh_add_addr6(iface, addr6, mask6, lsmblob);
if (audit_buf != NULL)
netlbl_af6list_audit_addr(audit_buf, 1,
dev_name,
@@ -438,11 +437,7 @@ int netlbl_unlhsh_add(struct net *net,
unlhsh_add_return:
rcu_read_unlock();
if (audit_buf != NULL) {
- /* lsmblob_init() puts secid into all of the secids in blob.
- * security_secid_to_secctx() will know which security module
- * to use to create the secctx. */
- lsmblob_init(&blob, secid);
- if (security_secid_to_secctx(&blob, &context) == 0) {
+ if (security_secid_to_secctx(lsmblob, &context) == 0) {
audit_log_format(audit_buf, " sec_obj=%s",
context.context);
security_release_secctx(&context);
@@ -477,7 +472,6 @@ static int netlbl_unlhsh_remove_addr4(struct net *net,
struct audit_buffer *audit_buf;
struct net_device *dev;
struct lsmcontext context;
- struct lsmblob blob;
spin_lock(&netlbl_unlhsh_lock);
list_entry = netlbl_af4list_remove(addr->s_addr, mask->s_addr,
@@ -497,13 +491,8 @@ static int netlbl_unlhsh_remove_addr4(struct net *net,
addr->s_addr, mask->s_addr);
if (dev != NULL)
dev_put(dev);
- /* lsmblob_init() puts entry->secid into all of the secids
- * in blob. security_secid_to_secctx() will know which
- * security module to use to create the secctx. */
- if (entry != NULL)
- lsmblob_init(&blob, entry->secid);
if (entry != NULL &&
- security_secid_to_secctx(&blob, &context) == 0) {
+ security_secid_to_secctx(&entry->lsmblob, &context) == 0) {
audit_log_format(audit_buf, " sec_obj=%s",
context.context);
security_release_secctx(&context);
@@ -544,7 +533,6 @@ static int netlbl_unlhsh_remove_addr6(struct net *net,
struct audit_buffer *audit_buf;
struct net_device *dev;
struct lsmcontext context;
- struct lsmblob blob;
spin_lock(&netlbl_unlhsh_lock);
list_entry = netlbl_af6list_remove(addr, mask, &iface->addr6_list);
@@ -563,13 +551,8 @@ static int netlbl_unlhsh_remove_addr6(struct net *net,
addr, mask);
if (dev != NULL)
dev_put(dev);
- /* lsmblob_init() puts entry->secid into all of the secids
- * in blob. security_secid_to_secctx() will know which
- * security module to use to create the secctx. */
- if (entry != NULL)
- lsmblob_init(&blob, entry->secid);
if (entry != NULL &&
- security_secid_to_secctx(&blob, &context) == 0) {
+ security_secid_to_secctx(&entry->lsmblob, &context) == 0) {
audit_log_format(audit_buf, " sec_obj=%s",
context.context);
security_release_secctx(&context);
@@ -923,14 +906,8 @@ static int netlbl_unlabel_staticadd(struct sk_buff *skb,
if (ret_val != 0)
return ret_val;
- /* netlbl_unlhsh_add will be changed to pass a struct lsmblob *
- * instead of a u32 later in this patch set. security_secctx_to_secid()
- * will only be setting one entry in the lsmblob struct, so it is
- * safe to use lsmblob_value() to get that one value. */
-
- return netlbl_unlhsh_add(&init_net,
- dev_name, addr, mask, addr_len,
- lsmblob_value(&blob), &audit_info);
+ return netlbl_unlhsh_add(&init_net, dev_name, addr, mask, addr_len,
+ &blob, &audit_info);
}
/**
@@ -977,11 +954,8 @@ static int netlbl_unlabel_staticadddef(struct sk_buff *skb,
if (ret_val != 0)
return ret_val;
- /* security_secctx_to_secid() will only put one secid into the lsmblob
- * so it's safe to use lsmblob_value() to get the secid. */
- return netlbl_unlhsh_add(&init_net,
- NULL, addr, mask, addr_len,
- lsmblob_value(&blob), &audit_info);
+ return netlbl_unlhsh_add(&init_net, NULL, addr, mask, addr_len, &blob,
+ &audit_info);
}
/**
@@ -1093,8 +1067,7 @@ static int netlbl_unlabel_staticlist_gen(u32 cmd,
struct net_device *dev;
struct lsmcontext context;
void *data;
- u32 secid;
- struct lsmblob blob;
+ struct lsmblob *lsmb;
data = genlmsg_put(cb_arg->skb, NETLINK_CB(cb_arg->nl_cb->skb).portid,
cb_arg->seq, &netlbl_unlabel_gnl_family,
@@ -1132,7 +1105,7 @@ static int netlbl_unlabel_staticlist_gen(u32 cmd,
if (ret_val != 0)
goto list_cb_failure;
- secid = addr4->secid;
+ lsmb = (struct lsmblob *)&addr4->lsmblob;
} else {
ret_val = nla_put_in6_addr(cb_arg->skb,
NLBL_UNLABEL_A_IPV6ADDR,
@@ -1146,14 +1119,10 @@ static int netlbl_unlabel_staticlist_gen(u32 cmd,
if (ret_val != 0)
goto list_cb_failure;
- secid = addr6->secid;
+ lsmb = (struct lsmblob *)&addr6->lsmblob;
}
- /* lsmblob_init() secid into all of the secids in blob.
- * security_secid_to_secctx() will know which security module
- * to use to create the secctx. */
- lsmblob_init(&blob, secid);
- ret_val = security_secid_to_secctx(&blob, &context);
+ ret_val = security_secid_to_secctx(lsmb, &context);
if (ret_val != 0)
goto list_cb_failure;
ret_val = nla_put(cb_arg->skb,
@@ -1505,7 +1474,7 @@ int netlbl_unlabel_getattr(const struct sk_buff *skb,
&iface->addr4_list);
if (addr4 == NULL)
goto unlabel_getattr_nolabel;
- secattr->attr.secid = netlbl_unlhsh_addr4_entry(addr4)->secid;
+ secattr->attr.lsmblob = netlbl_unlhsh_addr4_entry(addr4)->lsmblob;
break;
}
#if IS_ENABLED(CONFIG_IPV6)
@@ -1518,7 +1487,7 @@ int netlbl_unlabel_getattr(const struct sk_buff *skb,
&iface->addr6_list);
if (addr6 == NULL)
goto unlabel_getattr_nolabel;
- secattr->attr.secid = netlbl_unlhsh_addr6_entry(addr6)->secid;
+ secattr->attr.lsmblob = netlbl_unlhsh_addr6_entry(addr6)->lsmblob;
break;
}
#endif /* IPv6 */
diff --git a/net/netlabel/netlabel_unlabeled.h b/net/netlabel/netlabel_unlabeled.h
index 058e3a285d56..168920780994 100644
--- a/net/netlabel/netlabel_unlabeled.h
+++ b/net/netlabel/netlabel_unlabeled.h
@@ -211,7 +211,7 @@ int netlbl_unlhsh_add(struct net *net,
const void *addr,
const void *mask,
u32 addr_len,
- u32 secid,
+ struct lsmblob *lsmblob,
struct netlbl_audit *audit_info);
int netlbl_unlhsh_remove(struct net *net,
const char *dev_name,
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index bedcf737ff26..c13c207c5da1 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -6940,7 +6940,7 @@ static int selinux_perf_event_write(struct perf_event *event)
}
#endif
-static struct lsm_id selinux_lsmid __lsm_ro_after_init = {
+struct lsm_id selinux_lsmid __lsm_ro_after_init = {
.lsm = "selinux",
.slot = LSMBLOB_NEEDED
};
diff --git a/security/selinux/include/security.h b/security/selinux/include/security.h
index b0e02cfe3ce1..cee2987647dd 100644
--- a/security/selinux/include/security.h
+++ b/security/selinux/include/security.h
@@ -71,6 +71,7 @@
struct netlbl_lsm_secattr;
extern int selinux_enabled_boot;
+extern struct lsm_id selinux_lsmid;
/* Policy capabilities */
enum {
diff --git a/security/selinux/netlabel.c b/security/selinux/netlabel.c
index 6a94b31b5472..d8d7603ab14e 100644
--- a/security/selinux/netlabel.c
+++ b/security/selinux/netlabel.c
@@ -108,7 +108,7 @@ static struct netlbl_lsm_secattr *selinux_netlbl_sock_getattr(
return NULL;
if ((secattr->flags & NETLBL_SECATTR_SECID) &&
- (secattr->attr.secid == sid))
+ (secattr->attr.lsmblob.secid[selinux_lsmid.slot] == sid))
return secattr;
return NULL;
diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c
index ef0afd878bfc..c59ecf7b61b7 100644
--- a/security/selinux/ss/services.c
+++ b/security/selinux/ss/services.c
@@ -3637,7 +3637,7 @@ int security_netlbl_secattr_to_sid(struct selinux_state *state,
if (secattr->flags & NETLBL_SECATTR_CACHE)
*sid = *(u32 *)secattr->cache->data;
else if (secattr->flags & NETLBL_SECATTR_SECID)
- *sid = secattr->attr.secid;
+ *sid = secattr->attr.lsmblob.secid[selinux_lsmid.slot];
else if (secattr->flags & NETLBL_SECATTR_MLS_LVL) {
rc = -EIDRM;
ctx = sidtab_search(sidtab, SECINITSID_NETMSG);
@@ -3710,7 +3710,7 @@ int security_netlbl_sid_to_secattr(struct selinux_state *state,
if (secattr->domain == NULL)
goto out;
- secattr->attr.secid = sid;
+ secattr->attr.lsmblob.secid[selinux_lsmid.slot] = sid;
secattr->flags |= NETLBL_SECATTR_DOMAIN_CPY | NETLBL_SECATTR_SECID;
mls_export_netlbl_lvl(policydb, ctx, secattr);
rc = mls_export_netlbl_cat(policydb, ctx, secattr);
diff --git a/security/smack/smack.h b/security/smack/smack.h
index 7481fa71de19..c284b104e1cc 100644
--- a/security/smack/smack.h
+++ b/security/smack/smack.h
@@ -310,6 +310,7 @@ void smk_destroy_label_list(struct list_head *list);
* Shared data.
*/
extern int smack_enabled;
+extern struct lsm_id smack_lsmid;
extern int smack_cipso_direct;
extern int smack_cipso_mapped;
extern struct smack_known *smack_net_ambient;
diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
index 8b708cca921a..6f0cdb40addc 100644
--- a/security/smack/smack_lsm.c
+++ b/security/smack/smack_lsm.c
@@ -3751,7 +3751,8 @@ static struct smack_known *smack_from_secattr(struct netlbl_lsm_secattr *sap,
/*
* Looks like a fallback, which gives us a secid.
*/
- return smack_from_secid(sap->attr.secid);
+ return smack_from_secid(
+ sap->attr.lsmblob.secid[smack_lsmid.slot]);
/*
* Without guidance regarding the smack value
* for the packet fall back on the network
@@ -4656,7 +4657,7 @@ struct lsm_blob_sizes smack_blob_sizes __lsm_ro_after_init = {
.lbs_sock = sizeof(struct socket_smack),
};
-static struct lsm_id smack_lsmid __lsm_ro_after_init = {
+struct lsm_id smack_lsmid __lsm_ro_after_init = {
.lsm = "smack",
.slot = LSMBLOB_NEEDED
};
diff --git a/security/smack/smackfs.c b/security/smack/smackfs.c
index c21b656b3263..177e69b43a52 100644
--- a/security/smack/smackfs.c
+++ b/security/smack/smackfs.c
@@ -1132,6 +1132,7 @@ static void smk_net4addr_insert(struct smk_net4addr *new)
static ssize_t smk_write_net4addr(struct file *file, const char __user *buf,
size_t count, loff_t *ppos)
{
+ struct lsmblob lsmblob;
struct smk_net4addr *snp;
struct sockaddr_in newname;
char *smack;
@@ -1263,10 +1264,13 @@ static ssize_t smk_write_net4addr(struct file *file, const char __user *buf,
* this host so that incoming packets get labeled.
* but only if we didn't get the special CIPSO option
*/
- if (rc == 0 && skp != NULL)
+ if (rc == 0 && skp != NULL) {
+ lsmblob_init(&lsmblob, 0);
+ lsmblob.secid[smack_lsmid.slot] = snp->smk_label->smk_secid;
rc = netlbl_cfg_unlbl_static_add(&init_net, NULL,
- &snp->smk_host, &snp->smk_mask, PF_INET,
- snp->smk_label->smk_secid, &audit_info);
+ &snp->smk_host, &snp->smk_mask, PF_INET, &lsmblob,
+ &audit_info);
+ }
if (rc == 0)
rc = count;
--
2.24.1
^ permalink raw reply related
* [PATCH v18 19/23] LSM: Verify LSM display sanity in binder
From: Casey Schaufler @ 2020-07-09 0:12 UTC (permalink / raw)
To: casey.schaufler, jmorris, linux-security-module, selinux
Cc: casey, keescook, john.johansen, penguin-kernel, paul, sds
In-Reply-To: <20200709001234.9719-1-casey@schaufler-ca.com>
Verify that the tasks on the ends of a binder transaction
use the same "display" security module. This prevents confusion
of security "contexts".
Reviewed-by: Kees Cook <keescook@chromium.org>
Acked-by: Stephen Smalley <sds@tycho.nsa.gov>
Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
---
security/security.c | 29 +++++++++++++++++++++++++++++
1 file changed, 29 insertions(+)
diff --git a/security/security.c b/security/security.c
index d75ad048ffc9..c3fa4b7e9338 100644
--- a/security/security.c
+++ b/security/security.c
@@ -788,9 +788,38 @@ int security_binder_set_context_mgr(struct task_struct *mgr)
return call_int_hook(binder_set_context_mgr, 0, mgr);
}
+/**
+ * security_binder_transaction - Binder driver transaction check
+ * @from: source of the transaction
+ * @to: destination of the transaction
+ *
+ * Verify that the tasks have the same LSM "display", then
+ * call the security module hooks.
+ *
+ * Returns -EINVAL if the displays don't match, or the
+ * result of the security module checks.
+ */
int security_binder_transaction(struct task_struct *from,
struct task_struct *to)
{
+ int from_display = lsm_task_display(from);
+ int to_display = lsm_task_display(to);
+
+ /*
+ * If the display is LSMBLOB_INVALID the first module that has
+ * an entry is used. This will be in the 0 slot.
+ *
+ * This is currently only required if the server has requested
+ * peer contexts, but it would be unwieldly to have too much of
+ * the binder driver detail here.
+ */
+ if (from_display == LSMBLOB_INVALID)
+ from_display = 0;
+ if (to_display == LSMBLOB_INVALID)
+ to_display = 0;
+ if (from_display != to_display)
+ return -EINVAL;
+
return call_int_hook(binder_transaction, 0, from, to);
}
--
2.24.1
^ permalink raw reply related
* [PATCH v18 20/23] Audit: Add new record for multiple process LSM attributes
From: Casey Schaufler @ 2020-07-09 0:12 UTC (permalink / raw)
To: casey.schaufler, jmorris, linux-security-module, selinux
Cc: casey, keescook, john.johansen, penguin-kernel, paul, sds,
linux-audit
In-Reply-To: <20200709001234.9719-1-casey@schaufler-ca.com>
Create a new audit record type to contain the subject information
when there are multiple security modules that require such data.
This record is linked with the same timestamp and serial number.
The record is produced only in cases where there is more than one
security module with a process "context".
Before this change the only audit events that required multiple
records were syscall events. Several non-syscall events include
subject contexts, so the use of audit_context data has been expanded
as necessary.
Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
Cc: linux-audit@redhat.com
---
drivers/android/binder.c | 2 +-
include/linux/audit.h | 13 +++-
include/linux/security.h | 9 ++-
include/net/netlabel.h | 2 +-
include/net/scm.h | 3 +-
include/net/xfrm.h | 4 +-
include/uapi/linux/audit.h | 1 +
kernel/audit.c | 90 ++++++++++++++++++-------
kernel/audit.h | 5 ++
kernel/auditfilter.c | 2 +-
kernel/auditsc.c | 87 ++++++++++++++++++++++--
net/ipv4/ip_sockglue.c | 2 +-
net/netfilter/nf_conntrack_netlink.c | 4 +-
net/netfilter/nf_conntrack_standalone.c | 2 +-
net/netfilter/nfnetlink_queue.c | 2 +-
net/netlabel/netlabel_unlabeled.c | 16 ++---
net/netlabel/netlabel_user.c | 12 ++--
net/netlabel/netlabel_user.h | 6 +-
security/integrity/integrity_audit.c | 2 +-
security/security.c | 71 ++++++++++++++-----
security/smack/smackfs.c | 3 +-
21 files changed, 255 insertions(+), 83 deletions(-)
diff --git a/drivers/android/binder.c b/drivers/android/binder.c
index ceb5987c7d76..99b1e65b193c 100644
--- a/drivers/android/binder.c
+++ b/drivers/android/binder.c
@@ -3109,7 +3109,7 @@ static void binder_transaction(struct binder_proc *proc,
size_t added_size;
security_task_getsecid(proc->tsk, &blob);
- ret = security_secid_to_secctx(&blob, &lsmctx);
+ ret = security_secid_to_secctx(&blob, &lsmctx, LSMBLOB_DISPLAY);
if (ret) {
return_error = BR_FAILED_REPLY;
return_error_param = ret;
diff --git a/include/linux/audit.h b/include/linux/audit.h
index aabbbe6d9296..4693febcf41f 100644
--- a/include/linux/audit.h
+++ b/include/linux/audit.h
@@ -168,7 +168,9 @@ extern void audit_log_path_denied(int type,
const char *operation);
extern void audit_log_lost(const char *message);
-extern int audit_log_task_context(struct audit_buffer *ab);
+extern void audit_log_lsm(struct lsmblob *blob, bool exiting);
+extern int audit_log_task_context(struct audit_buffer *ab,
+ struct lsmblob *blob);
extern void audit_log_task_info(struct audit_buffer *ab);
extern int audit_update_lsm_rules(void);
@@ -228,7 +230,10 @@ static inline void audit_log_key(struct audit_buffer *ab, char *key)
{ }
static inline void audit_log_path_denied(int type, const char *operation)
{ }
-static inline int audit_log_task_context(struct audit_buffer *ab)
+static inline void audit_log_lsm(struct lsmblob *blob, bool exiting)
+{ }
+static inline int audit_log_task_context(struct audit_buffer *ab,
+ struct lsmblob *blob);
{
return 0;
}
@@ -287,6 +292,7 @@ extern void audit_seccomp(unsigned long syscall, long signr, int code);
extern void audit_seccomp_actions_logged(const char *names,
const char *old_names, int res);
extern void __audit_ptrace(struct task_struct *t);
+extern void audit_stamp_context(struct audit_context *ctx);
static inline void audit_set_context(struct task_struct *task, struct audit_context *ctx)
{
@@ -665,6 +671,9 @@ static inline void audit_ntp_log(const struct audit_ntp_data *ad)
static inline void audit_ptrace(struct task_struct *t)
{ }
+static inline void audit_stamp_context(struct audit_context *ctx)
+{ }
+
static inline void audit_log_nfcfg(const char *name, u8 af,
unsigned int nentries,
enum audit_nfcfgop op)
diff --git a/include/linux/security.h b/include/linux/security.h
index 02dc3b5ef57b..ab62f9040e19 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -179,6 +179,8 @@ struct lsmblob {
#define LSMBLOB_INVALID -1 /* Not a valid LSM slot number */
#define LSMBLOB_NEEDED -2 /* Slot requested on initialization */
#define LSMBLOB_NOT_NEEDED -3 /* Slot not requested */
+#define LSMBLOB_DISPLAY -4 /* Use the "display" slot */
+#define LSMBLOB_FIRST -5 /* Use the default "display" slot */
/**
* lsmblob_init - initialize an lsmblob structure.
@@ -241,6 +243,8 @@ static inline u32 lsmblob_value(const struct lsmblob *blob)
return 0;
}
+const char *security_lsm_slot_name(int slot);
+
/* These functions are in security/commoncap.c */
extern int cap_capable(const struct cred *cred, struct user_namespace *ns,
int cap, unsigned int opts);
@@ -553,7 +557,8 @@ int security_setprocattr(const char *lsm, const char *name, void *value,
size_t size);
int security_netlink_send(struct sock *sk, struct sk_buff *skb);
int security_ismaclabel(const char *name);
-int security_secid_to_secctx(struct lsmblob *blob, struct lsmcontext *cp);
+int security_secid_to_secctx(struct lsmblob *blob, struct lsmcontext *cp,
+ int display);
int security_secctx_to_secid(const char *secdata, u32 seclen,
struct lsmblob *blob);
void security_release_secctx(struct lsmcontext *cp);
@@ -1371,7 +1376,7 @@ static inline int security_ismaclabel(const char *name)
}
static inline int security_secid_to_secctx(struct lsmblob *blob,
- struct lsmcontext *cp)
+ struct lsmcontext *cp, int display)
{
return -EOPNOTSUPP;
}
diff --git a/include/net/netlabel.h b/include/net/netlabel.h
index 73fc25b4042b..216cb1ffc8f0 100644
--- a/include/net/netlabel.h
+++ b/include/net/netlabel.h
@@ -97,7 +97,7 @@ struct calipso_doi;
/* NetLabel audit information */
struct netlbl_audit {
- u32 secid;
+ struct lsmblob lsmdata;
kuid_t loginuid;
unsigned int sessionid;
};
diff --git a/include/net/scm.h b/include/net/scm.h
index 4a6ad8caf423..8b5a4737e1b8 100644
--- a/include/net/scm.h
+++ b/include/net/scm.h
@@ -96,7 +96,8 @@ static inline void scm_passec(struct socket *sock, struct msghdr *msg, struct sc
int err;
if (test_bit(SOCK_PASSSEC, &sock->flags)) {
- err = security_secid_to_secctx(&scm->lsmblob, &context);
+ err = security_secid_to_secctx(&scm->lsmblob, &context,
+ LSMBLOB_DISPLAY);
if (!err) {
put_cmsg(msg, SOL_SOCKET, SCM_SECURITY,
diff --git a/include/net/xfrm.h b/include/net/xfrm.h
index c7d213c9f9d8..930432c3912e 100644
--- a/include/net/xfrm.h
+++ b/include/net/xfrm.h
@@ -672,11 +672,13 @@ static inline struct audit_buffer *xfrm_audit_start(const char *op)
if (audit_enabled == AUDIT_OFF)
return NULL;
+ audit_stamp_context(audit_context());
audit_buf = audit_log_start(audit_context(), GFP_ATOMIC,
AUDIT_MAC_IPSEC_EVENT);
if (audit_buf == NULL)
return NULL;
audit_log_format(audit_buf, "op=%s", op);
+ audit_log_lsm(NULL, false);
return audit_buf;
}
@@ -690,7 +692,7 @@ static inline void xfrm_audit_helper_usrinfo(bool task_valid,
AUDIT_SID_UNSET;
audit_log_format(audit_buf, " auid=%u ses=%u", auid, ses);
- audit_log_task_context(audit_buf);
+ audit_log_task_context(audit_buf, NULL);
}
void xfrm_audit_policy_add(struct xfrm_policy *xp, int result, bool task_valid);
diff --git a/include/uapi/linux/audit.h b/include/uapi/linux/audit.h
index 9b6a973f4cc3..a5403efc2fdf 100644
--- a/include/uapi/linux/audit.h
+++ b/include/uapi/linux/audit.h
@@ -139,6 +139,7 @@
#define AUDIT_MAC_UNLBL_STCDEL 1417 /* NetLabel: del a static label */
#define AUDIT_MAC_CALIPSO_ADD 1418 /* NetLabel: add CALIPSO DOI entry */
#define AUDIT_MAC_CALIPSO_DEL 1419 /* NetLabel: del CALIPSO DOI entry */
+#define AUDIT_MAC_TASK_CONTEXTS 1420 /* Multiple LSM contexts */
#define AUDIT_FIRST_KERN_ANOM_MSG 1700
#define AUDIT_LAST_KERN_ANOM_MSG 1799
diff --git a/kernel/audit.c b/kernel/audit.c
index d300e41ca443..9ca8445ca539 100644
--- a/kernel/audit.c
+++ b/kernel/audit.c
@@ -389,10 +389,11 @@ static int audit_log_config_change(char *function_name, u32 new, u32 old,
return rc;
audit_log_format(ab, "op=set %s=%u old=%u ", function_name, new, old);
audit_log_session_info(ab);
- rc = audit_log_task_context(ab);
+ rc = audit_log_task_context(ab, NULL);
if (rc)
allow_changes = 0; /* Something weird, deny request */
audit_log_format(ab, " res=%d", allow_changes);
+ audit_log_lsm(NULL, false);
audit_log_end(ab);
return rc;
}
@@ -1065,13 +1066,31 @@ static void audit_log_common_recv_msg(struct audit_context *context,
return;
audit_log_format(*ab, "pid=%d uid=%u ", pid, uid);
audit_log_session_info(*ab);
- audit_log_task_context(*ab);
+ audit_log_task_context(*ab, NULL);
}
static inline void audit_log_user_recv_msg(struct audit_buffer **ab,
u16 msg_type)
{
- audit_log_common_recv_msg(NULL, ab, msg_type);
+ struct audit_context *context;
+
+ if (!audit_lsm_multiple_contexts()) {
+ audit_log_common_recv_msg(NULL, ab, msg_type);
+ return;
+ }
+
+ context = audit_context();
+ if (context) {
+ if (!context->in_syscall)
+ audit_stamp_context(context);
+ audit_log_common_recv_msg(context, ab, msg_type);
+ return;
+ }
+
+ audit_alloc(current);
+ context = audit_context();
+
+ audit_log_common_recv_msg(context, ab, msg_type);
}
int is_audit_feature_set(int i)
@@ -1361,6 +1380,7 @@ static int audit_receive_msg(struct sk_buff *skb, struct nlmsghdr *nlh)
audit_log_n_untrustedstring(ab, str, data_len);
}
audit_log_end(ab);
+ audit_log_lsm(NULL, false);
}
break;
case AUDIT_ADD_RULE:
@@ -1430,7 +1450,7 @@ static int audit_receive_msg(struct sk_buff *skb, struct nlmsghdr *nlh)
case AUDIT_SIGNAL_INFO:
if (lsmblob_is_set(&audit_sig_lsm)) {
err = security_secid_to_secctx(&audit_sig_lsm,
- &context);
+ &context, LSMBLOB_FIRST);
if (err)
return err;
}
@@ -1556,7 +1576,7 @@ static void audit_log_multicast(int group, const char *op, int err)
tty ? tty_name(tty) : "(none)",
audit_get_sessionid(current));
audit_put_tty(tty);
- audit_log_task_context(ab); /* subj= */
+ audit_log_task_context(ab, NULL); /* subj= */
audit_log_format(ab, " comm=");
audit_log_untrustedstring(ab, get_task_comm(comm, current));
audit_log_d_path_exe(ab, current->mm); /* exe= */
@@ -1851,6 +1871,11 @@ struct audit_buffer *audit_log_start(struct audit_context *ctx, gfp_t gfp_mask,
audit_get_stamp(ab->ctx, &t, &serial);
audit_clear_dummy(ab->ctx);
+
+ if (type == AUDIT_MAC_TASK_CONTEXTS && ab->ctx->serial == 0) {
+ audit_stamp_context(ab->ctx);
+ audit_get_stamp(ab->ctx, &t, &serial);
+ }
audit_log_format(ab, "audit(%llu.%03lu:%u): ",
(unsigned long long)t.tv_sec, t.tv_nsec/1000000, serial);
@@ -2108,30 +2133,47 @@ void audit_log_key(struct audit_buffer *ab, char *key)
audit_log_format(ab, "(null)");
}
-int audit_log_task_context(struct audit_buffer *ab)
+int audit_log_task_context(struct audit_buffer *ab, struct lsmblob *blob)
{
+ int i;
int error;
- struct lsmblob blob;
- struct lsmcontext context;
+ struct lsmblob localblob;
+ struct lsmcontext lsmdata;
- security_task_getsecid(current, &blob);
- if (!lsmblob_is_set(&blob))
+ /*
+ * If there is more than one security module that has a
+ * subject "context" it's necessary to put the subject data
+ * into a separate record to maintain compatibility.
+ */
+ if (audit_lsm_multiple_contexts()) {
+ audit_log_format(ab, " subj=?");
return 0;
+ }
- error = security_secid_to_secctx(&blob, &context);
- if (error) {
- if (error != -EINVAL)
- goto error_path;
- return 0;
+ if (blob == NULL) {
+ security_task_getsecid(current, &localblob);
+ if (!lsmblob_is_set(&localblob)) {
+ audit_log_format(ab, " subj=?");
+ return 0;
+ }
+ blob = &localblob;
}
- audit_log_format(ab, " subj=%s", context.context);
- security_release_secctx(&context);
- return 0;
+ for (i = 0; i < LSMBLOB_ENTRIES; i++) {
+ if (blob->secid[i] == 0)
+ continue;
+ error = security_secid_to_secctx(blob, &lsmdata, i);
+ if (error && error != -EINVAL) {
+ audit_panic("error in audit_log_task_context");
+ return error;
+ }
+
+ audit_log_format(ab, " subj=%s", lsmdata.context);
+ security_release_secctx(&lsmdata);
+ break;
+ }
-error_path:
- audit_panic("error in audit_log_task_context");
- return error;
+ return 0;
}
EXPORT_SYMBOL(audit_log_task_context);
@@ -2203,7 +2245,7 @@ void audit_log_task_info(struct audit_buffer *ab)
audit_log_format(ab, " comm=");
audit_log_untrustedstring(ab, get_task_comm(comm, current));
audit_log_d_path_exe(ab, current->mm);
- audit_log_task_context(ab);
+ audit_log_task_context(ab, NULL);
}
EXPORT_SYMBOL(audit_log_task_info);
@@ -2261,6 +2303,7 @@ static void audit_log_set_loginuid(kuid_t koldloginuid, kuid_t kloginuid,
if (!audit_enabled)
return;
+ audit_stamp_context(audit_context());
ab = audit_log_start(audit_context(), GFP_KERNEL, AUDIT_LOGIN);
if (!ab)
return;
@@ -2271,11 +2314,12 @@ static void audit_log_set_loginuid(kuid_t koldloginuid, kuid_t kloginuid,
tty = audit_get_tty();
audit_log_format(ab, "pid=%d uid=%u", task_tgid_nr(current), uid);
- audit_log_task_context(ab);
+ audit_log_task_context(ab, NULL);
audit_log_format(ab, " old-auid=%u auid=%u tty=%s old-ses=%u ses=%u res=%d",
oldloginuid, loginuid, tty ? tty_name(tty) : "(none)",
oldsessionid, sessionid, !rc);
audit_put_tty(tty);
+ audit_log_lsm(NULL, true);
audit_log_end(ab);
}
diff --git a/kernel/audit.h b/kernel/audit.h
index 6ab012e5fe98..8efb68c21c2d 100644
--- a/kernel/audit.h
+++ b/kernel/audit.h
@@ -298,6 +298,11 @@ static inline void audit_clear_dummy(struct audit_context *ctx)
ctx->dummy = 0;
}
+static inline bool audit_lsm_multiple_contexts(void)
+{
+ return security_lsm_slot_name(1) != NULL;
+}
+
#else /* CONFIG_AUDITSYSCALL */
#define auditsc_get_stamp(c, t, s) 0
#define audit_put_watch(w) {}
diff --git a/kernel/auditfilter.c b/kernel/auditfilter.c
index 31732023b689..6c03e463668e 100644
--- a/kernel/auditfilter.c
+++ b/kernel/auditfilter.c
@@ -1107,7 +1107,7 @@ static void audit_log_rule_change(char *action, struct audit_krule *rule, int re
if (!ab)
return;
audit_log_session_info(ab);
- audit_log_task_context(ab);
+ audit_log_task_context(ab, NULL);
audit_log_format(ab, " op=%s", action);
audit_log_key(ab, rule->filterkey);
audit_log_format(ab, " list=%d res=%d", rule->listnr, res);
diff --git a/kernel/auditsc.c b/kernel/auditsc.c
index 1f7bd6b34ec7..54639da63dbf 100644
--- a/kernel/auditsc.c
+++ b/kernel/auditsc.c
@@ -455,7 +455,7 @@ static int audit_filter_rules(struct task_struct *tsk,
{
const struct cred *cred;
int i, need_sid = 1;
- struct lsmblob blob;
+ struct lsmblob blob = { };
unsigned int sessionid;
cred = rcu_dereference_check(tsk->cred, tsk == current || task_creation);
@@ -944,10 +944,12 @@ int audit_alloc(struct task_struct *tsk)
return 0; /* Return if not auditing. */
state = audit_filter_task(tsk, &key);
- if (state == AUDIT_DISABLED) {
+ if (!audit_lsm_multiple_contexts() && state == AUDIT_DISABLED) {
clear_tsk_thread_flag(tsk, TIF_SYSCALL_AUDIT);
return 0;
}
+ if (state == AUDIT_DISABLED)
+ clear_tsk_thread_flag(tsk, TIF_SYSCALL_AUDIT);
if (!(context = audit_alloc_context(state))) {
kfree(key);
@@ -991,7 +993,7 @@ static int audit_log_pid_context(struct audit_context *context, pid_t pid,
from_kuid(&init_user_ns, auid),
from_kuid(&init_user_ns, uid), sessionid);
if (lsmblob_is_set(blob)) {
- if (security_secid_to_secctx(blob, &lsmctx)) {
+ if (security_secid_to_secctx(blob, &lsmctx, LSMBLOB_FIRST)) {
audit_log_format(ab, " obj=(none)");
rc = 1;
} else {
@@ -1236,7 +1238,8 @@ static void show_special(struct audit_context *context, int *call_panic)
struct lsmblob blob;
lsmblob_init(&blob, osid);
- if (security_secid_to_secctx(&blob, &lsmcxt)) {
+ if (security_secid_to_secctx(&blob, &lsmcxt,
+ LSMBLOB_FIRST)) {
audit_log_format(ab, " osid=%u", osid);
*call_panic = 1;
} else {
@@ -1388,7 +1391,7 @@ static void audit_log_name(struct audit_context *context, struct audit_names *n,
struct lsmcontext lsmctx;
lsmblob_init(&blob, n->osid);
- if (security_secid_to_secctx(&blob, &lsmctx)) {
+ if (security_secid_to_secctx(&blob, &lsmctx, LSMBLOB_FIRST)) {
audit_log_format(ab, " osid=%u", n->osid);
if (call_panic)
*call_panic = 2;
@@ -1462,6 +1465,52 @@ static void audit_log_proctitle(void)
audit_log_end(ab);
}
+void audit_log_lsm(struct lsmblob *blob, bool exiting)
+{
+ struct audit_context *context = audit_context();
+ struct lsmcontext lsmdata;
+ struct audit_buffer *ab;
+ struct lsmblob localblob;
+ bool sep = false;
+ int error;
+ int i;
+
+ if (!audit_lsm_multiple_contexts())
+ return;
+
+ if (context && context->in_syscall && !exiting)
+ return;
+
+ ab = audit_log_start(context, GFP_KERNEL, AUDIT_MAC_TASK_CONTEXTS);
+ if (!ab)
+ return; /* audit_panic or being filtered */
+
+ if (blob == NULL) {
+ security_task_getsecid(current, &localblob);
+ if (!lsmblob_is_set(&localblob))
+ return;
+ blob = &localblob;
+ }
+
+ for (i = 0; i < LSMBLOB_ENTRIES; i++) {
+ if (blob->secid[i] == 0)
+ continue;
+ error = security_secid_to_secctx(blob, &lsmdata, i);
+ if (error && error != -EINVAL) {
+ audit_panic("error in audit_log_lsm");
+ return;
+ }
+
+ audit_log_format(ab, "%ssubj_%s=%s", sep ? " " : "",
+ security_lsm_slot_name(i), lsmdata.context);
+ sep = true;
+
+ security_release_secctx(&lsmdata);
+ }
+
+ audit_log_end(ab);
+}
+
static void audit_log_exit(void)
{
int i, call_panic = 0;
@@ -1585,6 +1634,7 @@ static void audit_log_exit(void)
}
audit_log_proctitle();
+ audit_log_lsm(NULL, true);
/* Send end of event record to help user space know we are finished */
ab = audit_log_start(context, GFP_KERNEL, AUDIT_EOE);
@@ -2183,6 +2233,21 @@ void __audit_inode_child(struct inode *parent,
}
EXPORT_SYMBOL_GPL(__audit_inode_child);
+/**
+ * audit_stamp_context - set the timestamp+serial in an audit context
+ * @ctx: audit_context to set
+ */
+void audit_stamp_context(struct audit_context *ctx)
+{
+ /* ctx will be NULL unless audit_lsm_multiple_contexts() is true */
+ if (!ctx)
+ return;
+
+ ktime_get_coarse_real_ts64(&ctx->ctime);
+ ctx->serial = audit_serial();
+ ctx->current_state = AUDIT_BUILD_CONTEXT;
+}
+
/**
* auditsc_get_stamp - get local copies of audit_context values
* @ctx: audit_context for the task
@@ -2194,6 +2259,12 @@ EXPORT_SYMBOL_GPL(__audit_inode_child);
int auditsc_get_stamp(struct audit_context *ctx,
struct timespec64 *t, unsigned int *serial)
{
+ if (ctx->serial && !ctx->in_syscall) {
+ t->tv_sec = ctx->ctime.tv_sec;
+ t->tv_nsec = ctx->ctime.tv_nsec;
+ *serial = ctx->serial;
+ return 1;
+ }
if (!ctx->in_syscall)
return 0;
if (!ctx->serial)
@@ -2588,7 +2659,7 @@ void __audit_log_nfcfg(const char *name, u8 af, unsigned int nentries,
name, af, nentries, audit_nfcfgs[op].s);
audit_log_format(ab, " pid=%u", task_pid_nr(current));
- audit_log_task_context(ab); /* subj= */
+ audit_log_task_context(ab, NULL); /* subj= */
audit_log_format(ab, " comm=");
audit_log_untrustedstring(ab, get_task_comm(comm, current));
audit_log_end(ab);
@@ -2611,7 +2682,7 @@ static void audit_log_task(struct audit_buffer *ab)
from_kuid(&init_user_ns, uid),
from_kgid(&init_user_ns, gid),
sessionid);
- audit_log_task_context(ab);
+ audit_log_task_context(ab, NULL);
audit_log_format(ab, " pid=%d comm=", task_tgid_nr(current));
audit_log_untrustedstring(ab, get_task_comm(comm, current));
audit_log_d_path_exe(ab, current->mm);
@@ -2634,11 +2705,13 @@ void audit_core_dumps(long signr)
if (signr == SIGQUIT) /* don't care for those */
return;
+ audit_stamp_context(audit_context());
ab = audit_log_start(audit_context(), GFP_KERNEL, AUDIT_ANOM_ABEND);
if (unlikely(!ab))
return;
audit_log_task(ab);
audit_log_format(ab, " sig=%ld res=1", signr);
+ audit_log_lsm(NULL, true);
audit_log_end(ab);
}
diff --git a/net/ipv4/ip_sockglue.c b/net/ipv4/ip_sockglue.c
index 176ac9ce6069..e5c7a1b94ca4 100644
--- a/net/ipv4/ip_sockglue.c
+++ b/net/ipv4/ip_sockglue.c
@@ -138,7 +138,7 @@ static void ip_cmsg_recv_security(struct msghdr *msg, struct sk_buff *skb)
if (err)
return;
- err = security_secid_to_secctx(&lb, &context);
+ err = security_secid_to_secctx(&lb, &context, LSMBLOB_DISPLAY);
if (err)
return;
diff --git a/net/netfilter/nf_conntrack_netlink.c b/net/netfilter/nf_conntrack_netlink.c
index e38b5182e301..3c90b9a488d5 100644
--- a/net/netfilter/nf_conntrack_netlink.c
+++ b/net/netfilter/nf_conntrack_netlink.c
@@ -339,7 +339,7 @@ static int ctnetlink_dump_secctx(struct sk_buff *skb, const struct nf_conn *ct)
* security_secid_to_secctx() will know which security module
* to use to create the secctx. */
lsmblob_init(&blob, ct->secmark);
- ret = security_secid_to_secctx(&blob, &context);
+ ret = security_secid_to_secctx(&blob, &context, LSMBLOB_DISPLAY);
if (ret)
return 0;
@@ -655,7 +655,7 @@ static inline int ctnetlink_secctx_size(const struct nf_conn *ct)
struct lsmblob blob;
struct lsmcontext context;
- ret = security_secid_to_secctx(&blob, &context);
+ ret = security_secid_to_secctx(&blob, &context, LSMBLOB_DISPLAY);
if (ret)
return 0;
diff --git a/net/netfilter/nf_conntrack_standalone.c b/net/netfilter/nf_conntrack_standalone.c
index 0ecd1040f4f1..de85317a366e 100644
--- a/net/netfilter/nf_conntrack_standalone.c
+++ b/net/netfilter/nf_conntrack_standalone.c
@@ -177,7 +177,7 @@ static void ct_show_secctx(struct seq_file *s, const struct nf_conn *ct)
struct lsmcontext context;
lsmblob_init(&blob, ct->secmark);
- ret = security_secid_to_secctx(&blob, &context);
+ ret = security_secid_to_secctx(&blob, &context, LSMBLOB_DISPLAY);
if (ret)
return;
diff --git a/net/netfilter/nfnetlink_queue.c b/net/netfilter/nfnetlink_queue.c
index a4d4602ab9b7..15cabd7be92a 100644
--- a/net/netfilter/nfnetlink_queue.c
+++ b/net/netfilter/nfnetlink_queue.c
@@ -316,7 +316,7 @@ static u32 nfqnl_get_sk_secctx(struct sk_buff *skb, struct lsmcontext *context)
* blob. security_secid_to_secctx() will know which security
* module to use to create the secctx. */
lsmblob_init(&blob, skb->secmark);
- security_secid_to_secctx(&blob, context);
+ security_secid_to_secctx(&blob, context, LSMBLOB_DISPLAY);
}
read_unlock_bh(&skb->sk->sk_callback_lock);
diff --git a/net/netlabel/netlabel_unlabeled.c b/net/netlabel/netlabel_unlabeled.c
index c14a485ff045..99579fa49293 100644
--- a/net/netlabel/netlabel_unlabeled.c
+++ b/net/netlabel/netlabel_unlabeled.c
@@ -437,7 +437,8 @@ int netlbl_unlhsh_add(struct net *net,
unlhsh_add_return:
rcu_read_unlock();
if (audit_buf != NULL) {
- if (security_secid_to_secctx(lsmblob, &context) == 0) {
+ if (security_secid_to_secctx(lsmblob, &context,
+ LSMBLOB_FIRST) == 0) {
audit_log_format(audit_buf, " sec_obj=%s",
context.context);
security_release_secctx(&context);
@@ -492,7 +493,8 @@ static int netlbl_unlhsh_remove_addr4(struct net *net,
if (dev != NULL)
dev_put(dev);
if (entry != NULL &&
- security_secid_to_secctx(&entry->lsmblob, &context) == 0) {
+ security_secid_to_secctx(&entry->lsmblob, &context,
+ LSMBLOB_FIRST) == 0) {
audit_log_format(audit_buf, " sec_obj=%s",
context.context);
security_release_secctx(&context);
@@ -552,7 +554,8 @@ static int netlbl_unlhsh_remove_addr6(struct net *net,
if (dev != NULL)
dev_put(dev);
if (entry != NULL &&
- security_secid_to_secctx(&entry->lsmblob, &context) == 0) {
+ security_secid_to_secctx(&entry->lsmblob, &context,
+ LSMBLOB_FIRST) == 0) {
audit_log_format(audit_buf, " sec_obj=%s",
context.context);
security_release_secctx(&context);
@@ -1122,7 +1125,7 @@ static int netlbl_unlabel_staticlist_gen(u32 cmd,
lsmb = (struct lsmblob *)&addr6->lsmblob;
}
- ret_val = security_secid_to_secctx(lsmb, &context);
+ ret_val = security_secid_to_secctx(lsmb, &context, LSMBLOB_FIRST);
if (ret_val != 0)
goto list_cb_failure;
ret_val = nla_put(cb_arg->skb,
@@ -1521,14 +1524,11 @@ int __init netlbl_unlabel_defconf(void)
int ret_val;
struct netlbl_dom_map *entry;
struct netlbl_audit audit_info;
- struct lsmblob blob;
/* Only the kernel is allowed to call this function and the only time
* it is called is at bootup before the audit subsystem is reporting
* messages so don't worry to much about these values. */
- security_task_getsecid(current, &blob);
- /* scaffolding until audit_info.secid is converted */
- audit_info.secid = blob.secid[0];
+ security_task_getsecid(current, &audit_info.lsmdata);
audit_info.loginuid = GLOBAL_ROOT_UID;
audit_info.sessionid = 0;
diff --git a/net/netlabel/netlabel_user.c b/net/netlabel/netlabel_user.c
index 951ba0639d20..4e9064754b5f 100644
--- a/net/netlabel/netlabel_user.c
+++ b/net/netlabel/netlabel_user.c
@@ -84,12 +84,12 @@ struct audit_buffer *netlbl_audit_start_common(int type,
struct netlbl_audit *audit_info)
{
struct audit_buffer *audit_buf;
- struct lsmcontext context;
- struct lsmblob blob;
if (audit_enabled == AUDIT_OFF)
return NULL;
+ audit_stamp_context(audit_context());
+
audit_buf = audit_log_start(audit_context(), GFP_ATOMIC, type);
if (audit_buf == NULL)
return NULL;
@@ -98,12 +98,8 @@ struct audit_buffer *netlbl_audit_start_common(int type,
from_kuid(&init_user_ns, audit_info->loginuid),
audit_info->sessionid);
- lsmblob_init(&blob, audit_info->secid);
- if (audit_info->secid != 0 &&
- security_secid_to_secctx(&blob, &context) == 0) {
- audit_log_format(audit_buf, " subj=%s", context.context);
- security_release_secctx(&context);
- }
+ audit_log_task_context(audit_buf, &audit_info->lsmdata);
+ audit_log_lsm(&audit_info->lsmdata, false);
return audit_buf;
}
diff --git a/net/netlabel/netlabel_user.h b/net/netlabel/netlabel_user.h
index 438b5db6c714..bd4335443b87 100644
--- a/net/netlabel/netlabel_user.h
+++ b/net/netlabel/netlabel_user.h
@@ -34,11 +34,7 @@
static inline void netlbl_netlink_auditinfo(struct sk_buff *skb,
struct netlbl_audit *audit_info)
{
- struct lsmblob blob;
-
- security_task_getsecid(current, &blob);
- /* scaffolding until secid is converted */
- audit_info->secid = blob.secid[0];
+ security_task_getsecid(current, &audit_info->lsmdata);
audit_info->loginuid = audit_get_loginuid(current);
audit_info->sessionid = audit_get_sessionid(current);
}
diff --git a/security/integrity/integrity_audit.c b/security/integrity/integrity_audit.c
index 5109173839cc..00d8aa358651 100644
--- a/security/integrity/integrity_audit.c
+++ b/security/integrity/integrity_audit.c
@@ -41,7 +41,7 @@ void integrity_audit_msg(int audit_msgno, struct inode *inode,
from_kuid(&init_user_ns, current_cred()->uid),
from_kuid(&init_user_ns, audit_get_loginuid(current)),
audit_get_sessionid(current));
- audit_log_task_context(ab);
+ audit_log_task_context(ab, NULL);
audit_log_format(ab, " op=%s cause=%s comm=", op, cause);
audit_log_untrustedstring(ab, get_task_comm(name, current));
if (fname) {
diff --git a/security/security.c b/security/security.c
index c3fa4b7e9338..2b729d8c94b4 100644
--- a/security/security.c
+++ b/security/security.c
@@ -480,7 +480,31 @@ static int lsm_append(const char *new, char **result)
* Pointers to the LSM id structures for local use.
*/
static int lsm_slot __lsm_ro_after_init;
-static struct lsm_id *lsm_slotlist[LSMBLOB_ENTRIES];
+static struct lsm_id *lsm_slotlist[LSMBLOB_ENTRIES] __lsm_ro_after_init;
+
+/**
+ * security_lsm_slot_name - Get the name of the security module in a slot
+ * @slot: index into the "display" slot list.
+ *
+ * Provide the name of the security module associated with
+ * a display slot.
+ *
+ * If @slot is LSMBLOB_INVALID return the value
+ * for slot 0 if it has been set, otherwise NULL.
+ *
+ * Returns a pointer to the name string or NULL.
+ */
+const char *security_lsm_slot_name(int slot)
+{
+ if (slot == LSMBLOB_INVALID)
+ slot = 0;
+ else if (slot >= LSMBLOB_ENTRIES || slot < 0)
+ return NULL;
+
+ if (lsm_slotlist[slot] == NULL)
+ return NULL;
+ return lsm_slotlist[slot]->lsm;
+}
/**
* security_add_hooks - Add a modules hooks to the hook lists.
@@ -2205,13 +2229,32 @@ int security_ismaclabel(const char *name)
}
EXPORT_SYMBOL(security_ismaclabel);
-int security_secid_to_secctx(struct lsmblob *blob, struct lsmcontext *cp)
+int security_secid_to_secctx(struct lsmblob *blob, struct lsmcontext *cp,
+ int display)
{
struct security_hook_list *hp;
- int display = lsm_task_display(current);
memset(cp, 0, sizeof(*cp));
+ /*
+ * display either is the slot number use for formatting
+ * or an instruction on which relative slot to use.
+ */
+ if (display == LSMBLOB_DISPLAY)
+ display = lsm_task_display(current);
+ else if (display == LSMBLOB_FIRST)
+ display = LSMBLOB_INVALID;
+ else if (display < 0) {
+ WARN_ONCE(true,
+ "LSM: %s unknown display\n", __func__);
+ display = LSMBLOB_INVALID;
+ } else if (display >= lsm_slot) {
+ WARN_ONCE(true,
+ "LSM: %s invalid display\n", __func__);
+ display = LSMBLOB_INVALID;
+ }
+
+
hlist_for_each_entry(hp, &security_hook_heads.secid_to_secctx, list) {
if (WARN_ON(hp->lsmid->slot < 0 || hp->lsmid->slot >= lsm_slot))
continue;
@@ -2241,7 +2284,7 @@ int security_secctx_to_secid(const char *secdata, u32 seclen,
return hp->hook.secctx_to_secid(secdata, seclen,
&blob->secid[hp->lsmid->slot]);
}
- return 0;
+ return -EOPNOTSUPP;
}
EXPORT_SYMBOL(security_secctx_to_secid);
@@ -2742,23 +2785,17 @@ int security_key_getsecurity(struct key *key, char **_buffer)
int security_audit_rule_init(u32 field, u32 op, char *rulestr, void **lsmrule)
{
struct security_hook_list *hp;
- bool one_is_good = false;
- int rc = 0;
- int trc;
+ int display = lsm_task_display(current);
hlist_for_each_entry(hp, &security_hook_heads.audit_rule_init, list) {
if (WARN_ON(hp->lsmid->slot < 0 || hp->lsmid->slot >= lsm_slot))
continue;
- trc = hp->hook.audit_rule_init(field, op, rulestr,
- &lsmrule[hp->lsmid->slot]);
- if (trc == 0)
- one_is_good = true;
- else
- rc = trc;
+ if (display != LSMBLOB_INVALID && display != hp->lsmid->slot)
+ continue;
+ return hp->hook.audit_rule_init(field, op, rulestr,
+ &lsmrule[hp->lsmid->slot]);
}
- if (one_is_good)
- return 0;
- return rc;
+ return 0;
}
int security_audit_rule_known(struct audit_krule *krule)
@@ -2790,6 +2827,8 @@ int security_audit_rule_match(struct lsmblob *blob, u32 field, u32 op,
continue;
if (lsmrule[hp->lsmid->slot] == NULL)
continue;
+ if (lsmrule[hp->lsmid->slot] == NULL)
+ continue;
rc = hp->hook.audit_rule_match(blob->secid[hp->lsmid->slot],
field, op,
&lsmrule[hp->lsmid->slot]);
diff --git a/security/smack/smackfs.c b/security/smack/smackfs.c
index 177e69b43a52..2abdb45b3a61 100644
--- a/security/smack/smackfs.c
+++ b/security/smack/smackfs.c
@@ -185,7 +185,8 @@ static void smk_netlabel_audit_set(struct netlbl_audit *nap)
nap->loginuid = audit_get_loginuid(current);
nap->sessionid = audit_get_sessionid(current);
- nap->secid = skp->smk_secid;
+ lsmblob_init(&nap->lsmdata, 0);
+ nap->lsmdata.secid[smack_lsmid.slot] = skp->smk_secid;
}
/*
--
2.24.1
^ permalink raw reply related
* [PATCH v18 21/23] Audit: Add a new record for multiple object LSM attributes
From: Casey Schaufler @ 2020-07-09 0:12 UTC (permalink / raw)
To: casey.schaufler, jmorris, linux-security-module, selinux
Cc: casey, keescook, john.johansen, penguin-kernel, paul, sds,
linux-audit
In-Reply-To: <20200709001234.9719-1-casey@schaufler-ca.com>
Create a new audit record type to contain the object information
when there are multiple security modules that require such data.
This record is emitted before the other records for the event, but
is linked with the same timestamp and serial number.
Reviewed-by: Kees Cook <keescook@chromium.org>
Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
Cc: linux-audit@redhat.com
---
include/linux/audit.h | 2 ++
include/uapi/linux/audit.h | 1 +
kernel/audit.c | 53 +++++++++++++++++++++++++++++
kernel/audit.h | 4 +--
kernel/auditsc.c | 70 +++++---------------------------------
5 files changed, 67 insertions(+), 63 deletions(-)
diff --git a/include/linux/audit.h b/include/linux/audit.h
index 4693febcf41f..3cc02ddd7779 100644
--- a/include/linux/audit.h
+++ b/include/linux/audit.h
@@ -171,6 +171,8 @@ extern void audit_log_lost(const char *message);
extern void audit_log_lsm(struct lsmblob *blob, bool exiting);
extern int audit_log_task_context(struct audit_buffer *ab,
struct lsmblob *blob);
+extern int audit_log_object_context(struct audit_buffer *ab,
+ struct lsmblob *blob);
extern void audit_log_task_info(struct audit_buffer *ab);
extern int audit_update_lsm_rules(void);
diff --git a/include/uapi/linux/audit.h b/include/uapi/linux/audit.h
index a5403efc2fdf..753a69c6bbbe 100644
--- a/include/uapi/linux/audit.h
+++ b/include/uapi/linux/audit.h
@@ -140,6 +140,7 @@
#define AUDIT_MAC_CALIPSO_ADD 1418 /* NetLabel: add CALIPSO DOI entry */
#define AUDIT_MAC_CALIPSO_DEL 1419 /* NetLabel: del CALIPSO DOI entry */
#define AUDIT_MAC_TASK_CONTEXTS 1420 /* Multiple LSM contexts */
+#define AUDIT_MAC_OBJ_CONTEXTS 1421 /* Multiple LSM object contexts */
#define AUDIT_FIRST_KERN_ANOM_MSG 1700
#define AUDIT_LAST_KERN_ANOM_MSG 1799
diff --git a/kernel/audit.c b/kernel/audit.c
index 9ca8445ca539..e05f9e1f1254 100644
--- a/kernel/audit.c
+++ b/kernel/audit.c
@@ -2177,6 +2177,59 @@ int audit_log_task_context(struct audit_buffer *ab, struct lsmblob *blob)
}
EXPORT_SYMBOL(audit_log_task_context);
+int audit_log_object_context(struct audit_buffer *ab,
+ struct lsmblob *blob)
+{
+ int i;
+ int error;
+ bool sep = false;
+ struct lsmcontext lsmdata;
+ struct audit_buffer *lsmab = NULL;
+ struct audit_context *context = NULL;
+
+ /*
+ * If there is more than one security module that has a
+ * object "context" it's necessary to put the object data
+ * into a separate record to maintain compatibility.
+ */
+ if (security_lsm_slot_name(1) != NULL) {
+ audit_log_format(ab, " obj=?");
+ context = ab->ctx;
+ if (context)
+ lsmab = audit_log_start(context, GFP_KERNEL,
+ AUDIT_MAC_OBJ_CONTEXTS);
+ }
+
+ for (i = 0; i < LSMBLOB_ENTRIES; i++) {
+ if (blob->secid[i] == 0)
+ continue;
+ error = security_secid_to_secctx(blob, &lsmdata, i);
+ if (error && error != -EINVAL) {
+ audit_panic("error in audit_log_object_context");
+ return error;
+ }
+
+ if (context) {
+ audit_log_format(lsmab, "%sobj_%s=%s",
+ sep ? " " : "",
+ security_lsm_slot_name(i),
+ lsmdata.context);
+ sep = true;
+ } else
+ audit_log_format(ab, " obj=%s", lsmdata.context);
+
+ security_release_secctx(&lsmdata);
+ if (!context)
+ break;
+ }
+
+ if (context)
+ audit_log_end(lsmab);
+
+ return 0;
+}
+EXPORT_SYMBOL(audit_log_object_context);
+
void audit_log_d_path_exe(struct audit_buffer *ab,
struct mm_struct *mm)
{
diff --git a/kernel/audit.h b/kernel/audit.h
index 8efb68c21c2d..830e4dfeb83b 100644
--- a/kernel/audit.h
+++ b/kernel/audit.h
@@ -78,7 +78,7 @@ struct audit_names {
kuid_t uid;
kgid_t gid;
dev_t rdev;
- u32 osid;
+ struct lsmblob oblob;
struct audit_cap_data fcap;
unsigned int fcap_ver;
unsigned char type; /* record type */
@@ -152,7 +152,7 @@ struct audit_context {
kuid_t uid;
kgid_t gid;
umode_t mode;
- u32 osid;
+ struct lsmblob oblob;
int has_perm;
uid_t perm_uid;
gid_t perm_gid;
diff --git a/kernel/auditsc.c b/kernel/auditsc.c
index 54639da63dbf..db8628709971 100644
--- a/kernel/auditsc.c
+++ b/kernel/auditsc.c
@@ -670,14 +670,6 @@ static int audit_filter_rules(struct task_struct *tsk,
if (f->lsm_isset) {
/* Find files that match */
if (name) {
- /*
- * lsmblob_init sets all values in the
- * lsmblob to sid. This is temporary
- * until name->osid is converted to a
- * lsmblob, which happens later in
- * this patch set.
- */
- lsmblob_init(&blob, name->osid);
result = security_audit_rule_match(
&blob,
f->type,
@@ -685,7 +677,6 @@ static int audit_filter_rules(struct task_struct *tsk,
f->lsm_rules);
} else if (ctx) {
list_for_each_entry(n, &ctx->names_list, list) {
- lsmblob_init(&blob, name->osid);
if (security_audit_rule_match(
&blob,
f->type,
@@ -699,8 +690,7 @@ static int audit_filter_rules(struct task_struct *tsk,
/* Find ipc objects that match */
if (!ctx || ctx->type != AUDIT_IPC)
break;
- lsmblob_init(&blob, ctx->ipc.osid);
- if (security_audit_rule_match(&blob,
+ if (security_audit_rule_match(&ctx->ipc.oblob,
f->type, f->op,
f->lsm_rules))
++result;
@@ -982,7 +972,6 @@ static int audit_log_pid_context(struct audit_context *context, pid_t pid,
struct lsmblob *blob, char *comm)
{
struct audit_buffer *ab;
- struct lsmcontext lsmctx;
int rc = 0;
ab = audit_log_start(context, GFP_KERNEL, AUDIT_OBJ_PID);
@@ -992,15 +981,7 @@ static int audit_log_pid_context(struct audit_context *context, pid_t pid,
audit_log_format(ab, "opid=%d oauid=%d ouid=%d oses=%d", pid,
from_kuid(&init_user_ns, auid),
from_kuid(&init_user_ns, uid), sessionid);
- if (lsmblob_is_set(blob)) {
- if (security_secid_to_secctx(blob, &lsmctx, LSMBLOB_FIRST)) {
- audit_log_format(ab, " obj=(none)");
- rc = 1;
- } else {
- audit_log_format(ab, " obj=%s", lsmctx.context);
- security_release_secctx(&lsmctx);
- }
- }
+ rc = audit_log_object_context(ab, blob);
audit_log_format(ab, " ocomm=");
audit_log_untrustedstring(ab, comm);
audit_log_end(ab);
@@ -1227,26 +1208,14 @@ static void show_special(struct audit_context *context, int *call_panic)
context->socketcall.args[i]);
break; }
case AUDIT_IPC: {
- u32 osid = context->ipc.osid;
+ struct lsmblob *oblob = &context->ipc.oblob;
audit_log_format(ab, "ouid=%u ogid=%u mode=%#ho",
from_kuid(&init_user_ns, context->ipc.uid),
from_kgid(&init_user_ns, context->ipc.gid),
context->ipc.mode);
- if (osid) {
- struct lsmcontext lsmcxt;
- struct lsmblob blob;
-
- lsmblob_init(&blob, osid);
- if (security_secid_to_secctx(&blob, &lsmcxt,
- LSMBLOB_FIRST)) {
- audit_log_format(ab, " osid=%u", osid);
- *call_panic = 1;
- } else {
- audit_log_format(ab, " obj=%s", lsmcxt.context);
- security_release_secctx(&lsmcxt);
- }
- }
+ if (audit_log_object_context(ab, oblob))
+ *call_panic = 1;
if (context->ipc.has_perm) {
audit_log_end(ab);
ab = audit_log_start(context, GFP_KERNEL,
@@ -1386,20 +1355,8 @@ static void audit_log_name(struct audit_context *context, struct audit_names *n,
from_kgid(&init_user_ns, n->gid),
MAJOR(n->rdev),
MINOR(n->rdev));
- if (n->osid != 0) {
- struct lsmblob blob;
- struct lsmcontext lsmctx;
-
- lsmblob_init(&blob, n->osid);
- if (security_secid_to_secctx(&blob, &lsmctx, LSMBLOB_FIRST)) {
- audit_log_format(ab, " osid=%u", n->osid);
- if (call_panic)
- *call_panic = 2;
- } else {
- audit_log_format(ab, " obj=%s", lsmctx.context);
- security_release_secctx(&lsmctx);
- }
- }
+ if (audit_log_object_context(ab, &n->oblob) && call_panic)
+ *call_panic = 2;
/* log the audit_names record type */
switch (n->type) {
@@ -1992,17 +1949,13 @@ static void audit_copy_inode(struct audit_names *name,
const struct dentry *dentry,
struct inode *inode, unsigned int flags)
{
- struct lsmblob blob;
-
name->ino = inode->i_ino;
name->dev = inode->i_sb->s_dev;
name->mode = inode->i_mode;
name->uid = inode->i_uid;
name->gid = inode->i_gid;
name->rdev = inode->i_rdev;
- security_inode_getsecid(inode, &blob);
- /* scaffolding until osid is updated */
- name->osid = blob.secid[0];
+ security_inode_getsecid(inode, &name->oblob);
if (flags & AUDIT_INODE_NOEVAL) {
name->fcap_ver = -1;
return;
@@ -2369,16 +2322,11 @@ void __audit_mq_getsetattr(mqd_t mqdes, struct mq_attr *mqstat)
void __audit_ipc_obj(struct kern_ipc_perm *ipcp)
{
struct audit_context *context = audit_context();
- struct lsmblob blob;
context->ipc.uid = ipcp->uid;
context->ipc.gid = ipcp->gid;
context->ipc.mode = ipcp->mode;
context->ipc.has_perm = 0;
- security_ipc_getsecid(ipcp, &blob);
- /* context->ipc.osid will be changed to a lsmblob later in
- * the patch series. This will allow auditing of all the object
- * labels associated with the ipc object. */
- context->ipc.osid = lsmblob_value(&blob);
+ security_ipc_getsecid(ipcp, &context->ipc.oblob);
context->type = AUDIT_IPC;
}
--
2.24.1
^ permalink raw reply related
* [PATCH v18 22/23] LSM: Add /proc attr entry for full LSM context
From: Casey Schaufler @ 2020-07-09 0:12 UTC (permalink / raw)
To: casey.schaufler, jmorris, linux-security-module, selinux
Cc: casey, keescook, john.johansen, penguin-kernel, paul, sds,
linux-api
In-Reply-To: <20200709001234.9719-1-casey@schaufler-ca.com>
Add an entry /proc/.../attr/context which displays the full
process security "context" in compound format:
lsm1\0value\0lsm2\0value\0...
This entry is not writable.
Reviewed-by: Kees Cook <keescook@chromium.org>
Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
Cc: linux-api@vger.kernel.org
---
Documentation/security/lsm.rst | 28 +++++++++++++
fs/proc/base.c | 1 +
include/linux/lsm_hooks.h | 6 +++
security/apparmor/include/procattr.h | 2 +-
security/apparmor/lsm.c | 8 +++-
security/apparmor/procattr.c | 22 +++++-----
security/security.c | 63 ++++++++++++++++++++++++++++
security/selinux/hooks.c | 2 +-
security/smack/smack_lsm.c | 2 +-
9 files changed, 119 insertions(+), 15 deletions(-)
diff --git a/Documentation/security/lsm.rst b/Documentation/security/lsm.rst
index 6a2a2e973080..fd4c87358d54 100644
--- a/Documentation/security/lsm.rst
+++ b/Documentation/security/lsm.rst
@@ -129,3 +129,31 @@ to identify it as the first security module to be registered.
The capabilities security module does not use the general security
blobs, unlike other modules. The reasons are historical and are
based on overhead, complexity and performance concerns.
+
+LSM External Interfaces
+=======================
+
+The LSM infrastructure does not generally provide external interfaces.
+The individual security modules provide what external interfaces they
+require.
+
+The file ``/sys/kernel/security/lsm`` provides a comma
+separated list of the active security modules.
+
+The file ``/proc/pid/attr/display`` contains the name of the security
+module for which the ``/proc/pid/attr/current`` interface will
+apply. This interface can be written to.
+
+The infrastructure does provide an interface for the special
+case where multiple security modules provide a process context.
+This is provided in compound context format.
+
+- `lsm\0value\0lsm\0value\0`
+
+The `lsm` and `value` fields are nul terminated bytestrings.
+Each field may contain whitespace or non-printable characters.
+The nul bytes are included in the size of a compound context.
+The context ``Bell\0Secret\0Biba\0Loose\0`` has a size of 23.
+
+The file ``/proc/pid/attr/context`` provides the security
+context of the identified process.
diff --git a/fs/proc/base.c b/fs/proc/base.c
index 40471a12ced2..ba8b0316e999 100644
--- a/fs/proc/base.c
+++ b/fs/proc/base.c
@@ -2795,6 +2795,7 @@ static const struct pid_entry attr_dir_stuff[] = {
ATTR(NULL, "keycreate", 0666),
ATTR(NULL, "sockcreate", 0666),
ATTR(NULL, "display", 0666),
+ ATTR(NULL, "context", 0444),
#ifdef CONFIG_SECURITY_SMACK
DIR("smack", 0555,
proc_smack_attr_dir_inode_ops, proc_smack_attr_dir_ops),
diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
index e6d72a010606..ceaba9d4792a 100644
--- a/include/linux/lsm_hooks.h
+++ b/include/linux/lsm_hooks.h
@@ -1359,6 +1359,12 @@
* @pages contains the number of pages.
* Return 0 if permission is granted.
*
+ * @getprocattr:
+ * Provide the named process attribute for display in special files in
+ * the /proc/.../attr directory. Attribute naming and the data displayed
+ * is at the discretion of the security modules. The exception is the
+ * "context" attribute, which will contain the security context of the
+ * task as a nul terminated text string without trailing whitespace.
* @ismaclabel:
* Check if the extended attribute specified by @name
* represents a MAC label. Returns 1 if name is a MAC
diff --git a/security/apparmor/include/procattr.h b/security/apparmor/include/procattr.h
index 31689437e0e1..03dbfdb2f2c0 100644
--- a/security/apparmor/include/procattr.h
+++ b/security/apparmor/include/procattr.h
@@ -11,7 +11,7 @@
#ifndef __AA_PROCATTR_H
#define __AA_PROCATTR_H
-int aa_getprocattr(struct aa_label *label, char **string);
+int aa_getprocattr(struct aa_label *label, char **string, bool newline);
int aa_setprocattr_changehat(char *args, size_t size, int flags);
#endif /* __AA_PROCATTR_H */
diff --git a/security/apparmor/lsm.c b/security/apparmor/lsm.c
index 31a6f11890f1..7ce570b0f491 100644
--- a/security/apparmor/lsm.c
+++ b/security/apparmor/lsm.c
@@ -593,6 +593,7 @@ static int apparmor_getprocattr(struct task_struct *task, char *name,
const struct cred *cred = get_task_cred(task);
struct aa_task_ctx *ctx = task_ctx(current);
struct aa_label *label = NULL;
+ bool newline = true;
if (strcmp(name, "current") == 0)
label = aa_get_newest_label(cred_label(cred));
@@ -600,11 +601,14 @@ static int apparmor_getprocattr(struct task_struct *task, char *name,
label = aa_get_newest_label(ctx->previous);
else if (strcmp(name, "exec") == 0 && ctx->onexec)
label = aa_get_newest_label(ctx->onexec);
- else
+ else if (strcmp(name, "context") == 0) {
+ label = aa_get_newest_label(cred_label(cred));
+ newline = false;
+ } else
error = -EINVAL;
if (label)
- error = aa_getprocattr(label, value);
+ error = aa_getprocattr(label, value, newline);
aa_put_label(label);
put_cred(cred);
diff --git a/security/apparmor/procattr.c b/security/apparmor/procattr.c
index c929bf4a3df1..be3b083d9b74 100644
--- a/security/apparmor/procattr.c
+++ b/security/apparmor/procattr.c
@@ -20,6 +20,7 @@
* aa_getprocattr - Return the profile information for @profile
* @profile: the profile to print profile info about (NOT NULL)
* @string: Returns - string containing the profile info (NOT NULL)
+ * @newline: Should a newline be added to @string.
*
* Returns: length of @string on success else error on failure
*
@@ -30,20 +31,21 @@
*
* Returns: size of string placed in @string else error code on failure
*/
-int aa_getprocattr(struct aa_label *label, char **string)
+int aa_getprocattr(struct aa_label *label, char **string, bool newline)
{
struct aa_ns *ns = labels_ns(label);
struct aa_ns *current_ns = aa_get_current_ns();
+ int flags = FLAG_VIEW_SUBNS | FLAG_HIDDEN_UNCONFINED;
int len;
if (!aa_ns_visible(current_ns, ns, true)) {
aa_put_ns(current_ns);
return -EACCES;
}
+ if (newline)
+ flags |= FLAG_SHOW_MODE;
- len = aa_label_snxprint(NULL, 0, current_ns, label,
- FLAG_SHOW_MODE | FLAG_VIEW_SUBNS |
- FLAG_HIDDEN_UNCONFINED);
+ len = aa_label_snxprint(NULL, 0, current_ns, label, flags);
AA_BUG(len < 0);
*string = kmalloc(len + 2, GFP_KERNEL);
@@ -52,19 +54,19 @@ int aa_getprocattr(struct aa_label *label, char **string)
return -ENOMEM;
}
- len = aa_label_snxprint(*string, len + 2, current_ns, label,
- FLAG_SHOW_MODE | FLAG_VIEW_SUBNS |
- FLAG_HIDDEN_UNCONFINED);
+ len = aa_label_snxprint(*string, len + 2, current_ns, label, flags);
if (len < 0) {
aa_put_ns(current_ns);
return len;
}
- (*string)[len] = '\n';
- (*string)[len + 1] = 0;
+ if (newline) {
+ (*string)[len] = '\n';
+ (*string)[++len] = 0;
+ }
aa_put_ns(current_ns);
- return len + 1;
+ return len;
}
/**
diff --git a/security/security.c b/security/security.c
index 2b729d8c94b4..3469a387d6b0 100644
--- a/security/security.c
+++ b/security/security.c
@@ -754,6 +754,42 @@ static void __init lsm_early_task(struct task_struct *task)
panic("%s: Early task alloc failed.\n", __func__);
}
+/**
+ * append_ctx - append a lsm/context pair to a compound context
+ * @ctx: the existing compound context
+ * @ctxlen: size of the old context, including terminating nul byte
+ * @lsm: new lsm name, nul terminated
+ * @new: new context, possibly nul terminated
+ * @newlen: maximum size of @new
+ *
+ * replace @ctx with a new compound context, appending @newlsm and @new
+ * to @ctx. On exit the new data replaces the old, which is freed.
+ * @ctxlen is set to the new size, which includes a trailing nul byte.
+ *
+ * Returns 0 on success, -ENOMEM if no memory is available.
+ */
+static int append_ctx(char **ctx, int *ctxlen, const char *lsm, char *new,
+ int newlen)
+{
+ char *final;
+ int llen;
+
+ llen = strlen(lsm) + 1;
+ newlen = strnlen(new, newlen) + 1;
+
+ final = kzalloc(*ctxlen + llen + newlen, GFP_KERNEL);
+ if (final == NULL)
+ return -ENOMEM;
+ if (*ctxlen)
+ memcpy(final, *ctx, *ctxlen);
+ memcpy(final + *ctxlen, lsm, llen);
+ memcpy(final + *ctxlen + llen, new, newlen);
+ kfree(*ctx);
+ *ctx = final;
+ *ctxlen = *ctxlen + llen + newlen;
+ return 0;
+}
+
/*
* The default value of the LSM hook is defined in linux/lsm_hook_defs.h and
* can be accessed with:
@@ -2109,6 +2145,10 @@ int security_getprocattr(struct task_struct *p, const char *lsm, char *name,
char **value)
{
struct security_hook_list *hp;
+ char *final = NULL;
+ char *cp;
+ int rc = 0;
+ int finallen = 0;
int display = lsm_task_display(current);
int slot = 0;
@@ -2136,6 +2176,29 @@ int security_getprocattr(struct task_struct *p, const char *lsm, char *name,
return -ENOMEM;
}
+ if (!strcmp(name, "context")) {
+ hlist_for_each_entry(hp, &security_hook_heads.getprocattr,
+ list) {
+ rc = hp->hook.getprocattr(p, "context", &cp);
+ if (rc == -EINVAL)
+ continue;
+ if (rc < 0) {
+ kfree(final);
+ return rc;
+ }
+ rc = append_ctx(&final, &finallen, hp->lsmid->lsm,
+ cp, rc);
+ if (rc < 0) {
+ kfree(final);
+ return rc;
+ }
+ }
+ if (final == NULL)
+ return -EINVAL;
+ *value = final;
+ return finallen;
+ }
+
hlist_for_each_entry(hp, &security_hook_heads.getprocattr, list) {
if (lsm != NULL && strcmp(lsm, hp->lsmid->lsm))
continue;
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index c13c207c5da1..43d5c09b9a9e 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -6288,7 +6288,7 @@ static int selinux_getprocattr(struct task_struct *p,
goto bad;
}
- if (!strcmp(name, "current"))
+ if (!strcmp(name, "current") || !strcmp(name, "context"))
sid = __tsec->sid;
else if (!strcmp(name, "prev"))
sid = __tsec->osid;
diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
index 6f0cdb40addc..d7bb6442f192 100644
--- a/security/smack/smack_lsm.c
+++ b/security/smack/smack_lsm.c
@@ -3463,7 +3463,7 @@ static int smack_getprocattr(struct task_struct *p, char *name, char **value)
char *cp;
int slen;
- if (strcmp(name, "current") != 0)
+ if (strcmp(name, "current") != 0 && strcmp(name, "context") != 0)
return -EINVAL;
cp = kstrdup(skp->smk_known, GFP_KERNEL);
--
2.24.1
^ permalink raw reply related
* [PATCH v18 23/23] AppArmor: Remove the exclusive flag
From: Casey Schaufler @ 2020-07-09 0:12 UTC (permalink / raw)
To: casey.schaufler, jmorris, linux-security-module, selinux
Cc: casey, keescook, john.johansen, penguin-kernel, paul, sds
In-Reply-To: <20200709001234.9719-1-casey@schaufler-ca.com>
With the inclusion of the "display" process attribute
mechanism AppArmor no longer needs to be treated as an
"exclusive" security module. Remove the flag that indicates
it is exclusive. Remove the stub getpeersec_dgram AppArmor
hook as it has no effect in the single LSM case and
interferes in the multiple LSM case.
Acked-by: Stephen Smalley <sds@tycho.nsa.gov>
Reviewed-by: Kees Cook <keescook@chromium.org>
Reviewed-by: John Johansen <john.johansen@canonical.com>
Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
---
security/apparmor/lsm.c | 20 +-------------------
1 file changed, 1 insertion(+), 19 deletions(-)
diff --git a/security/apparmor/lsm.c b/security/apparmor/lsm.c
index 7ce570b0f491..4b7cbe9bb1be 100644
--- a/security/apparmor/lsm.c
+++ b/security/apparmor/lsm.c
@@ -1129,22 +1129,6 @@ static int apparmor_socket_getpeersec_stream(struct socket *sock,
return error;
}
-/**
- * apparmor_socket_getpeersec_dgram - get security label of packet
- * @sock: the peer socket
- * @skb: packet data
- * @secid: pointer to where to put the secid of the packet
- *
- * Sets the netlabel socket state on sk from parent
- */
-static int apparmor_socket_getpeersec_dgram(struct socket *sock,
- struct sk_buff *skb, u32 *secid)
-
-{
- /* TODO: requires secid support */
- return -ENOPROTOOPT;
-}
-
/**
* apparmor_sock_graft - Initialize newly created socket
* @sk: child sock
@@ -1248,8 +1232,6 @@ static struct security_hook_list apparmor_hooks[] __lsm_ro_after_init = {
#endif
LSM_HOOK_INIT(socket_getpeersec_stream,
apparmor_socket_getpeersec_stream),
- LSM_HOOK_INIT(socket_getpeersec_dgram,
- apparmor_socket_getpeersec_dgram),
LSM_HOOK_INIT(sock_graft, apparmor_sock_graft),
#ifdef CONFIG_NETWORK_SECMARK
LSM_HOOK_INIT(inet_conn_request, apparmor_inet_conn_request),
@@ -1918,7 +1900,7 @@ static int __init apparmor_init(void)
DEFINE_LSM(apparmor) = {
.name = "apparmor",
- .flags = LSM_FLAG_LEGACY_MAJOR | LSM_FLAG_EXCLUSIVE,
+ .flags = LSM_FLAG_LEGACY_MAJOR,
.enabled = &apparmor_enabled,
.blobs = &apparmor_blob_sizes,
.init = apparmor_init,
--
2.24.1
^ permalink raw reply related
* [PATCH bpf-next v3 0/4] Generalizing bpf_local_storage
From: KP Singh @ 2020-07-09 0:56 UTC (permalink / raw)
To: linux-kernel, bpf, linux-security-module
Cc: Alexei Starovoitov, Daniel Borkmann, Paul Turner, Jann Horn,
Florent Revest
From: KP Singh <kpsingh@google.com>
# v2 -> v3
* Restructured the code as per Martin's suggestions:
- Common functionality in bpf_local_storage.c
- bpf_sk_storage functionality remains in net/bpf_sk_storage.
- bpf_inode_storage is kept separate as it is enabled only with
CONFIG_BPF_LSM.
* A separate cache for inode and sk storage with macros to define it.
* Use the ops style approach as suggested by Martin instead of the
enum + switch style.
* Added the inode map to bpftool bash completion and docs.
* Rebase and indentation fixes.
# v1 -> v2
* Use the security blob pointer instead of dedicated member in
struct inode.
* Better code re-use as suggested by Alexei.
* Dropped the inode count arithmetic as pointed out by Alexei.
* Minor bug fixes and rebase.
bpf_sk_storage can already be used by some BPF program types to annotate
socket objects. These annotations are managed with the life-cycle of the
object (i.e. freed when the object is freed) which makes BPF programs
much simpler and less prone to errors and leaks.
This patch series:
* Generalizes the bpf_sk_storage infrastructure to allow easy
implementation of local storage for other objects
* Implements local storage for inodes
* Makes both bpf_{sk, inode}_storage available to LSM programs.
Local storage is safe to use in LSM programs as the attachment sites are
limited and the owning object won't be freed, however, this is not the
case for tracing. Usage in tracing is expected to follow a white-list
based approach similar to the d_path helper
(https://lore.kernel.org/bpf/20200506132946.2164578-1-jolsa@kernel.org).
Access to local storage would allow LSM programs to implement stateful
detections like detecting the unlink of a running executable from the
examples shared as a part of the KRSI series
https://lore.kernel.org/bpf/20200329004356.27286-1-kpsingh@chromium.org/
and
https://github.com/sinkap/linux-krsi/blob/patch/v1/examples/samples/bpf/lsm_detect_exec_unlink.c
KP Singh (4):
bpf: Generalize bpf_sk_storage
bpf: Implement bpf_local_storage for inodes
bpf: Allow local storage to be used from LSM programs
bpf: Add selftests for local_storage
include/linux/bpf.h | 14 +
include/linux/bpf_local_storage.h | 190 ++++
include/linux/bpf_lsm.h | 21 +
include/linux/bpf_types.h | 3 +
include/net/bpf_sk_storage.h | 2 +
include/net/sock.h | 4 +-
include/uapi/linux/bpf.h | 54 +-
kernel/bpf/Makefile | 2 +
kernel/bpf/bpf_inode_storage.c | 333 +++++++
kernel/bpf/bpf_local_storage.c | 517 +++++++++++
kernel/bpf/bpf_lsm.c | 21 +-
kernel/bpf/syscall.c | 3 +-
kernel/bpf/verifier.c | 10 +
net/core/bpf_sk_storage.c | 826 ++++--------------
security/bpf/hooks.c | 7 +
.../bpf/bpftool/Documentation/bpftool-map.rst | 2 +-
tools/bpf/bpftool/bash-completion/bpftool | 3 +-
tools/bpf/bpftool/map.c | 3 +-
tools/include/uapi/linux/bpf.h | 54 +-
tools/lib/bpf/libbpf_probes.c | 5 +-
.../bpf/prog_tests/test_local_storage.c | 60 ++
.../selftests/bpf/progs/local_storage.c | 136 +++
22 files changed, 1599 insertions(+), 671 deletions(-)
create mode 100644 include/linux/bpf_local_storage.h
create mode 100644 kernel/bpf/bpf_inode_storage.c
create mode 100644 kernel/bpf/bpf_local_storage.c
create mode 100644 tools/testing/selftests/bpf/prog_tests/test_local_storage.c
create mode 100644 tools/testing/selftests/bpf/progs/local_storage.c
--
2.27.0.389.gc38d7665816-goog
^ permalink raw reply
* [PATCH bpf-next v3 3/4] bpf: Allow local storage to be used from LSM programs
From: KP Singh @ 2020-07-09 0:56 UTC (permalink / raw)
To: linux-kernel, bpf, linux-security-module
Cc: Alexei Starovoitov, Daniel Borkmann, Paul Turner, Jann Horn,
Florent Revest
In-Reply-To: <20200709005654.3324272-1-kpsingh@chromium.org>
From: KP Singh <kpsingh@google.com>
Adds support for both bpf_{sk, inode}_storage_{get, delete} to be used
in LSM programs. These helpers are not used for tracing programs
(currently) as their usage is tied to the life-cycle of the object and
should only be used where the owning object won't be freed (when the
owning object is passed as an argument to the LSM hook). Thus, they
are safer to use in LSM hooks than tracing. Usage of local storage in
tracing programs will probably follow a per function based whitelist
approach.
Since the UAPI helper signature for bpf_sk_storage expect a bpf_sock,
it, leads to a compilation warning for LSM programs, it's also updated
to accept a void * pointer instead.
Signed-off-by: KP Singh <kpsingh@google.com>
---
include/net/bpf_sk_storage.h | 2 ++
include/uapi/linux/bpf.h | 4 ++--
kernel/bpf/bpf_lsm.c | 21 ++++++++++++++++++++-
net/core/bpf_sk_storage.c | 22 ++++++++++++++++++++++
tools/include/uapi/linux/bpf.h | 4 ++--
5 files changed, 48 insertions(+), 5 deletions(-)
diff --git a/include/net/bpf_sk_storage.h b/include/net/bpf_sk_storage.h
index 5036c94c0503..d231da1b57ca 100644
--- a/include/net/bpf_sk_storage.h
+++ b/include/net/bpf_sk_storage.h
@@ -9,6 +9,8 @@ void bpf_sk_storage_free(struct sock *sk);
extern const struct bpf_func_proto bpf_sk_storage_get_proto;
extern const struct bpf_func_proto bpf_sk_storage_delete_proto;
+extern const struct bpf_func_proto sk_storage_get_btf_proto;
+extern const struct bpf_func_proto sk_storage_delete_btf_proto;
struct bpf_sk_storage_diag;
struct sk_buff;
diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index 42fc442f4586..3d2859ccc7ae 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -2787,7 +2787,7 @@ union bpf_attr {
*
* **-ERANGE** if resulting value was out of range.
*
- * void *bpf_sk_storage_get(struct bpf_map *map, struct bpf_sock *sk, void *value, u64 flags)
+ * void *bpf_sk_storage_get(struct bpf_map *map, void *sk, void *value, u64 flags)
* Description
* Get a bpf-local-storage from a *sk*.
*
@@ -2815,7 +2815,7 @@ union bpf_attr {
* **NULL** if not found or there was an error in adding
* a new bpf-local-storage.
*
- * long bpf_sk_storage_delete(struct bpf_map *map, struct bpf_sock *sk)
+ * long bpf_sk_storage_delete(struct bpf_map *map, void *sk)
* Description
* Delete a bpf-local-storage from a *sk*.
* Return
diff --git a/kernel/bpf/bpf_lsm.c b/kernel/bpf/bpf_lsm.c
index fb278144e9fd..9cd1428c7199 100644
--- a/kernel/bpf/bpf_lsm.c
+++ b/kernel/bpf/bpf_lsm.c
@@ -11,6 +11,8 @@
#include <linux/bpf_lsm.h>
#include <linux/kallsyms.h>
#include <linux/bpf_verifier.h>
+#include <net/bpf_sk_storage.h>
+#include <linux/bpf_local_storage.h>
/* For every LSM hook that allows attachment of BPF programs, declare a nop
* function where a BPF program can be attached.
@@ -45,10 +47,27 @@ int bpf_lsm_verify_prog(struct bpf_verifier_log *vlog,
return 0;
}
+static const struct bpf_func_proto *
+bpf_lsm_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
+{
+ switch (func_id) {
+ case BPF_FUNC_inode_storage_get:
+ return &bpf_inode_storage_get_proto;
+ case BPF_FUNC_inode_storage_delete:
+ return &bpf_inode_storage_delete_proto;
+ case BPF_FUNC_sk_storage_get:
+ return &sk_storage_get_btf_proto;
+ case BPF_FUNC_sk_storage_delete:
+ return &sk_storage_delete_btf_proto;
+ default:
+ return tracing_prog_func_proto(func_id, prog);
+ }
+}
+
const struct bpf_prog_ops lsm_prog_ops = {
};
const struct bpf_verifier_ops lsm_verifier_ops = {
- .get_func_proto = tracing_prog_func_proto,
+ .get_func_proto = bpf_lsm_func_proto,
.is_valid_access = btf_ctx_access,
};
diff --git a/net/core/bpf_sk_storage.c b/net/core/bpf_sk_storage.c
index 62b6526acf7e..add0340e9ad3 100644
--- a/net/core/bpf_sk_storage.c
+++ b/net/core/bpf_sk_storage.c
@@ -455,6 +455,28 @@ const struct bpf_func_proto bpf_sk_storage_delete_proto = {
.arg2_type = ARG_PTR_TO_SOCKET,
};
+static int sk_storage_get_btf_ids[4];
+const struct bpf_func_proto sk_storage_get_btf_proto = {
+ .func = bpf_sk_storage_get,
+ .gpl_only = false,
+ .ret_type = RET_PTR_TO_MAP_VALUE_OR_NULL,
+ .arg1_type = ARG_CONST_MAP_PTR,
+ .arg2_type = ARG_PTR_TO_BTF_ID,
+ .arg3_type = ARG_PTR_TO_MAP_VALUE_OR_NULL,
+ .arg4_type = ARG_ANYTHING,
+ .btf_id = sk_storage_get_btf_ids,
+};
+
+static int sk_storage_delete_btf_ids[2];
+const struct bpf_func_proto sk_storage_delete_btf_proto = {
+ .func = bpf_sk_storage_delete,
+ .gpl_only = false,
+ .ret_type = RET_INTEGER,
+ .arg1_type = ARG_CONST_MAP_PTR,
+ .arg2_type = ARG_PTR_TO_BTF_ID,
+ .btf_id = sk_storage_delete_btf_ids,
+};
+
struct bpf_sk_storage_diag {
u32 nr_maps;
struct bpf_map *maps[];
diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
index 42fc442f4586..3d2859ccc7ae 100644
--- a/tools/include/uapi/linux/bpf.h
+++ b/tools/include/uapi/linux/bpf.h
@@ -2787,7 +2787,7 @@ union bpf_attr {
*
* **-ERANGE** if resulting value was out of range.
*
- * void *bpf_sk_storage_get(struct bpf_map *map, struct bpf_sock *sk, void *value, u64 flags)
+ * void *bpf_sk_storage_get(struct bpf_map *map, void *sk, void *value, u64 flags)
* Description
* Get a bpf-local-storage from a *sk*.
*
@@ -2815,7 +2815,7 @@ union bpf_attr {
* **NULL** if not found or there was an error in adding
* a new bpf-local-storage.
*
- * long bpf_sk_storage_delete(struct bpf_map *map, struct bpf_sock *sk)
+ * long bpf_sk_storage_delete(struct bpf_map *map, void *sk)
* Description
* Delete a bpf-local-storage from a *sk*.
* Return
--
2.27.0.389.gc38d7665816-goog
^ permalink raw reply related
* [PATCH bpf-next v3 4/4] bpf: Add selftests for local_storage
From: KP Singh @ 2020-07-09 0:56 UTC (permalink / raw)
To: linux-kernel, bpf, linux-security-module
Cc: Andrii Nakryiko, Alexei Starovoitov, Daniel Borkmann, Paul Turner,
Jann Horn, Florent Revest
In-Reply-To: <20200709005654.3324272-1-kpsingh@chromium.org>
From: KP Singh <kpsingh@google.com>
inode_local_storage:
* Hook to the file_open and inode_unlink LSM hooks.
* Create and unlink a temporary file.
* Store some information in the inode's bpf_local_storage during
file_open.
* Verify that this information exists when the file is unlinked.
sk_local_storage:
* Hook to the socket_post_create and socket_bind LSM hooks.
* Open and bind a socket and set the sk_storage in the
socket_post_create hook using the start_server helper.
* Verify if the information is set in the socket_bind hook.
Acked-by: Andrii Nakryiko <andriin@fb.com>
Signed-off-by: KP Singh <kpsingh@google.com>
---
.../bpf/prog_tests/test_local_storage.c | 60 ++++++++
.../selftests/bpf/progs/local_storage.c | 136 ++++++++++++++++++
2 files changed, 196 insertions(+)
create mode 100644 tools/testing/selftests/bpf/prog_tests/test_local_storage.c
create mode 100644 tools/testing/selftests/bpf/progs/local_storage.c
diff --git a/tools/testing/selftests/bpf/prog_tests/test_local_storage.c b/tools/testing/selftests/bpf/prog_tests/test_local_storage.c
new file mode 100644
index 000000000000..d4ba89195c43
--- /dev/null
+++ b/tools/testing/selftests/bpf/prog_tests/test_local_storage.c
@@ -0,0 +1,60 @@
+// SPDX-License-Identifier: GPL-2.0
+
+/*
+ * Copyright (C) 2020 Google LLC.
+ */
+
+#include <test_progs.h>
+#include <linux/limits.h>
+
+#include "local_storage.skel.h"
+#include "network_helpers.h"
+
+int create_and_unlink_file(void)
+{
+ char fname[PATH_MAX] = "/tmp/fileXXXXXX";
+ int fd;
+
+ fd = mkstemp(fname);
+ if (fd < 0)
+ return fd;
+
+ close(fd);
+ unlink(fname);
+ return 0;
+}
+
+void test_test_local_storage(void)
+{
+ struct local_storage *skel = NULL;
+ int err, duration = 0, serv_sk = -1;
+
+ skel = local_storage__open_and_load();
+ if (CHECK(!skel, "skel_load", "lsm skeleton failed\n"))
+ goto close_prog;
+
+ err = local_storage__attach(skel);
+ if (CHECK(err, "attach", "lsm attach failed: %d\n", err))
+ goto close_prog;
+
+ skel->bss->monitored_pid = getpid();
+
+ err = create_and_unlink_file();
+ if (CHECK(err < 0, "exec_cmd", "err %d errno %d\n", err, errno))
+ goto close_prog;
+
+ CHECK(!skel->bss->inode_storage_result, "inode_storage_result",
+ "inode_local_storage not set");
+
+ serv_sk = start_server(AF_INET6, SOCK_STREAM, NULL, 0, 0);
+ if (CHECK(serv_sk < 0, "start_server", "failed to start server\n"))
+ goto close_prog;
+
+ CHECK(!skel->bss->sk_storage_result, "sk_storage_result",
+ "sk_local_storage not set");
+
+ close(serv_sk);
+
+close_prog:
+ local_storage__destroy(skel);
+}
diff --git a/tools/testing/selftests/bpf/progs/local_storage.c b/tools/testing/selftests/bpf/progs/local_storage.c
new file mode 100644
index 000000000000..cb608b7b90f0
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/local_storage.c
@@ -0,0 +1,136 @@
+// SPDX-License-Identifier: GPL-2.0
+
+/*
+ * Copyright 2020 Google LLC.
+ */
+
+#include <errno.h>
+#include <linux/bpf.h>
+#include <stdbool.h>
+#include <bpf/bpf_helpers.h>
+#include <bpf/bpf_tracing.h>
+
+char _license[] SEC("license") = "GPL";
+
+#define DUMMY_STORAGE_VALUE 0xdeadbeef
+
+int monitored_pid = 0;
+bool inode_storage_result = false;
+bool sk_storage_result = false;
+
+struct dummy_storage {
+ __u32 value;
+};
+
+struct {
+ __uint(type, BPF_MAP_TYPE_INODE_STORAGE);
+ __uint(map_flags, BPF_F_NO_PREALLOC);
+ __type(key, int);
+ __type(value, struct dummy_storage);
+} inode_storage_map SEC(".maps");
+
+struct {
+ __uint(type, BPF_MAP_TYPE_SK_STORAGE);
+ __uint(map_flags, BPF_F_NO_PREALLOC | BPF_F_CLONE);
+ __type(key, int);
+ __type(value, struct dummy_storage);
+} sk_storage_map SEC(".maps");
+
+/* TODO Use vmlinux.h once BTF pruning for embedded types is fixed.
+ */
+struct sock {} __attribute__((preserve_access_index));
+struct sockaddr {} __attribute__((preserve_access_index));
+struct socket {
+ struct sock *sk;
+} __attribute__((preserve_access_index));
+
+struct inode {} __attribute__((preserve_access_index));
+struct dentry {
+ struct inode *d_inode;
+} __attribute__((preserve_access_index));
+struct file {
+ struct inode *f_inode;
+} __attribute__((preserve_access_index));
+
+
+SEC("lsm/inode_unlink")
+int BPF_PROG(unlink_hook, struct inode *dir, struct dentry *victim)
+{
+ __u32 pid = bpf_get_current_pid_tgid() >> 32;
+ struct dummy_storage *storage;
+
+ if (pid != monitored_pid)
+ return 0;
+
+ storage = bpf_inode_storage_get(&inode_storage_map, victim->d_inode, 0,
+ BPF_SK_STORAGE_GET_F_CREATE);
+ if (!storage)
+ return 0;
+
+ if (storage->value == DUMMY_STORAGE_VALUE)
+ inode_storage_result = true;
+
+ return 0;
+}
+
+SEC("lsm/socket_bind")
+int BPF_PROG(socket_bind, struct socket *sock, struct sockaddr *address,
+ int addrlen)
+{
+ __u32 pid = bpf_get_current_pid_tgid() >> 32;
+ struct dummy_storage *storage;
+
+ if (pid != monitored_pid)
+ return 0;
+
+ storage = bpf_sk_storage_get(&sk_storage_map, sock->sk, 0,
+ BPF_SK_STORAGE_GET_F_CREATE);
+ if (!storage)
+ return 0;
+
+ if (storage->value == DUMMY_STORAGE_VALUE)
+ sk_storage_result = true;
+
+ return 0;
+}
+
+SEC("lsm/socket_post_create")
+int BPF_PROG(socket_post_create, struct socket *sock, int family, int type,
+ int protocol, int kern)
+{
+ __u32 pid = bpf_get_current_pid_tgid() >> 32;
+ struct dummy_storage *storage;
+
+ if (pid != monitored_pid)
+ return 0;
+
+ storage = bpf_sk_storage_get(&sk_storage_map, sock->sk, 0,
+ BPF_SK_STORAGE_GET_F_CREATE);
+ if (!storage)
+ return 0;
+
+ storage->value = DUMMY_STORAGE_VALUE;
+
+ return 0;
+}
+
+SEC("lsm/file_open")
+int BPF_PROG(test_int_hook, struct file *file)
+{
+ __u32 pid = bpf_get_current_pid_tgid() >> 32;
+ struct dummy_storage *storage;
+
+ if (pid != monitored_pid)
+ return 0;
+
+ if (!file->f_inode)
+ return 0;
+
+ storage = bpf_inode_storage_get(&inode_storage_map, file->f_inode, 0,
+ BPF_LOCAL_STORAGE_GET_F_CREATE);
+ if (!storage)
+ return 0;
+
+ storage->value = DUMMY_STORAGE_VALUE;
+ return 0;
+}
--
2.27.0.389.gc38d7665816-goog
^ permalink raw reply related
* [PATCH bpf-next v3 1/4] bpf: Generalize bpf_sk_storage
From: KP Singh @ 2020-07-09 0:56 UTC (permalink / raw)
To: linux-kernel, bpf, linux-security-module
Cc: Alexei Starovoitov, Daniel Borkmann, Paul Turner, Jann Horn,
Florent Revest
In-Reply-To: <20200709005654.3324272-1-kpsingh@chromium.org>
From: KP Singh <kpsingh@google.com>
Refactor the functionality in bpf_sk_storage.c so that concept of
storage linked to kernel objects can be extended to other objects like
inode, task_struct etc.
bpf_sk_storage is updated to be bpf_local_storage with a union that
contains a pointer to the owner object. The type of the
bpf_local_storage can be determined using the newly added
bpf_local_storage_type enum.
Each new local storage will still be a separate map and provide its own
set of helpers. This allows for future object specific extensions and
still share a lot of the underlying implementation.
Signed-off-by: KP Singh <kpsingh@google.com>
---
include/linux/bpf.h | 14 +
include/linux/bpf_local_storage.h | 175 +++++++
include/net/sock.h | 4 +-
include/uapi/linux/bpf.h | 12 +-
kernel/bpf/Makefile | 1 +
kernel/bpf/bpf_local_storage.c | 517 +++++++++++++++++++
net/core/bpf_sk_storage.c | 804 ++++++------------------------
tools/include/uapi/linux/bpf.h | 12 +-
8 files changed, 879 insertions(+), 660 deletions(-)
create mode 100644 include/linux/bpf_local_storage.h
create mode 100644 kernel/bpf/bpf_local_storage.c
diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index 0cd7f6884c5c..95ab7031cd8e 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -33,6 +33,9 @@ struct btf;
struct btf_type;
struct exception_table_entry;
struct seq_operations;
+struct bpf_local_storage;
+struct bpf_local_storage_map;
+struct bpf_local_storage_elem;
extern struct idr btf_idr;
extern spinlock_t btf_idr_lock;
@@ -93,6 +96,17 @@ struct bpf_map_ops {
__poll_t (*map_poll)(struct bpf_map *map, struct file *filp,
struct poll_table_struct *pts);
+ /* Functions called by bpf_local_storage maps */
+ void (*map_local_storage_unlink)(struct bpf_local_storage *local_storage,
+ bool uncharge_omem);
+ struct bpf_local_storage_elem *(*map_selem_alloc)(
+ struct bpf_local_storage_map *smap, void *owner, void *value,
+ bool charge_omem);
+ struct bpf_local_storage_data *(*map_local_storage_update)(
+ void *owner, struct bpf_map *map, void *value, u64 flags);
+ int (*map_local_storage_alloc)(void *owner,
+ struct bpf_local_storage_map *smap,
+ struct bpf_local_storage_elem *elem);
/* BTF name and id of struct allocated by map_alloc */
const char * const map_btf_name;
int *map_btf_id;
diff --git a/include/linux/bpf_local_storage.h b/include/linux/bpf_local_storage.h
new file mode 100644
index 000000000000..605b81f2f806
--- /dev/null
+++ b/include/linux/bpf_local_storage.h
@@ -0,0 +1,175 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (c) 2019 Facebook
+ * Copyright 2020 Google LLC.
+ */
+
+#ifndef _BPF_LOCAL_STORAGE_H
+#define _BPF_LOCAL_STORAGE_H
+
+#include <linux/bpf.h>
+#include <linux/rculist.h>
+#include <linux/list.h>
+#include <linux/hash.h>
+#include <linux/types.h>
+#include <uapi/linux/btf.h>
+
+#define LOCAL_STORAGE_CREATE_FLAG_MASK \
+ (BPF_F_NO_PREALLOC | BPF_F_CLONE)
+
+struct bucket {
+ struct hlist_head list;
+ raw_spinlock_t lock;
+};
+
+/* Thp map is not the primary owner of a bpf_local_storage_elem.
+ * Instead, the container object (eg. sk->sk_bpf_storage) is.
+ *
+ * The map (bpf_local_storage_map) is for two purposes
+ * 1. Define the size of the "local storage". It is
+ * the map's value_size.
+ *
+ * 2. Maintain a list to keep track of all elems such
+ * that they can be cleaned up during the map destruction.
+ *
+ * When a bpf local storage is being looked up for a
+ * particular object, the "bpf_map" pointer is actually used
+ * as the "key" to search in the list of elem in
+ * the respective bpf_local_storage owned by the object.
+ *
+ * e.g. sk->sk_bpf_storage is the mini-map with the "bpf_map" pointer
+ * as the searching key.
+ */
+struct bpf_local_storage_map {
+ struct bpf_map map;
+ /* Lookup elem does not require accessing the map.
+ *
+ * Updating/Deleting requires a bucket lock to
+ * link/unlink the elem from the map. Having
+ * multiple buckets to improve contention.
+ */
+ struct bucket *buckets;
+ u32 bucket_log;
+ u16 elem_size;
+ u16 cache_idx;
+};
+
+struct bpf_local_storage_data {
+ /* smap is used as the searching key when looking up
+ * from the obejct's bpf_local_storage.
+ *
+ * Put it in the same cacheline as the data to minimize
+ * the number of cachelines access during the cache hit case.
+ */
+ struct bpf_local_storage_map __rcu *smap;
+ u8 data[] __aligned(8);
+};
+
+/* Linked to bpf_local_storage and bpf_local_storage_map */
+struct bpf_local_storage_elem {
+ struct hlist_node map_node; /* Linked to bpf_local_storage_map */
+ struct hlist_node snode; /* Linked to bpf_local_storage */
+ struct bpf_local_storage __rcu *local_storage;
+ struct rcu_head rcu;
+ /* 8 bytes hole */
+ /* The data is stored in aother cacheline to minimize
+ * the number of cachelines access during a cache hit.
+ */
+ struct bpf_local_storage_data sdata ____cacheline_aligned;
+};
+
+#define SELEM(_SDATA) \
+ container_of((_SDATA), struct bpf_local_storage_elem, sdata)
+#define SDATA(_SELEM) (&(_SELEM)->sdata)
+#define BPF_STORAGE_CACHE_SIZE 16
+
+u16 bpf_ls_cache_idx_get(spinlock_t *cache_idx_lock,
+ u64 *cache_idx_usage_count);
+
+void bpf_ls_cache_idx_free(spinlock_t *cache_idx_lock,
+ u64 *cache_idx_usage_counts, u16 idx);
+
+#define DEFINE_BPF_STORAGE_CACHE(type) \
+static DEFINE_SPINLOCK(cache_idx_lock_##type); \
+static u64 cache_idx_usage_counts_##type[BPF_STORAGE_CACHE_SIZE]; \
+static u16 cache_idx_get_##type(void) \
+{ \
+ return bpf_ls_cache_idx_get(&cache_idx_lock_##type, \
+ cache_idx_usage_counts_##type); \
+} \
+static void cache_idx_free_##type(u16 idx) \
+{ \
+ return bpf_ls_cache_idx_free(&cache_idx_lock_##type, \
+ cache_idx_usage_counts_##type, \
+ idx); \
+}
+
+/* U16_MAX is much more than enough for sk local storage
+ * considering a tcp_sock is ~2k.
+ */
+#define BPF_LOCAL_STORAGE_MAX_VALUE_SIZE \
+ min_t(u32, \
+ (KMALLOC_MAX_SIZE - MAX_BPF_STACK - \
+ sizeof(struct bpf_local_storage_elem)), \
+ (U16_MAX - sizeof(struct bpf_local_storage_elem)))
+
+struct bpf_local_storage {
+ struct bpf_local_storage_data __rcu *cache[BPF_STORAGE_CACHE_SIZE];
+ struct hlist_head list; /* List of bpf_local_storage_elem */
+ /* The object that owns the the above "list" of
+ * bpf_local_storage_elem
+ */
+ union {
+ struct sock *sk;
+ };
+ struct rcu_head rcu;
+ raw_spinlock_t lock; /* Protect adding/removing from the "list" */
+};
+
+/* Helper functions for bpf_local_storage */
+int bpf_local_storage_map_alloc_check(union bpf_attr *attr);
+
+struct bpf_local_storage_map *bpf_local_storage_map_alloc(union bpf_attr *attr);
+
+struct bpf_local_storage_data *
+bpf_local_storage_lookup(struct bpf_local_storage *local_storage,
+ struct bpf_local_storage_map *smap,
+ bool cacheit_lockit);
+
+void bpf_local_storage_map_free(struct bpf_local_storage_map *smap);
+
+int bpf_local_storage_map_check_btf(const struct bpf_map *map,
+ const struct btf *btf,
+ const struct btf_type *key_type,
+ const struct btf_type *value_type);
+
+void bpf_selem_link(struct bpf_local_storage *local_storage,
+ struct bpf_local_storage_elem *selem);
+
+bool bpf_selem_unlink(struct bpf_local_storage *local_storage,
+ struct bpf_local_storage_elem *selem, bool uncharge_omem);
+
+void bpf_selem_unlink_map_elem(struct bpf_local_storage_elem *selem);
+
+void bpf_selem_link_map(struct bpf_local_storage_map *smap,
+ struct bpf_local_storage_elem *selem);
+
+void bpf_selem_unlink_map(struct bpf_local_storage_elem *selem);
+
+struct bpf_local_storage_elem *
+bpf_selem_alloc(struct bpf_local_storage_map *smap, void *value);
+
+struct bpf_local_storage *
+bpf_local_storage_alloc(struct bpf_local_storage_map *smap);
+
+int bpf_local_storage_publish(struct bpf_local_storage_elem *first_selem,
+ struct bpf_local_storage **addr,
+ struct bpf_local_storage *curr);
+
+int bpf_local_storage_check_update_flags(struct bpf_map *map, u64 map_flags);
+
+struct bpf_local_storage_data *
+bpf_local_storage_update(void *owner, struct bpf_map *map,
+ struct bpf_local_storage *local_storage, void *value,
+ u64 map_flags);
+#endif /* _BPF_LOCAL_STORAGE_H */
diff --git a/include/net/sock.h b/include/net/sock.h
index 84c813dd0152..5eada8a5eb21 100644
--- a/include/net/sock.h
+++ b/include/net/sock.h
@@ -245,7 +245,7 @@ struct sock_common {
/* public: */
};
-struct bpf_sk_storage;
+struct bpf_local_storage;
/**
* struct sock - network layer representation of sockets
@@ -516,7 +516,7 @@ struct sock {
void (*sk_destruct)(struct sock *sk);
struct sock_reuseport __rcu *sk_reuseport_cb;
#ifdef CONFIG_BPF_SYSCALL
- struct bpf_sk_storage __rcu *sk_bpf_storage;
+ struct bpf_local_storage __rcu *sk_bpf_storage;
#endif
struct rcu_head sk_rcu;
};
diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index 548a749aebb3..1f3e831c4813 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -2802,10 +2802,10 @@ union bpf_attr {
* "type". The bpf-local-storage "type" (i.e. the *map*) is
* searched against all bpf-local-storages residing at *sk*.
*
- * An optional *flags* (**BPF_SK_STORAGE_GET_F_CREATE**) can be
+ * An optional *flags* (**BPF_LOCAL_STORAGE_GET_F_CREATE**) can be
* used such that a new bpf-local-storage will be
* created if one does not exist. *value* can be used
- * together with **BPF_SK_STORAGE_GET_F_CREATE** to specify
+ * together with **BPF_LOCAL_STORAGE_GET_F_CREATE** to specify
* the initial value of a bpf-local-storage. If *value* is
* **NULL**, the new bpf-local-storage will be zero initialized.
* Return
@@ -3572,9 +3572,13 @@ enum {
BPF_F_SYSCTL_BASE_NAME = (1ULL << 0),
};
-/* BPF_FUNC_sk_storage_get flags */
+/* BPF_FUNC_<local>_storage_get flags */
enum {
- BPF_SK_STORAGE_GET_F_CREATE = (1ULL << 0),
+ BPF_LOCAL_STORAGE_GET_F_CREATE = (1ULL << 0),
+ /* BPF_SK_STORAGE_GET_F_CREATE is only kept for backward compatibility
+ * and BPF_LOCAL_STORAGE_GET_F_CREATE must be used instead.
+ */
+ BPF_SK_STORAGE_GET_F_CREATE = BPF_LOCAL_STORAGE_GET_F_CREATE,
};
/* BPF_FUNC_read_branch_records flags. */
diff --git a/kernel/bpf/Makefile b/kernel/bpf/Makefile
index 1131a921e1a6..0acb8f8a6042 100644
--- a/kernel/bpf/Makefile
+++ b/kernel/bpf/Makefile
@@ -12,6 +12,7 @@ obj-$(CONFIG_BPF_JIT) += dispatcher.o
ifeq ($(CONFIG_NET),y)
obj-$(CONFIG_BPF_SYSCALL) += devmap.o
obj-$(CONFIG_BPF_SYSCALL) += cpumap.o
+obj-$(CONFIG_BPF_SYSCALL) += bpf_local_storage.o
obj-$(CONFIG_BPF_SYSCALL) += offload.o
obj-$(CONFIG_BPF_SYSCALL) += net_namespace.o
endif
diff --git a/kernel/bpf/bpf_local_storage.c b/kernel/bpf/bpf_local_storage.c
new file mode 100644
index 000000000000..c818eb6f8261
--- /dev/null
+++ b/kernel/bpf/bpf_local_storage.c
@@ -0,0 +1,517 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2019 Facebook
+ * Copyright 2020 Google LLC.
+ */
+
+#include <linux/rculist.h>
+#include <linux/list.h>
+#include <linux/hash.h>
+#include <linux/types.h>
+#include <linux/spinlock.h>
+#include <linux/bpf.h>
+#include <linux/bpf_local_storage.h>
+#include <net/sock.h>
+#include <uapi/linux/sock_diag.h>
+#include <uapi/linux/btf.h>
+
+#define SELEM(_SDATA) \
+ container_of((_SDATA), struct bpf_local_storage_elem, sdata)
+#define SDATA(_SELEM) (&(_SELEM)->sdata)
+
+static struct bucket *select_bucket(struct bpf_local_storage_map *smap,
+ struct bpf_local_storage_elem *selem)
+{
+ return &smap->buckets[hash_ptr(selem, smap->bucket_log)];
+}
+
+static bool selem_linked_to_node(const struct bpf_local_storage_elem *selem)
+{
+ return !hlist_unhashed(&selem->snode);
+}
+
+static bool selem_linked_to_map(const struct bpf_local_storage_elem *selem)
+{
+ return !hlist_unhashed(&selem->map_node);
+}
+
+struct bpf_local_storage_elem *
+bpf_selem_alloc(struct bpf_local_storage_map *smap, void *value)
+{
+ struct bpf_local_storage_elem *selem;
+
+ selem = kzalloc(smap->elem_size, GFP_ATOMIC | __GFP_NOWARN);
+ if (selem) {
+ if (value)
+ memcpy(SDATA(selem)->data, value, smap->map.value_size);
+ return selem;
+ }
+
+ return NULL;
+}
+
+/* local_storage->lock must be held and selem->local_storage == local_storage.
+ * The caller must ensure selem->smap is still valid to be
+ * dereferenced for its smap->elem_size and smap->cache_idx.
+ *
+ * uncharge_omem is only relevant for BPF_MAP_TYPE_SK_STORAGE.
+ */
+bool bpf_selem_unlink(struct bpf_local_storage *local_storage,
+ struct bpf_local_storage_elem *selem, bool uncharge_omem)
+{
+ struct bpf_local_storage_map *smap;
+ bool free_local_storage;
+
+ smap = rcu_dereference(SDATA(selem)->smap);
+ free_local_storage = hlist_is_singular_node(&selem->snode,
+ &local_storage->list);
+
+ /* local_storage is not freed now. local_storage->lock is
+ * still held and raw_spin_unlock_bh(&local_storage->lock)
+ * will be done by the caller.
+ * Although the unlock will be done under
+ * rcu_read_lock(), it is more intutivie to
+ * read if kfree_rcu(local_storage, rcu) is done
+ * after the raw_spin_unlock_bh(&local_storage->lock).
+ *
+ * Hence, a "bool free_local_storage" is returned
+ * to the caller which then calls the kfree_rcu()
+ * after unlock.
+ */
+ if (free_local_storage)
+ smap->map.ops->map_local_storage_unlink(local_storage,
+ uncharge_omem);
+
+ hlist_del_init_rcu(&selem->snode);
+ if (rcu_access_pointer(local_storage->cache[smap->cache_idx]) ==
+ SDATA(selem))
+ RCU_INIT_POINTER(local_storage->cache[smap->cache_idx], NULL);
+
+ kfree_rcu(selem, rcu);
+
+ return free_local_storage;
+}
+
+void bpf_selem_link(struct bpf_local_storage *local_storage,
+ struct bpf_local_storage_elem *selem)
+{
+ RCU_INIT_POINTER(selem->local_storage, local_storage);
+ hlist_add_head(&selem->snode, &local_storage->list);
+}
+
+void bpf_selem_unlink_map(struct bpf_local_storage_elem *selem)
+{
+ struct bpf_local_storage_map *smap;
+ struct bucket *b;
+
+ if (unlikely(!selem_linked_to_map(selem)))
+ /* selem has already be unlinked from smap */
+ return;
+
+ smap = rcu_dereference(SDATA(selem)->smap);
+ b = select_bucket(smap, selem);
+ raw_spin_lock_bh(&b->lock);
+ if (likely(selem_linked_to_map(selem)))
+ hlist_del_init_rcu(&selem->map_node);
+ raw_spin_unlock_bh(&b->lock);
+}
+
+void bpf_selem_link_map(struct bpf_local_storage_map *smap,
+ struct bpf_local_storage_elem *selem)
+{
+ struct bucket *b = select_bucket(smap, selem);
+
+ raw_spin_lock_bh(&b->lock);
+ RCU_INIT_POINTER(SDATA(selem)->smap, smap);
+ hlist_add_head_rcu(&selem->map_node, &b->list);
+ raw_spin_unlock_bh(&b->lock);
+}
+
+void bpf_selem_unlink_map_elem(struct bpf_local_storage_elem *selem)
+{
+ struct bpf_local_storage *local_storage;
+ bool free_local_storage = false;
+
+ /* Always unlink from map before unlinking from local_storage
+ * because selem will be freed after successfully unlinked from
+ * the local_storage.
+ */
+ bpf_selem_unlink_map(selem);
+
+ if (unlikely(!selem_linked_to_node(selem)))
+ /* selem has already been unlinked from its owner */
+ return;
+
+ local_storage = rcu_dereference(selem->local_storage);
+ raw_spin_lock_bh(&local_storage->lock);
+ if (likely(selem_linked_to_node(selem)))
+ free_local_storage =
+ bpf_selem_unlink(local_storage, selem, true);
+ raw_spin_unlock_bh(&local_storage->lock);
+
+ if (free_local_storage)
+ kfree_rcu(local_storage, rcu);
+}
+
+struct bpf_local_storage_data *
+bpf_local_storage_lookup(struct bpf_local_storage *local_storage,
+ struct bpf_local_storage_map *smap,
+ bool cacheit_lockit)
+{
+ struct bpf_local_storage_data *sdata;
+ struct bpf_local_storage_elem *selem;
+
+ /* Fast path (cache hit) */
+ sdata = rcu_dereference(local_storage->cache[smap->cache_idx]);
+ if (sdata && rcu_access_pointer(sdata->smap) == smap)
+ return sdata;
+
+ /* Slow path (cache miss) */
+ hlist_for_each_entry_rcu(selem, &local_storage->list, snode)
+ if (rcu_access_pointer(SDATA(selem)->smap) == smap)
+ break;
+
+ if (!selem)
+ return NULL;
+
+ sdata = SDATA(selem);
+ if (cacheit_lockit) {
+ /* spinlock is needed to avoid racing with the
+ * parallel delete. Otherwise, publishing an already
+ * deleted sdata to the cache will become a use-after-free
+ * problem in the next bpf_local_storage_lookup().
+ */
+ raw_spin_lock_bh(&local_storage->lock);
+ if (selem_linked_to_node(selem))
+ rcu_assign_pointer(
+ local_storage->cache[smap->cache_idx], sdata);
+ raw_spin_unlock_bh(&local_storage->lock);
+ }
+
+ return sdata;
+}
+
+static int check_flags(const struct bpf_local_storage_data *old_sdata,
+ u64 map_flags)
+{
+ if (old_sdata && (map_flags & ~BPF_F_LOCK) == BPF_NOEXIST)
+ /* elem already exists */
+ return -EEXIST;
+
+ if (!old_sdata && (map_flags & ~BPF_F_LOCK) == BPF_EXIST)
+ /* elem doesn't exist, cannot update it */
+ return -ENOENT;
+
+ return 0;
+}
+
+struct bpf_local_storage *
+bpf_local_storage_alloc(struct bpf_local_storage_map *smap)
+{
+ struct bpf_local_storage *storage;
+
+ storage = kzalloc(sizeof(*storage), GFP_ATOMIC | __GFP_NOWARN);
+ if (!storage)
+ return NULL;
+
+ INIT_HLIST_HEAD(&storage->list);
+ raw_spin_lock_init(&storage->lock);
+ return storage;
+}
+
+/* Publish local_storage to the address. This is used because we are already
+ * in a region where we cannot grab a lock on the object owning the storage (
+ * (e.g sk->sk_lock). Hence, atomic ops is used.
+ *
+ * From now on, the addr pointer is protected
+ * by the local_storage->lock. Hence, upon freeing,
+ * the local_storage->lock must be held before unlinking the storage from the
+ * owner.
+ */
+int bpf_local_storage_publish(struct bpf_local_storage_elem *first_selem,
+ struct bpf_local_storage **addr,
+ struct bpf_local_storage *curr)
+{
+ struct bpf_local_storage *prev;
+
+ prev = cmpxchg(addr, NULL, curr);
+ if (unlikely(prev)) {
+ /* Note that even first_selem was linked to smap's
+ * bucket->list, first_selem can be freed immediately
+ * (instead of kfree_rcu) because
+ * bpf_local_storage_map_free() does a
+ * synchronize_rcu() before walking the bucket->list.
+ * Hence, no one is accessing selem from the
+ * bucket->list under rcu_read_lock().
+ */
+ bpf_selem_unlink_map(first_selem);
+ return -EAGAIN;
+ }
+
+ return 0;
+}
+
+int bpf_local_storage_check_update_flags(struct bpf_map *map, u64 map_flags)
+{
+ /* BPF_EXIST and BPF_NOEXIST cannot be both set */
+ if (unlikely((map_flags & ~BPF_F_LOCK) > BPF_EXIST) ||
+ /* BPF_F_LOCK can only be used in a value with spin_lock */
+ unlikely((map_flags & BPF_F_LOCK) && !map_value_has_spin_lock(map)))
+ return -EINVAL;
+
+ return 0;
+}
+
+struct bpf_local_storage_data *
+bpf_local_storage_update(void *owner, struct bpf_map *map,
+ struct bpf_local_storage *local_storage, void *value,
+ u64 map_flags)
+{
+ struct bpf_local_storage_data *old_sdata = NULL;
+ struct bpf_local_storage_elem *selem;
+ struct bpf_local_storage_map *smap;
+ int err;
+
+ smap = (struct bpf_local_storage_map *)map;
+
+ if ((map_flags & BPF_F_LOCK) && !(map_flags & BPF_NOEXIST)) {
+ /* Hoping to find an old_sdata to do inline update
+ * such that it can avoid taking the local_storage->lock
+ * and changing the lists.
+ */
+ old_sdata = bpf_local_storage_lookup(local_storage, smap, false);
+ err = check_flags(old_sdata, map_flags);
+ if (err)
+ return ERR_PTR(err);
+
+ if (old_sdata && selem_linked_to_node(SELEM(old_sdata))) {
+ copy_map_value_locked(map, old_sdata->data,
+ value, false);
+ return old_sdata;
+ }
+ }
+
+ raw_spin_lock_bh(&local_storage->lock);
+
+ /* Recheck local_storage->list under local_storage->lock */
+ if (unlikely(hlist_empty(&local_storage->list))) {
+ /* A parallel del is happening and local_storage is going
+ * away. It has just been checked before, so very
+ * unlikely. Return instead of retry to keep things
+ * simple.
+ */
+ err = -EAGAIN;
+ goto unlock_err;
+ }
+
+ old_sdata = bpf_local_storage_lookup(local_storage, smap, false);
+ err = check_flags(old_sdata, map_flags);
+ if (err)
+ goto unlock_err;
+
+ if (old_sdata && (map_flags & BPF_F_LOCK)) {
+ copy_map_value_locked(map, old_sdata->data, value, false);
+ selem = SELEM(old_sdata);
+ goto unlock;
+ }
+
+ /* local_storage->lock is held. Hence, we are sure
+ * we can unlink and uncharge the old_sdata successfully
+ * later. Hence, instead of charging the new selem now
+ * and then uncharge the old selem later (which may cause
+ * a potential but unnecessary charge failure), avoid taking
+ * a charge at all here (the "!old_sdata" check) and the
+ * old_sdata will not be uncharged later during bpf_selem_unlink().
+ */
+ selem = map->ops->map_selem_alloc(smap, owner, value, !old_sdata);
+ if (!selem) {
+ err = -ENOMEM;
+ goto unlock_err;
+ }
+
+ /* First, link the new selem to the map */
+ bpf_selem_link_map(smap, selem);
+
+ /* Second, link (and publish) the new selem to local_storage */
+ bpf_selem_link(local_storage, selem);
+
+ /* Third, remove old selem, SELEM(old_sdata) */
+ if (old_sdata) {
+ bpf_selem_unlink_map(SELEM(old_sdata));
+ bpf_selem_unlink(local_storage, SELEM(old_sdata), false);
+ }
+
+unlock:
+ raw_spin_unlock_bh(&local_storage->lock);
+ return SDATA(selem);
+
+unlock_err:
+ raw_spin_unlock_bh(&local_storage->lock);
+ return ERR_PTR(err);
+}
+
+u16 bpf_ls_cache_idx_get(spinlock_t *cache_idx_lock,
+ u64 *cache_idx_usage_counts)
+{
+ u64 min_usage = U64_MAX;
+ u16 i, res = 0;
+
+ spin_lock(cache_idx_lock);
+
+ for (i = 0; i < BPF_STORAGE_CACHE_SIZE; i++) {
+ if (cache_idx_usage_counts[i] < min_usage) {
+ min_usage = cache_idx_usage_counts[i];
+ res = i;
+
+ /* Found a free cache_idx */
+ if (!min_usage)
+ break;
+ }
+ }
+ cache_idx_usage_counts[res]++;
+
+ spin_unlock(cache_idx_lock);
+
+ return res;
+}
+
+void bpf_ls_cache_idx_free(spinlock_t *cache_idx_lock,
+ u64 *cache_idx_usage_counts, u16 idx)
+{
+ spin_lock(cache_idx_lock);
+ cache_idx_usage_counts[idx]--;
+ spin_unlock(cache_idx_lock);
+}
+
+void bpf_local_storage_map_free(struct bpf_local_storage_map *smap)
+{
+ struct bpf_local_storage_elem *selem;
+ struct bucket *b;
+ unsigned int i;
+
+ /* Note that this map might be concurrently cloned from
+ * bpf_sk_storage_clone. Wait for any existing bpf_sk_storage_clone
+ * RCU read section to finish before proceeding. New RCU
+ * read sections should be prevented via bpf_map_inc_not_zero.
+ */
+ synchronize_rcu();
+
+ /* bpf prog and the userspace can no longer access this map
+ * now. No new selem (of this map) can be added
+ * to the bpf_local_storage or to the map bucket's list.
+ *
+ * The elem of this map can be cleaned up here
+ * or by bpf_local_storage_free() during the destruction of the
+ * owner object. eg. __sk_destruct.
+ */
+ for (i = 0; i < (1U << smap->bucket_log); i++) {
+ b = &smap->buckets[i];
+
+ rcu_read_lock();
+ /* No one is adding to b->list now */
+ while ((selem = hlist_entry_safe(rcu_dereference_raw(hlist_first_rcu(&b->list)),
+ struct bpf_local_storage_elem,
+ map_node))) {
+ bpf_selem_unlink_map_elem(selem);
+ cond_resched_rcu();
+ }
+ rcu_read_unlock();
+ }
+
+ /* bpf_local_storage_free() may still need to access the map.
+ * e.g. bpf_local_storage_free() has unlinked selem from the map
+ * which then made the above while((selem = ...)) loop
+ * exited immediately.
+ *
+ * However, the bpf_local_storage_free() still needs to access
+ * the smap->elem_size to do the uncharging in
+ * bpf_selem_unlink().
+ *
+ * Hence, wait another rcu grace period for the
+ * bpf_local_storage_free() to finish.
+ */
+ synchronize_rcu();
+
+ kvfree(smap->buckets);
+ kfree(smap);
+}
+
+int bpf_local_storage_map_alloc_check(union bpf_attr *attr)
+{
+ if (attr->map_flags & ~LOCAL_STORAGE_CREATE_FLAG_MASK ||
+ !(attr->map_flags & BPF_F_NO_PREALLOC) ||
+ attr->max_entries ||
+ attr->key_size != sizeof(int) || !attr->value_size ||
+ /* Enforce BTF for userspace sk dumping */
+ !attr->btf_key_type_id || !attr->btf_value_type_id)
+ return -EINVAL;
+
+ if (!bpf_capable())
+ return -EPERM;
+
+ if (attr->value_size > BPF_LOCAL_STORAGE_MAX_VALUE_SIZE)
+ return -E2BIG;
+
+ return 0;
+}
+
+struct bpf_local_storage_map *bpf_local_storage_map_alloc(union bpf_attr *attr)
+{
+ struct bpf_local_storage_map *smap;
+ unsigned int i;
+ u32 nbuckets;
+ u64 cost;
+ int ret;
+
+ smap = kzalloc(sizeof(*smap), GFP_USER | __GFP_NOWARN);
+ if (!smap)
+ return ERR_PTR(-ENOMEM);
+ bpf_map_init_from_attr(&smap->map, attr);
+
+ nbuckets = roundup_pow_of_two(num_possible_cpus());
+ /* Use at least 2 buckets, select_bucket() is undefined behavior with 1 bucket */
+ nbuckets = max_t(u32, 2, nbuckets);
+ smap->bucket_log = ilog2(nbuckets);
+ cost = sizeof(*smap->buckets) * nbuckets + sizeof(*smap);
+
+ ret = bpf_map_charge_init(&smap->map.memory, cost);
+ if (ret < 0) {
+ kfree(smap);
+ return ERR_PTR(ret);
+ }
+
+ smap->buckets = kvcalloc(sizeof(*smap->buckets), nbuckets,
+ GFP_USER | __GFP_NOWARN);
+ if (!smap->buckets) {
+ bpf_map_charge_finish(&smap->map.memory);
+ kfree(smap);
+ return ERR_PTR(-ENOMEM);
+ }
+
+ for (i = 0; i < nbuckets; i++) {
+ INIT_HLIST_HEAD(&smap->buckets[i].list);
+ raw_spin_lock_init(&smap->buckets[i].lock);
+ }
+
+ smap->elem_size =
+ sizeof(struct bpf_local_storage_elem) + attr->value_size;
+
+ return smap;
+}
+
+int bpf_local_storage_map_check_btf(const struct bpf_map *map,
+ const struct btf *btf,
+ const struct btf_type *key_type,
+ const struct btf_type *value_type)
+{
+ u32 int_data;
+
+ if (BTF_INFO_KIND(key_type->info) != BTF_KIND_INT)
+ return -EINVAL;
+
+ int_data = *(u32 *)(key_type + 1);
+ if (BTF_INT_BITS(int_data) != 32 || BTF_INT_OFFSET(int_data))
+ return -EINVAL;
+
+ return 0;
+}
diff --git a/net/core/bpf_sk_storage.c b/net/core/bpf_sk_storage.c
index 6f921c4ddc2c..62b6526acf7e 100644
--- a/net/core/bpf_sk_storage.c
+++ b/net/core/bpf_sk_storage.c
@@ -1,103 +1,19 @@
// SPDX-License-Identifier: GPL-2.0
/* Copyright (c) 2019 Facebook */
+#include "linux/bpf.h"
+#include "asm-generic/bug.h"
+#include "linux/err.h"
#include <linux/rculist.h>
#include <linux/list.h>
#include <linux/hash.h>
#include <linux/types.h>
#include <linux/spinlock.h>
#include <linux/bpf.h>
-#include <net/bpf_sk_storage.h>
+#include <linux/bpf_local_storage.h>
#include <net/sock.h>
#include <uapi/linux/sock_diag.h>
#include <uapi/linux/btf.h>
-#define SK_STORAGE_CREATE_FLAG_MASK \
- (BPF_F_NO_PREALLOC | BPF_F_CLONE)
-
-struct bucket {
- struct hlist_head list;
- raw_spinlock_t lock;
-};
-
-/* Thp map is not the primary owner of a bpf_sk_storage_elem.
- * Instead, the sk->sk_bpf_storage is.
- *
- * The map (bpf_sk_storage_map) is for two purposes
- * 1. Define the size of the "sk local storage". It is
- * the map's value_size.
- *
- * 2. Maintain a list to keep track of all elems such
- * that they can be cleaned up during the map destruction.
- *
- * When a bpf local storage is being looked up for a
- * particular sk, the "bpf_map" pointer is actually used
- * as the "key" to search in the list of elem in
- * sk->sk_bpf_storage.
- *
- * Hence, consider sk->sk_bpf_storage is the mini-map
- * with the "bpf_map" pointer as the searching key.
- */
-struct bpf_sk_storage_map {
- struct bpf_map map;
- /* Lookup elem does not require accessing the map.
- *
- * Updating/Deleting requires a bucket lock to
- * link/unlink the elem from the map. Having
- * multiple buckets to improve contention.
- */
- struct bucket *buckets;
- u32 bucket_log;
- u16 elem_size;
- u16 cache_idx;
-};
-
-struct bpf_sk_storage_data {
- /* smap is used as the searching key when looking up
- * from sk->sk_bpf_storage.
- *
- * Put it in the same cacheline as the data to minimize
- * the number of cachelines access during the cache hit case.
- */
- struct bpf_sk_storage_map __rcu *smap;
- u8 data[] __aligned(8);
-};
-
-/* Linked to bpf_sk_storage and bpf_sk_storage_map */
-struct bpf_sk_storage_elem {
- struct hlist_node map_node; /* Linked to bpf_sk_storage_map */
- struct hlist_node snode; /* Linked to bpf_sk_storage */
- struct bpf_sk_storage __rcu *sk_storage;
- struct rcu_head rcu;
- /* 8 bytes hole */
- /* The data is stored in aother cacheline to minimize
- * the number of cachelines access during a cache hit.
- */
- struct bpf_sk_storage_data sdata ____cacheline_aligned;
-};
-
-#define SELEM(_SDATA) container_of((_SDATA), struct bpf_sk_storage_elem, sdata)
-#define SDATA(_SELEM) (&(_SELEM)->sdata)
-#define BPF_SK_STORAGE_CACHE_SIZE 16
-
-static DEFINE_SPINLOCK(cache_idx_lock);
-static u64 cache_idx_usage_counts[BPF_SK_STORAGE_CACHE_SIZE];
-
-struct bpf_sk_storage {
- struct bpf_sk_storage_data __rcu *cache[BPF_SK_STORAGE_CACHE_SIZE];
- struct hlist_head list; /* List of bpf_sk_storage_elem */
- struct sock *sk; /* The sk that owns the the above "list" of
- * bpf_sk_storage_elem.
- */
- struct rcu_head rcu;
- raw_spinlock_t lock; /* Protect adding/removing from the "list" */
-};
-
-static struct bucket *select_bucket(struct bpf_sk_storage_map *smap,
- struct bpf_sk_storage_elem *selem)
-{
- return &smap->buckets[hash_ptr(selem, smap->bucket_log)];
-}
-
static int omem_charge(struct sock *sk, unsigned int size)
{
/* same check as in sock_kmalloc() */
@@ -110,31 +26,19 @@ static int omem_charge(struct sock *sk, unsigned int size)
return -ENOMEM;
}
-static bool selem_linked_to_sk(const struct bpf_sk_storage_elem *selem)
-{
- return !hlist_unhashed(&selem->snode);
-}
-
-static bool selem_linked_to_map(const struct bpf_sk_storage_elem *selem)
+static struct bpf_local_storage_elem *
+sk_selem_alloc(struct bpf_local_storage_map *smap, void *owner, void *value,
+ bool charge_omem)
{
- return !hlist_unhashed(&selem->map_node);
-}
-
-static struct bpf_sk_storage_elem *selem_alloc(struct bpf_sk_storage_map *smap,
- struct sock *sk, void *value,
- bool charge_omem)
-{
- struct bpf_sk_storage_elem *selem;
+ struct bpf_local_storage_elem *selem;
+ struct sock *sk = owner;
if (charge_omem && omem_charge(sk, smap->elem_size))
return NULL;
- selem = kzalloc(smap->elem_size, GFP_ATOMIC | __GFP_NOWARN);
- if (selem) {
- if (value)
- memcpy(SDATA(selem)->data, value, smap->map.value_size);
+ selem = bpf_selem_alloc(smap, value);
+ if (selem)
return selem;
- }
if (charge_omem)
atomic_sub(smap->elem_size, &sk->sk_omem_alloc);
@@ -142,242 +46,53 @@ static struct bpf_sk_storage_elem *selem_alloc(struct bpf_sk_storage_map *smap,
return NULL;
}
-/* sk_storage->lock must be held and selem->sk_storage == sk_storage.
- * The caller must ensure selem->smap is still valid to be
- * dereferenced for its smap->elem_size and smap->cache_idx.
- */
-static bool __selem_unlink_sk(struct bpf_sk_storage *sk_storage,
- struct bpf_sk_storage_elem *selem,
+static void unlink_sk_storage(struct bpf_local_storage *local_storage,
bool uncharge_omem)
{
- struct bpf_sk_storage_map *smap;
- bool free_sk_storage;
- struct sock *sk;
-
- smap = rcu_dereference(SDATA(selem)->smap);
- sk = sk_storage->sk;
+ struct sock *sk = local_storage->sk;
- /* All uncharging on sk->sk_omem_alloc must be done first.
- * sk may be freed once the last selem is unlinked from sk_storage.
- */
if (uncharge_omem)
- atomic_sub(smap->elem_size, &sk->sk_omem_alloc);
-
- free_sk_storage = hlist_is_singular_node(&selem->snode,
- &sk_storage->list);
- if (free_sk_storage) {
- atomic_sub(sizeof(struct bpf_sk_storage), &sk->sk_omem_alloc);
- sk_storage->sk = NULL;
- /* After this RCU_INIT, sk may be freed and cannot be used */
- RCU_INIT_POINTER(sk->sk_bpf_storage, NULL);
-
- /* sk_storage is not freed now. sk_storage->lock is
- * still held and raw_spin_unlock_bh(&sk_storage->lock)
- * will be done by the caller.
- *
- * Although the unlock will be done under
- * rcu_read_lock(), it is more intutivie to
- * read if kfree_rcu(sk_storage, rcu) is done
- * after the raw_spin_unlock_bh(&sk_storage->lock).
- *
- * Hence, a "bool free_sk_storage" is returned
- * to the caller which then calls the kfree_rcu()
- * after unlock.
- */
- }
- hlist_del_init_rcu(&selem->snode);
- if (rcu_access_pointer(sk_storage->cache[smap->cache_idx]) ==
- SDATA(selem))
- RCU_INIT_POINTER(sk_storage->cache[smap->cache_idx], NULL);
-
- kfree_rcu(selem, rcu);
-
- return free_sk_storage;
-}
-
-static void selem_unlink_sk(struct bpf_sk_storage_elem *selem)
-{
- struct bpf_sk_storage *sk_storage;
- bool free_sk_storage = false;
-
- if (unlikely(!selem_linked_to_sk(selem)))
- /* selem has already been unlinked from sk */
- return;
-
- sk_storage = rcu_dereference(selem->sk_storage);
- raw_spin_lock_bh(&sk_storage->lock);
- if (likely(selem_linked_to_sk(selem)))
- free_sk_storage = __selem_unlink_sk(sk_storage, selem, true);
- raw_spin_unlock_bh(&sk_storage->lock);
-
- if (free_sk_storage)
- kfree_rcu(sk_storage, rcu);
-}
+ atomic_sub(sizeof(struct bpf_local_storage),
+ &sk->sk_omem_alloc);
-static void __selem_link_sk(struct bpf_sk_storage *sk_storage,
- struct bpf_sk_storage_elem *selem)
-{
- RCU_INIT_POINTER(selem->sk_storage, sk_storage);
- hlist_add_head(&selem->snode, &sk_storage->list);
+ /* After this RCU_INIT, sk may be freed and cannot be used */
+ RCU_INIT_POINTER(sk->sk_bpf_storage, NULL);
+ local_storage->sk = NULL;
}
-static void selem_unlink_map(struct bpf_sk_storage_elem *selem)
+static int sk_storage_alloc(void *owner,
+ struct bpf_local_storage_map *smap,
+ struct bpf_local_storage_elem *first_selem)
{
- struct bpf_sk_storage_map *smap;
- struct bucket *b;
-
- if (unlikely(!selem_linked_to_map(selem)))
- /* selem has already be unlinked from smap */
- return;
-
- smap = rcu_dereference(SDATA(selem)->smap);
- b = select_bucket(smap, selem);
- raw_spin_lock_bh(&b->lock);
- if (likely(selem_linked_to_map(selem)))
- hlist_del_init_rcu(&selem->map_node);
- raw_spin_unlock_bh(&b->lock);
-}
-
-static void selem_link_map(struct bpf_sk_storage_map *smap,
- struct bpf_sk_storage_elem *selem)
-{
- struct bucket *b = select_bucket(smap, selem);
-
- raw_spin_lock_bh(&b->lock);
- RCU_INIT_POINTER(SDATA(selem)->smap, smap);
- hlist_add_head_rcu(&selem->map_node, &b->list);
- raw_spin_unlock_bh(&b->lock);
-}
-
-static void selem_unlink(struct bpf_sk_storage_elem *selem)
-{
- /* Always unlink from map before unlinking from sk_storage
- * because selem will be freed after successfully unlinked from
- * the sk_storage.
- */
- selem_unlink_map(selem);
- selem_unlink_sk(selem);
-}
-
-static struct bpf_sk_storage_data *
-__sk_storage_lookup(struct bpf_sk_storage *sk_storage,
- struct bpf_sk_storage_map *smap,
- bool cacheit_lockit)
-{
- struct bpf_sk_storage_data *sdata;
- struct bpf_sk_storage_elem *selem;
-
- /* Fast path (cache hit) */
- sdata = rcu_dereference(sk_storage->cache[smap->cache_idx]);
- if (sdata && rcu_access_pointer(sdata->smap) == smap)
- return sdata;
-
- /* Slow path (cache miss) */
- hlist_for_each_entry_rcu(selem, &sk_storage->list, snode)
- if (rcu_access_pointer(SDATA(selem)->smap) == smap)
- break;
-
- if (!selem)
- return NULL;
-
- sdata = SDATA(selem);
- if (cacheit_lockit) {
- /* spinlock is needed to avoid racing with the
- * parallel delete. Otherwise, publishing an already
- * deleted sdata to the cache will become a use-after-free
- * problem in the next __sk_storage_lookup().
- */
- raw_spin_lock_bh(&sk_storage->lock);
- if (selem_linked_to_sk(selem))
- rcu_assign_pointer(sk_storage->cache[smap->cache_idx],
- sdata);
- raw_spin_unlock_bh(&sk_storage->lock);
- }
-
- return sdata;
-}
-
-static struct bpf_sk_storage_data *
-sk_storage_lookup(struct sock *sk, struct bpf_map *map, bool cacheit_lockit)
-{
- struct bpf_sk_storage *sk_storage;
- struct bpf_sk_storage_map *smap;
-
- sk_storage = rcu_dereference(sk->sk_bpf_storage);
- if (!sk_storage)
- return NULL;
-
- smap = (struct bpf_sk_storage_map *)map;
- return __sk_storage_lookup(sk_storage, smap, cacheit_lockit);
-}
-
-static int check_flags(const struct bpf_sk_storage_data *old_sdata,
- u64 map_flags)
-{
- if (old_sdata && (map_flags & ~BPF_F_LOCK) == BPF_NOEXIST)
- /* elem already exists */
- return -EEXIST;
-
- if (!old_sdata && (map_flags & ~BPF_F_LOCK) == BPF_EXIST)
- /* elem doesn't exist, cannot update it */
- return -ENOENT;
-
- return 0;
-}
-
-static int sk_storage_alloc(struct sock *sk,
- struct bpf_sk_storage_map *smap,
- struct bpf_sk_storage_elem *first_selem)
-{
- struct bpf_sk_storage *prev_sk_storage, *sk_storage;
+ struct bpf_local_storage *curr;
+ struct sock *sk = owner;
int err;
- err = omem_charge(sk, sizeof(*sk_storage));
+ err = omem_charge(sk, sizeof(*curr));
if (err)
return err;
- sk_storage = kzalloc(sizeof(*sk_storage), GFP_ATOMIC | __GFP_NOWARN);
- if (!sk_storage) {
+ curr = bpf_local_storage_alloc(smap);
+ if (!curr) {
err = -ENOMEM;
goto uncharge;
}
- INIT_HLIST_HEAD(&sk_storage->list);
- raw_spin_lock_init(&sk_storage->lock);
- sk_storage->sk = sk;
-
- __selem_link_sk(sk_storage, first_selem);
- selem_link_map(smap, first_selem);
- /* Publish sk_storage to sk. sk->sk_lock cannot be acquired.
- * Hence, atomic ops is used to set sk->sk_bpf_storage
- * from NULL to the newly allocated sk_storage ptr.
- *
- * From now on, the sk->sk_bpf_storage pointer is protected
- * by the sk_storage->lock. Hence, when freeing
- * the sk->sk_bpf_storage, the sk_storage->lock must
- * be held before setting sk->sk_bpf_storage to NULL.
- */
- prev_sk_storage = cmpxchg((struct bpf_sk_storage **)&sk->sk_bpf_storage,
- NULL, sk_storage);
- if (unlikely(prev_sk_storage)) {
- selem_unlink_map(first_selem);
- err = -EAGAIN;
- goto uncharge;
- /* Note that even first_selem was linked to smap's
- * bucket->list, first_selem can be freed immediately
- * (instead of kfree_rcu) because
- * bpf_sk_storage_map_free() does a
- * synchronize_rcu() before walking the bucket->list.
- * Hence, no one is accessing selem from the
- * bucket->list under rcu_read_lock().
- */
- }
+ curr->sk = sk;
+
+ bpf_selem_link(curr, first_selem);
+ bpf_selem_link_map(smap, first_selem);
+
+ err = bpf_local_storage_publish(first_selem,
+ (struct bpf_local_storage **)&sk->sk_bpf_storage, curr);
+ if (err)
+ goto uncharge;
return 0;
uncharge:
- kfree(sk_storage);
- atomic_sub(sizeof(*sk_storage), &sk->sk_omem_alloc);
+ kfree(curr);
+ atomic_sub(sizeof(*curr), &sk->sk_omem_alloc);
return err;
}
@@ -386,36 +101,31 @@ static int sk_storage_alloc(struct sock *sk,
* Otherwise, it will become a leak (and other memory issues
* during map destruction).
*/
-static struct bpf_sk_storage_data *sk_storage_update(struct sock *sk,
- struct bpf_map *map,
- void *value,
- u64 map_flags)
+static struct bpf_local_storage_data *
+sk_storage_update(void *owner, struct bpf_map *map, void *value, u64 map_flags)
{
- struct bpf_sk_storage_data *old_sdata = NULL;
- struct bpf_sk_storage_elem *selem;
- struct bpf_sk_storage *sk_storage;
- struct bpf_sk_storage_map *smap;
+ struct bpf_local_storage_data *old_sdata = NULL;
+ struct bpf_local_storage_elem *selem;
+ struct bpf_local_storage *local_storage;
+ struct bpf_local_storage_map *smap;
+ struct sock *sk;
int err;
- /* BPF_EXIST and BPF_NOEXIST cannot be both set */
- if (unlikely((map_flags & ~BPF_F_LOCK) > BPF_EXIST) ||
- /* BPF_F_LOCK can only be used in a value with spin_lock */
- unlikely((map_flags & BPF_F_LOCK) && !map_value_has_spin_lock(map)))
- return ERR_PTR(-EINVAL);
+ err = bpf_local_storage_check_update_flags(map, map_flags);
+ if (err)
+ return ERR_PTR(err);
- smap = (struct bpf_sk_storage_map *)map;
- sk_storage = rcu_dereference(sk->sk_bpf_storage);
- if (!sk_storage || hlist_empty(&sk_storage->list)) {
- /* Very first elem for this sk */
- err = check_flags(NULL, map_flags);
- if (err)
- return ERR_PTR(err);
+ sk = owner;
+ local_storage = rcu_dereference(sk->sk_bpf_storage);
+ smap = (struct bpf_local_storage_map *)map;
- selem = selem_alloc(smap, sk, value, true);
+ if (!local_storage || hlist_empty(&local_storage->list)) {
+ /* Very first elem */
+ selem = map->ops->map_selem_alloc(smap, owner, value, !old_sdata);
if (!selem)
return ERR_PTR(-ENOMEM);
- err = sk_storage_alloc(sk, smap, selem);
+ err = map->ops->map_local_storage_alloc(owner, smap, selem);
if (err) {
kfree(selem);
atomic_sub(smap->elem_size, &sk->sk_omem_alloc);
@@ -425,130 +135,42 @@ static struct bpf_sk_storage_data *sk_storage_update(struct sock *sk,
return SDATA(selem);
}
- if ((map_flags & BPF_F_LOCK) && !(map_flags & BPF_NOEXIST)) {
- /* Hoping to find an old_sdata to do inline update
- * such that it can avoid taking the sk_storage->lock
- * and changing the lists.
- */
- old_sdata = __sk_storage_lookup(sk_storage, smap, false);
- err = check_flags(old_sdata, map_flags);
- if (err)
- return ERR_PTR(err);
- if (old_sdata && selem_linked_to_sk(SELEM(old_sdata))) {
- copy_map_value_locked(map, old_sdata->data,
- value, false);
- return old_sdata;
- }
- }
-
- raw_spin_lock_bh(&sk_storage->lock);
-
- /* Recheck sk_storage->list under sk_storage->lock */
- if (unlikely(hlist_empty(&sk_storage->list))) {
- /* A parallel del is happening and sk_storage is going
- * away. It has just been checked before, so very
- * unlikely. Return instead of retry to keep things
- * simple.
- */
- err = -EAGAIN;
- goto unlock_err;
- }
-
- old_sdata = __sk_storage_lookup(sk_storage, smap, false);
- err = check_flags(old_sdata, map_flags);
- if (err)
- goto unlock_err;
-
- if (old_sdata && (map_flags & BPF_F_LOCK)) {
- copy_map_value_locked(map, old_sdata->data, value, false);
- selem = SELEM(old_sdata);
- goto unlock;
- }
-
- /* sk_storage->lock is held. Hence, we are sure
- * we can unlink and uncharge the old_sdata successfully
- * later. Hence, instead of charging the new selem now
- * and then uncharge the old selem later (which may cause
- * a potential but unnecessary charge failure), avoid taking
- * a charge at all here (the "!old_sdata" check) and the
- * old_sdata will not be uncharged later during __selem_unlink_sk().
- */
- selem = selem_alloc(smap, sk, value, !old_sdata);
- if (!selem) {
- err = -ENOMEM;
- goto unlock_err;
- }
-
- /* First, link the new selem to the map */
- selem_link_map(smap, selem);
-
- /* Second, link (and publish) the new selem to sk_storage */
- __selem_link_sk(sk_storage, selem);
+ return bpf_local_storage_update(owner, map, local_storage, value,
+ map_flags);
+}
- /* Third, remove old selem, SELEM(old_sdata) */
- if (old_sdata) {
- selem_unlink_map(SELEM(old_sdata));
- __selem_unlink_sk(sk_storage, SELEM(old_sdata), false);
- }
+static struct bpf_local_storage_data *
+sk_storage_lookup(struct sock *sk, struct bpf_map *map, bool cacheit_lockit)
+{
+ struct bpf_local_storage *sk_storage;
+ struct bpf_local_storage_map *smap;
-unlock:
- raw_spin_unlock_bh(&sk_storage->lock);
- return SDATA(selem);
+ sk_storage = rcu_dereference(sk->sk_bpf_storage);
+ if (!sk_storage)
+ return NULL;
-unlock_err:
- raw_spin_unlock_bh(&sk_storage->lock);
- return ERR_PTR(err);
+ smap = (struct bpf_local_storage_map *)map;
+ return bpf_local_storage_lookup(sk_storage, smap, cacheit_lockit);
}
static int sk_storage_delete(struct sock *sk, struct bpf_map *map)
{
- struct bpf_sk_storage_data *sdata;
+ struct bpf_local_storage_data *sdata;
sdata = sk_storage_lookup(sk, map, false);
if (!sdata)
return -ENOENT;
- selem_unlink(SELEM(sdata));
+ bpf_selem_unlink_map_elem(SELEM(sdata));
return 0;
}
-static u16 cache_idx_get(void)
-{
- u64 min_usage = U64_MAX;
- u16 i, res = 0;
-
- spin_lock(&cache_idx_lock);
-
- for (i = 0; i < BPF_SK_STORAGE_CACHE_SIZE; i++) {
- if (cache_idx_usage_counts[i] < min_usage) {
- min_usage = cache_idx_usage_counts[i];
- res = i;
-
- /* Found a free cache_idx */
- if (!min_usage)
- break;
- }
- }
- cache_idx_usage_counts[res]++;
-
- spin_unlock(&cache_idx_lock);
-
- return res;
-}
-
-static void cache_idx_free(u16 idx)
-{
- spin_lock(&cache_idx_lock);
- cache_idx_usage_counts[idx]--;
- spin_unlock(&cache_idx_lock);
-}
-
/* Called by __sk_destruct() & bpf_sk_storage_clone() */
void bpf_sk_storage_free(struct sock *sk)
{
- struct bpf_sk_storage_elem *selem;
- struct bpf_sk_storage *sk_storage;
+ struct bpf_local_storage_elem *selem;
+ struct bpf_local_storage *sk_storage;
bool free_sk_storage = false;
struct hlist_node *n;
@@ -562,9 +184,9 @@ void bpf_sk_storage_free(struct sock *sk)
/* Netiher the bpf_prog nor the bpf-map's syscall
* could be modifying the sk_storage->list now.
* Thus, no elem can be added-to or deleted-from the
- * sk_storage->list by the bpf_prog or by the bpf-map's syscall.
+ * local_storage->list by the bpf_prog or by the bpf-map's syscall.
*
- * It is racing with bpf_sk_storage_map_free() alone
+ * It is racing with bpf_local_storage_map_free() alone
* when unlinking elem from the sk_storage->list and
* the map's bucket->list.
*/
@@ -573,8 +195,8 @@ void bpf_sk_storage_free(struct sock *sk)
/* Always unlink from map before unlinking from
* sk_storage.
*/
- selem_unlink_map(selem);
- free_sk_storage = __selem_unlink_sk(sk_storage, selem, true);
+ bpf_selem_unlink_map(selem);
+ free_sk_storage = bpf_selem_unlink(sk_storage, selem, true);
}
raw_spin_unlock_bh(&sk_storage->lock);
rcu_read_unlock();
@@ -583,163 +205,11 @@ void bpf_sk_storage_free(struct sock *sk)
kfree_rcu(sk_storage, rcu);
}
-static void bpf_sk_storage_map_free(struct bpf_map *map)
+static void *bpf_sk_storage_lookup_elem(struct bpf_map *map, void *key)
{
- struct bpf_sk_storage_elem *selem;
- struct bpf_sk_storage_map *smap;
- struct bucket *b;
- unsigned int i;
-
- smap = (struct bpf_sk_storage_map *)map;
-
- cache_idx_free(smap->cache_idx);
-
- /* Note that this map might be concurrently cloned from
- * bpf_sk_storage_clone. Wait for any existing bpf_sk_storage_clone
- * RCU read section to finish before proceeding. New RCU
- * read sections should be prevented via bpf_map_inc_not_zero.
- */
- synchronize_rcu();
-
- /* bpf prog and the userspace can no longer access this map
- * now. No new selem (of this map) can be added
- * to the sk->sk_bpf_storage or to the map bucket's list.
- *
- * The elem of this map can be cleaned up here
- * or
- * by bpf_sk_storage_free() during __sk_destruct().
- */
- for (i = 0; i < (1U << smap->bucket_log); i++) {
- b = &smap->buckets[i];
-
- rcu_read_lock();
- /* No one is adding to b->list now */
- while ((selem = hlist_entry_safe(rcu_dereference_raw(hlist_first_rcu(&b->list)),
- struct bpf_sk_storage_elem,
- map_node))) {
- selem_unlink(selem);
- cond_resched_rcu();
- }
- rcu_read_unlock();
- }
-
- /* bpf_sk_storage_free() may still need to access the map.
- * e.g. bpf_sk_storage_free() has unlinked selem from the map
- * which then made the above while((selem = ...)) loop
- * exited immediately.
- *
- * However, the bpf_sk_storage_free() still needs to access
- * the smap->elem_size to do the uncharging in
- * __selem_unlink_sk().
- *
- * Hence, wait another rcu grace period for the
- * bpf_sk_storage_free() to finish.
- */
- synchronize_rcu();
-
- kvfree(smap->buckets);
- kfree(map);
-}
-
-/* U16_MAX is much more than enough for sk local storage
- * considering a tcp_sock is ~2k.
- */
-#define MAX_VALUE_SIZE \
- min_t(u32, \
- (KMALLOC_MAX_SIZE - MAX_BPF_STACK - sizeof(struct bpf_sk_storage_elem)), \
- (U16_MAX - sizeof(struct bpf_sk_storage_elem)))
-
-static int bpf_sk_storage_map_alloc_check(union bpf_attr *attr)
-{
- if (attr->map_flags & ~SK_STORAGE_CREATE_FLAG_MASK ||
- !(attr->map_flags & BPF_F_NO_PREALLOC) ||
- attr->max_entries ||
- attr->key_size != sizeof(int) || !attr->value_size ||
- /* Enforce BTF for userspace sk dumping */
- !attr->btf_key_type_id || !attr->btf_value_type_id)
- return -EINVAL;
-
- if (!bpf_capable())
- return -EPERM;
-
- if (attr->value_size > MAX_VALUE_SIZE)
- return -E2BIG;
-
- return 0;
-}
-
-static struct bpf_map *bpf_sk_storage_map_alloc(union bpf_attr *attr)
-{
- struct bpf_sk_storage_map *smap;
- unsigned int i;
- u32 nbuckets;
- u64 cost;
- int ret;
-
- smap = kzalloc(sizeof(*smap), GFP_USER | __GFP_NOWARN);
- if (!smap)
- return ERR_PTR(-ENOMEM);
- bpf_map_init_from_attr(&smap->map, attr);
-
- nbuckets = roundup_pow_of_two(num_possible_cpus());
- /* Use at least 2 buckets, select_bucket() is undefined behavior with 1 bucket */
- nbuckets = max_t(u32, 2, nbuckets);
- smap->bucket_log = ilog2(nbuckets);
- cost = sizeof(*smap->buckets) * nbuckets + sizeof(*smap);
-
- ret = bpf_map_charge_init(&smap->map.memory, cost);
- if (ret < 0) {
- kfree(smap);
- return ERR_PTR(ret);
- }
-
- smap->buckets = kvcalloc(sizeof(*smap->buckets), nbuckets,
- GFP_USER | __GFP_NOWARN);
- if (!smap->buckets) {
- bpf_map_charge_finish(&smap->map.memory);
- kfree(smap);
- return ERR_PTR(-ENOMEM);
- }
-
- for (i = 0; i < nbuckets; i++) {
- INIT_HLIST_HEAD(&smap->buckets[i].list);
- raw_spin_lock_init(&smap->buckets[i].lock);
- }
-
- smap->elem_size = sizeof(struct bpf_sk_storage_elem) + attr->value_size;
- smap->cache_idx = cache_idx_get();
-
- return &smap->map;
-}
-
-static int notsupp_get_next_key(struct bpf_map *map, void *key,
- void *next_key)
-{
- return -ENOTSUPP;
-}
-
-static int bpf_sk_storage_map_check_btf(const struct bpf_map *map,
- const struct btf *btf,
- const struct btf_type *key_type,
- const struct btf_type *value_type)
-{
- u32 int_data;
-
- if (BTF_INFO_KIND(key_type->info) != BTF_KIND_INT)
- return -EINVAL;
-
- int_data = *(u32 *)(key_type + 1);
- if (BTF_INT_BITS(int_data) != 32 || BTF_INT_OFFSET(int_data))
- return -EINVAL;
-
- return 0;
-}
-
-static void *bpf_fd_sk_storage_lookup_elem(struct bpf_map *map, void *key)
-{
- struct bpf_sk_storage_data *sdata;
+ struct bpf_local_storage_data *sdata;
struct socket *sock;
- int fd, err;
+ int fd, err = -EINVAL;
fd = *(int *)key;
sock = sockfd_lookup(fd, &err);
@@ -752,17 +222,18 @@ static void *bpf_fd_sk_storage_lookup_elem(struct bpf_map *map, void *key)
return ERR_PTR(err);
}
-static int bpf_fd_sk_storage_update_elem(struct bpf_map *map, void *key,
- void *value, u64 map_flags)
+static int bpf_sk_storage_update_elem(struct bpf_map *map, void *key,
+ void *value, u64 map_flags)
{
- struct bpf_sk_storage_data *sdata;
+ struct bpf_local_storage_data *sdata;
struct socket *sock;
int fd, err;
fd = *(int *)key;
sock = sockfd_lookup(fd, &err);
if (sock) {
- sdata = sk_storage_update(sock->sk, map, value, map_flags);
+ sdata = map->ops->map_local_storage_update(sock->sk, map, value,
+ map_flags);
sockfd_put(sock);
return PTR_ERR_OR_ZERO(sdata);
}
@@ -770,7 +241,7 @@ static int bpf_fd_sk_storage_update_elem(struct bpf_map *map, void *key,
return err;
}
-static int bpf_fd_sk_storage_delete_elem(struct bpf_map *map, void *key)
+static int bpf_sk_storage_delete_elem(struct bpf_map *map, void *key)
{
struct socket *sock;
int fd, err;
@@ -780,20 +251,19 @@ static int bpf_fd_sk_storage_delete_elem(struct bpf_map *map, void *key)
if (sock) {
err = sk_storage_delete(sock->sk, map);
sockfd_put(sock);
- return err;
}
return err;
}
-static struct bpf_sk_storage_elem *
+static struct bpf_local_storage_elem *
bpf_sk_storage_clone_elem(struct sock *newsk,
- struct bpf_sk_storage_map *smap,
- struct bpf_sk_storage_elem *selem)
+ struct bpf_local_storage_map *smap,
+ struct bpf_local_storage_elem *selem)
{
- struct bpf_sk_storage_elem *copy_selem;
+ struct bpf_local_storage_elem *copy_selem;
- copy_selem = selem_alloc(smap, newsk, NULL, true);
+ copy_selem = sk_selem_alloc(smap, newsk, NULL, true);
if (!copy_selem)
return NULL;
@@ -809,9 +279,9 @@ bpf_sk_storage_clone_elem(struct sock *newsk,
int bpf_sk_storage_clone(const struct sock *sk, struct sock *newsk)
{
- struct bpf_sk_storage *new_sk_storage = NULL;
- struct bpf_sk_storage *sk_storage;
- struct bpf_sk_storage_elem *selem;
+ struct bpf_local_storage *new_sk_storage = NULL;
+ struct bpf_local_storage *sk_storage;
+ struct bpf_local_storage_elem *selem;
int ret = 0;
RCU_INIT_POINTER(newsk->sk_bpf_storage, NULL);
@@ -823,8 +293,8 @@ int bpf_sk_storage_clone(const struct sock *sk, struct sock *newsk)
goto out;
hlist_for_each_entry_rcu(selem, &sk_storage->list, snode) {
- struct bpf_sk_storage_elem *copy_selem;
- struct bpf_sk_storage_map *smap;
+ struct bpf_local_storage_elem *copy_selem;
+ struct bpf_local_storage_map *smap;
struct bpf_map *map;
smap = rcu_dereference(SDATA(selem)->smap);
@@ -832,7 +302,7 @@ int bpf_sk_storage_clone(const struct sock *sk, struct sock *newsk)
continue;
/* Note that for lockless listeners adding new element
- * here can race with cleanup in bpf_sk_storage_map_free.
+ * here can race with cleanup in bpf_local_storage_map_free.
* Try to grab map refcnt to make sure that it's still
* alive and prevent concurrent removal.
*/
@@ -848,8 +318,8 @@ int bpf_sk_storage_clone(const struct sock *sk, struct sock *newsk)
}
if (new_sk_storage) {
- selem_link_map(smap, copy_selem);
- __selem_link_sk(new_sk_storage, copy_selem);
+ bpf_selem_link_map(smap, copy_selem);
+ bpf_selem_link(new_sk_storage, copy_selem);
} else {
ret = sk_storage_alloc(newsk, smap, copy_selem);
if (ret) {
@@ -860,7 +330,8 @@ int bpf_sk_storage_clone(const struct sock *sk, struct sock *newsk)
goto out;
}
- new_sk_storage = rcu_dereference(copy_selem->sk_storage);
+ new_sk_storage =
+ rcu_dereference(copy_selem->local_storage);
}
bpf_map_put(map);
}
@@ -869,7 +340,7 @@ int bpf_sk_storage_clone(const struct sock *sk, struct sock *newsk)
rcu_read_unlock();
/* In case of an error, don't free anything explicitly here, the
- * caller is responsible to call bpf_sk_storage_free.
+ * caller is responsible to call bpf_local_storage_free.
*/
return ret;
@@ -878,7 +349,7 @@ int bpf_sk_storage_clone(const struct sock *sk, struct sock *newsk)
BPF_CALL_4(bpf_sk_storage_get, struct bpf_map *, map, struct sock *, sk,
void *, value, u64, flags)
{
- struct bpf_sk_storage_data *sdata;
+ struct bpf_local_storage_data *sdata;
if (flags > BPF_SK_STORAGE_GET_F_CREATE)
return (unsigned long)NULL;
@@ -887,7 +358,7 @@ BPF_CALL_4(bpf_sk_storage_get, struct bpf_map *, map, struct sock *, sk,
if (sdata)
return (unsigned long)sdata->data;
- if (flags == BPF_SK_STORAGE_GET_F_CREATE &&
+ if (flags == BPF_LOCAL_STORAGE_GET_F_CREATE &&
/* Cannot add new elem to a going away sk.
* Otherwise, the new elem may become a leak
* (and also other memory issues during map
@@ -919,18 +390,51 @@ BPF_CALL_2(bpf_sk_storage_delete, struct bpf_map *, map, struct sock *, sk)
return -ENOENT;
}
+static int notsupp_get_next_key(struct bpf_map *map, void *key,
+ void *next_key)
+{
+ return -ENOTSUPP;
+}
+
+DEFINE_BPF_STORAGE_CACHE(sk);
+
+struct bpf_map *sk_storage_map_alloc(union bpf_attr *attr)
+{
+ struct bpf_local_storage_map *smap;
+
+ smap = bpf_local_storage_map_alloc(attr);
+ if (IS_ERR(smap))
+ return ERR_CAST(smap);
+
+ smap->cache_idx = cache_idx_get_sk();
+ return &smap->map;
+}
+
+void sk_storage_map_free(struct bpf_map *map)
+{
+ struct bpf_local_storage_map *smap;
+
+ smap = (struct bpf_local_storage_map *)map;
+ cache_idx_free_sk(smap->cache_idx);
+ bpf_local_storage_map_free(smap);
+}
+
static int sk_storage_map_btf_id;
const struct bpf_map_ops sk_storage_map_ops = {
- .map_alloc_check = bpf_sk_storage_map_alloc_check,
- .map_alloc = bpf_sk_storage_map_alloc,
- .map_free = bpf_sk_storage_map_free,
+ .map_alloc_check = bpf_local_storage_map_alloc_check,
+ .map_alloc = sk_storage_map_alloc,
+ .map_free = sk_storage_map_free,
.map_get_next_key = notsupp_get_next_key,
- .map_lookup_elem = bpf_fd_sk_storage_lookup_elem,
- .map_update_elem = bpf_fd_sk_storage_update_elem,
- .map_delete_elem = bpf_fd_sk_storage_delete_elem,
- .map_check_btf = bpf_sk_storage_map_check_btf,
- .map_btf_name = "bpf_sk_storage_map",
+ .map_lookup_elem = bpf_sk_storage_lookup_elem,
+ .map_update_elem = bpf_sk_storage_update_elem,
+ .map_delete_elem = bpf_sk_storage_delete_elem,
+ .map_check_btf = bpf_local_storage_map_check_btf,
+ .map_btf_name = "bpf_local_storage_map",
.map_btf_id = &sk_storage_map_btf_id,
+ .map_local_storage_alloc = sk_storage_alloc,
+ .map_selem_alloc = sk_selem_alloc,
+ .map_local_storage_update = sk_storage_update,
+ .map_local_storage_unlink = unlink_sk_storage,
};
const struct bpf_func_proto bpf_sk_storage_get_proto = {
@@ -1011,7 +515,7 @@ bpf_sk_storage_diag_alloc(const struct nlattr *nla_stgs)
u32 nr_maps = 0;
int rem, err;
- /* bpf_sk_storage_map is currently limited to CAP_SYS_ADMIN as
+ /* bpf_local_storage_map is currently limited to CAP_SYS_ADMIN as
* the map_alloc_check() side also does.
*/
if (!bpf_capable())
@@ -1061,13 +565,13 @@ bpf_sk_storage_diag_alloc(const struct nlattr *nla_stgs)
}
EXPORT_SYMBOL_GPL(bpf_sk_storage_diag_alloc);
-static int diag_get(struct bpf_sk_storage_data *sdata, struct sk_buff *skb)
+static int diag_get(struct bpf_local_storage_data *sdata, struct sk_buff *skb)
{
struct nlattr *nla_stg, *nla_value;
- struct bpf_sk_storage_map *smap;
+ struct bpf_local_storage_map *smap;
/* It cannot exceed max nlattr's payload */
- BUILD_BUG_ON(U16_MAX - NLA_HDRLEN < MAX_VALUE_SIZE);
+ BUILD_BUG_ON(U16_MAX - NLA_HDRLEN < BPF_LOCAL_STORAGE_MAX_VALUE_SIZE);
nla_stg = nla_nest_start(skb, SK_DIAG_BPF_STORAGE);
if (!nla_stg)
@@ -1103,9 +607,9 @@ static int bpf_sk_storage_diag_put_all(struct sock *sk, struct sk_buff *skb,
{
/* stg_array_type (e.g. INET_DIAG_BPF_SK_STORAGES) */
unsigned int diag_size = nla_total_size(0);
- struct bpf_sk_storage *sk_storage;
- struct bpf_sk_storage_elem *selem;
- struct bpf_sk_storage_map *smap;
+ struct bpf_local_storage *sk_storage;
+ struct bpf_local_storage_elem *selem;
+ struct bpf_local_storage_map *smap;
struct nlattr *nla_stgs;
unsigned int saved_len;
int err = 0;
@@ -1158,8 +662,8 @@ int bpf_sk_storage_diag_put(struct bpf_sk_storage_diag *diag,
{
/* stg_array_type (e.g. INET_DIAG_BPF_SK_STORAGES) */
unsigned int diag_size = nla_total_size(0);
- struct bpf_sk_storage *sk_storage;
- struct bpf_sk_storage_data *sdata;
+ struct bpf_local_storage *sk_storage;
+ struct bpf_local_storage_data *sdata;
struct nlattr *nla_stgs;
unsigned int saved_len;
int err = 0;
@@ -1186,8 +690,8 @@ int bpf_sk_storage_diag_put(struct bpf_sk_storage_diag *diag,
saved_len = skb->len;
for (i = 0; i < diag->nr_maps; i++) {
- sdata = __sk_storage_lookup(sk_storage,
- (struct bpf_sk_storage_map *)diag->maps[i],
+ sdata = bpf_local_storage_lookup(sk_storage,
+ (struct bpf_local_storage_map *)diag->maps[i],
false);
if (!sdata)
diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
index 548a749aebb3..1f3e831c4813 100644
--- a/tools/include/uapi/linux/bpf.h
+++ b/tools/include/uapi/linux/bpf.h
@@ -2802,10 +2802,10 @@ union bpf_attr {
* "type". The bpf-local-storage "type" (i.e. the *map*) is
* searched against all bpf-local-storages residing at *sk*.
*
- * An optional *flags* (**BPF_SK_STORAGE_GET_F_CREATE**) can be
+ * An optional *flags* (**BPF_LOCAL_STORAGE_GET_F_CREATE**) can be
* used such that a new bpf-local-storage will be
* created if one does not exist. *value* can be used
- * together with **BPF_SK_STORAGE_GET_F_CREATE** to specify
+ * together with **BPF_LOCAL_STORAGE_GET_F_CREATE** to specify
* the initial value of a bpf-local-storage. If *value* is
* **NULL**, the new bpf-local-storage will be zero initialized.
* Return
@@ -3572,9 +3572,13 @@ enum {
BPF_F_SYSCTL_BASE_NAME = (1ULL << 0),
};
-/* BPF_FUNC_sk_storage_get flags */
+/* BPF_FUNC_<local>_storage_get flags */
enum {
- BPF_SK_STORAGE_GET_F_CREATE = (1ULL << 0),
+ BPF_LOCAL_STORAGE_GET_F_CREATE = (1ULL << 0),
+ /* BPF_SK_STORAGE_GET_F_CREATE is only kept for backward compatibility
+ * and BPF_LOCAL_STORAGE_GET_F_CREATE must be used instead.
+ */
+ BPF_SK_STORAGE_GET_F_CREATE = BPF_LOCAL_STORAGE_GET_F_CREATE,
};
/* BPF_FUNC_read_branch_records flags. */
--
2.27.0.389.gc38d7665816-goog
^ permalink raw reply related
* [PATCH bpf-next v3 2/4] bpf: Implement bpf_local_storage for inodes
From: KP Singh @ 2020-07-09 0:56 UTC (permalink / raw)
To: linux-kernel, bpf, linux-security-module
Cc: Alexei Starovoitov, Daniel Borkmann, Paul Turner, Jann Horn,
Florent Revest
In-Reply-To: <20200709005654.3324272-1-kpsingh@chromium.org>
From: KP Singh <kpsingh@google.com>
Similar to bpf_local_storage for sockets, add local storage for inodes.
The life-cycle of storage is managed with the life-cycle of the inode.
i.e. the storage is destroyed along with the owning inode.
The BPF LSM allocates an __rcu pointer to the bpf_local_storage in the
security blob which are now stackable and can co-exist with other LSMs.
Signed-off-by: KP Singh <kpsingh@google.com>
---
include/linux/bpf_local_storage.h | 15 +
include/linux/bpf_lsm.h | 21 ++
include/linux/bpf_types.h | 3 +
include/uapi/linux/bpf.h | 38 ++
kernel/bpf/Makefile | 1 +
kernel/bpf/bpf_inode_storage.c | 333 ++++++++++++++++++
kernel/bpf/syscall.c | 3 +-
kernel/bpf/verifier.c | 10 +
security/bpf/hooks.c | 7 +
.../bpf/bpftool/Documentation/bpftool-map.rst | 2 +-
tools/bpf/bpftool/bash-completion/bpftool | 3 +-
tools/bpf/bpftool/map.c | 3 +-
tools/include/uapi/linux/bpf.h | 38 ++
tools/lib/bpf/libbpf_probes.c | 5 +-
14 files changed, 476 insertions(+), 6 deletions(-)
create mode 100644 kernel/bpf/bpf_inode_storage.c
diff --git a/include/linux/bpf_local_storage.h b/include/linux/bpf_local_storage.h
index 605b81f2f806..63cc7b93ae80 100644
--- a/include/linux/bpf_local_storage.h
+++ b/include/linux/bpf_local_storage.h
@@ -113,6 +113,8 @@ static void cache_idx_free_##type(u16 idx) \
sizeof(struct bpf_local_storage_elem)), \
(U16_MAX - sizeof(struct bpf_local_storage_elem)))
+struct inode;
+
struct bpf_local_storage {
struct bpf_local_storage_data __rcu *cache[BPF_STORAGE_CACHE_SIZE];
struct hlist_head list; /* List of bpf_local_storage_elem */
@@ -121,6 +123,9 @@ struct bpf_local_storage {
*/
union {
struct sock *sk;
+#ifdef CONFIG_BPF_LSM
+ struct inode *inode;
+#endif
};
struct rcu_head rcu;
raw_spinlock_t lock; /* Protect adding/removing from the "list" */
@@ -172,4 +177,14 @@ struct bpf_local_storage_data *
bpf_local_storage_update(void *owner, struct bpf_map *map,
struct bpf_local_storage *local_storage, void *value,
u64 map_flags);
+
+#ifdef CONFIG_BPF_LSM
+extern const struct bpf_func_proto bpf_inode_storage_get_proto;
+extern const struct bpf_func_proto bpf_inode_storage_delete_proto;
+void bpf_inode_storage_free(struct inode *inode);
+#else
+static inline void bpf_inode_storage_free(struct inode *inode)
+{
+}
+#endif
#endif /* _BPF_LOCAL_STORAGE_H */
diff --git a/include/linux/bpf_lsm.h b/include/linux/bpf_lsm.h
index af74712af585..d0683ada1e49 100644
--- a/include/linux/bpf_lsm.h
+++ b/include/linux/bpf_lsm.h
@@ -17,9 +17,24 @@
#include <linux/lsm_hook_defs.h>
#undef LSM_HOOK
+struct bpf_storage_blob {
+ struct bpf_local_storage __rcu *storage;
+};
+
+extern struct lsm_blob_sizes bpf_lsm_blob_sizes;
+
int bpf_lsm_verify_prog(struct bpf_verifier_log *vlog,
const struct bpf_prog *prog);
+static inline struct bpf_storage_blob *bpf_inode(
+ const struct inode *inode)
+{
+ if (unlikely(!inode->i_security))
+ return NULL;
+
+ return inode->i_security + bpf_lsm_blob_sizes.lbs_inode;
+}
+
#else /* !CONFIG_BPF_LSM */
static inline int bpf_lsm_verify_prog(struct bpf_verifier_log *vlog,
@@ -28,6 +43,12 @@ static inline int bpf_lsm_verify_prog(struct bpf_verifier_log *vlog,
return -EOPNOTSUPP;
}
+static inline struct bpf_storage_blob *bpf_inode(
+ const struct inode *inode)
+{
+ return NULL;
+}
+
#endif /* CONFIG_BPF_LSM */
#endif /* _LINUX_BPF_LSM_H */
diff --git a/include/linux/bpf_types.h b/include/linux/bpf_types.h
index a18ae82a298a..2950576de4ae 100644
--- a/include/linux/bpf_types.h
+++ b/include/linux/bpf_types.h
@@ -105,6 +105,9 @@ BPF_MAP_TYPE(BPF_MAP_TYPE_SK_STORAGE, sk_storage_map_ops)
BPF_MAP_TYPE(BPF_MAP_TYPE_SOCKMAP, sock_map_ops)
BPF_MAP_TYPE(BPF_MAP_TYPE_SOCKHASH, sock_hash_ops)
#endif
+#ifdef CONFIG_BPF_LSM
+BPF_MAP_TYPE(BPF_MAP_TYPE_INODE_STORAGE, inode_storage_map_ops)
+#endif
BPF_MAP_TYPE(BPF_MAP_TYPE_CPUMAP, cpu_map_ops)
#if defined(CONFIG_XDP_SOCKETS)
BPF_MAP_TYPE(BPF_MAP_TYPE_XSKMAP, xsk_map_ops)
diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index 1f3e831c4813..42fc442f4586 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -148,6 +148,7 @@ enum bpf_map_type {
BPF_MAP_TYPE_DEVMAP_HASH,
BPF_MAP_TYPE_STRUCT_OPS,
BPF_MAP_TYPE_RINGBUF,
+ BPF_MAP_TYPE_INODE_STORAGE,
};
/* Note that tracing related programs such as
@@ -3319,6 +3320,41 @@ union bpf_attr {
* A non-negative value equal to or less than *size* on success,
* or a negative error in case of failure.
*
+ * void *bpf_inode_storage_get(struct bpf_map *map, void *inode, void *value, u64 flags)
+ * Description
+ * Get a bpf_local_storage from an *inode*.
+ *
+ * Logically, it could be thought of as getting the value from
+ * a *map* with *inode* as the **key**. From this
+ * perspective, the usage is not much different from
+ * **bpf_map_lookup_elem**\ (*map*, **&**\ *inode*) except this
+ * helper enforces the key must be an inode and the map must also
+ * be a **BPF_MAP_TYPE_INODE_STORAGE**.
+ *
+ * Underneath, the value is stored locally at *inode* instead of
+ * the *map*. The *map* is used as the bpf-local-storage
+ * "type". The bpf-local-storage "type" (i.e. the *map*) is
+ * searched against all bpf_local_storage residing at *inode*.
+ *
+ * An optional *flags* (**BPF_LOCAL_STORAGE_GET_F_CREATE**) can be
+ * used such that a new bpf_local_storage will be
+ * created if one does not exist. *value* can be used
+ * together with **BPF_LOCAL_STORAGE_GET_F_CREATE** to specify
+ * the initial value of a bpf_local_storage. If *value* is
+ * **NULL**, the new bpf_local_storage will be zero initialized.
+ * Return
+ * A bpf_local_storage pointer is returned on success.
+ *
+ * **NULL** if not found or there was an error in adding
+ * a new bpf_local_storage.
+ *
+ * int bpf_inode_storage_delete(struct bpf_map *map, void *inode)
+ * Description
+ * Delete a bpf_local_storage from an *inode*.
+ * Return
+ * 0 on success.
+ *
+ * **-ENOENT** if the bpf_local_storage cannot be found.
*/
#define __BPF_FUNC_MAPPER(FN) \
FN(unspec), \
@@ -3463,6 +3499,8 @@ union bpf_attr {
FN(skc_to_tcp_request_sock), \
FN(skc_to_udp6_sock), \
FN(get_task_stack), \
+ FN(inode_storage_get), \
+ FN(inode_storage_delete), \
/* */
/* integer value in 'imm' field of BPF_CALL instruction selects which helper
diff --git a/kernel/bpf/Makefile b/kernel/bpf/Makefile
index 0acb8f8a6042..0ea9fd15977c 100644
--- a/kernel/bpf/Makefile
+++ b/kernel/bpf/Makefile
@@ -5,6 +5,7 @@ CFLAGS_core.o += $(call cc-disable-warning, override-init)
obj-$(CONFIG_BPF_SYSCALL) += syscall.o verifier.o inode.o helpers.o tnum.o bpf_iter.o map_iter.o task_iter.o
obj-$(CONFIG_BPF_SYSCALL) += hashtab.o arraymap.o percpu_freelist.o bpf_lru_list.o lpm_trie.o map_in_map.o
obj-$(CONFIG_BPF_SYSCALL) += local_storage.o queue_stack_maps.o ringbuf.o
+obj-${CONFIG_BPF_LSM} += bpf_inode_storage.o
obj-$(CONFIG_BPF_SYSCALL) += disasm.o
obj-$(CONFIG_BPF_JIT) += trampoline.o
obj-$(CONFIG_BPF_SYSCALL) += btf.o
diff --git a/kernel/bpf/bpf_inode_storage.c b/kernel/bpf/bpf_inode_storage.c
new file mode 100644
index 000000000000..d9c3cadc31a5
--- /dev/null
+++ b/kernel/bpf/bpf_inode_storage.c
@@ -0,0 +1,333 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2019 Facebook
+ * Copyright 2020 Google LLC.
+ */
+
+#include <linux/rculist.h>
+#include <linux/list.h>
+#include <linux/hash.h>
+#include <linux/types.h>
+#include <linux/spinlock.h>
+#include <linux/bpf.h>
+#include <linux/bpf_local_storage.h>
+#include <net/sock.h>
+#include <uapi/linux/sock_diag.h>
+#include <uapi/linux/btf.h>
+#include <linux/bpf_lsm.h>
+
+static struct bpf_local_storage_elem *
+inode_selem_alloc(struct bpf_local_storage_map *smap, void *owner,
+ void *value, bool charge_omem)
+{
+ return bpf_selem_alloc(smap, value);
+}
+
+static void unlink_inode_storage(struct bpf_local_storage *local_storage,
+ bool uncharge_omem)
+{
+ struct bpf_storage_blob *bsb;
+ struct inode *inode;
+
+ inode = local_storage->inode;
+ bsb = bpf_inode(inode);
+ if (!bsb)
+ return;
+ RCU_INIT_POINTER(bsb->storage, NULL);
+ local_storage->inode = NULL;
+}
+
+static struct bpf_local_storage_data *inode_storage_lookup(struct inode *inode,
+ struct bpf_map *map,
+ bool cacheit_lockit)
+{
+ struct bpf_local_storage *inode_storage;
+ struct bpf_local_storage_map *smap;
+ struct bpf_storage_blob *bsb;
+
+ bsb = bpf_inode(inode);
+ if (!bsb)
+ return ERR_PTR(-ENOENT);
+
+ inode_storage = rcu_dereference(bsb->storage);
+ if (!inode_storage)
+ return NULL;
+
+ smap = (struct bpf_local_storage_map *)map;
+ return bpf_local_storage_lookup(inode_storage, smap, cacheit_lockit);
+}
+
+static int inode_storage_alloc(void *owner, struct bpf_local_storage_map *smap,
+ struct bpf_local_storage_elem *first_selem)
+{
+ struct bpf_local_storage *curr;
+ struct bpf_storage_blob *bsb;
+ struct inode *inode = owner;
+ int err;
+
+ bsb = bpf_inode(inode);
+ if (!bsb)
+ return -EINVAL;
+
+ curr = bpf_local_storage_alloc(smap);
+ if (!curr)
+ return -ENOMEM;
+
+ curr->inode = inode;
+
+ bpf_selem_link(curr, first_selem);
+ bpf_selem_link_map(smap, first_selem);
+
+ err = bpf_local_storage_publish(first_selem,
+ (struct bpf_local_storage **)&bsb->storage, curr);
+ if (err) {
+ kfree(curr);
+ return err;
+ }
+
+ return 0;
+}
+
+static struct bpf_local_storage_data *inode_storage_update(void *owner,
+ struct bpf_map *map,
+ void *value,
+ u64 map_flags)
+{
+ struct bpf_local_storage_data *old_sdata = NULL;
+ struct bpf_local_storage_elem *selem;
+ struct bpf_local_storage *local_storage;
+ struct bpf_local_storage_map *smap;
+ struct bpf_storage_blob *bsb;
+ struct inode *inode;
+ int err;
+
+ err = bpf_local_storage_check_update_flags(map, map_flags);
+ if (err)
+ return ERR_PTR(err);
+
+ inode = owner;
+ bsb = bpf_inode(inode);
+ local_storage = rcu_dereference(bsb->storage);
+ smap = (struct bpf_local_storage_map *)map;
+
+ if (!local_storage || hlist_empty(&local_storage->list)) {
+ /* Very first elem */
+ selem = map->ops->map_selem_alloc(smap, owner, value, !old_sdata);
+ if (!selem)
+ return ERR_PTR(-ENOMEM);
+
+ err = map->ops->map_local_storage_alloc(owner, smap, selem);
+ if (err) {
+ kfree(selem);
+ return ERR_PTR(err);
+ }
+
+ return SDATA(selem);
+ }
+
+ return bpf_local_storage_update(owner, map, local_storage, value,
+ map_flags);
+}
+
+
+void bpf_inode_storage_free(struct inode *inode)
+{
+ struct bpf_local_storage_elem *selem;
+ struct bpf_local_storage *local_storage;
+ bool free_inode_storage = false;
+ struct bpf_storage_blob *bsb;
+ struct hlist_node *n;
+
+ bsb = bpf_inode(inode);
+ if (!bsb)
+ return;
+
+ rcu_read_lock();
+
+ local_storage = rcu_dereference(bsb->storage);
+ if (!local_storage) {
+ rcu_read_unlock();
+ return;
+ }
+
+ /* Netiher the bpf_prog nor the bpf-map's syscall
+ * could be modifying the local_storage->list now.
+ * Thus, no elem can be added-to or deleted-from the
+ * local_storage->list by the bpf_prog or by the bpf-map's syscall.
+ *
+ * It is racing with bpf_local_storage_map_free() alone
+ * when unlinking elem from the local_storage->list and
+ * the map's bucket->list.
+ */
+ raw_spin_lock_bh(&local_storage->lock);
+ hlist_for_each_entry_safe(selem, n, &local_storage->list, snode) {
+ /* Always unlink from map before unlinking from
+ * local_storage.
+ */
+ bpf_selem_unlink_map(selem);
+ free_inode_storage =
+ bpf_selem_unlink(local_storage, selem, false);
+ }
+ raw_spin_unlock_bh(&local_storage->lock);
+ rcu_read_unlock();
+
+ /* free_inoode_storage should always be true as long as
+ * local_storage->list was non-empty.
+ */
+ if (free_inode_storage)
+ kfree_rcu(local_storage, rcu);
+}
+
+
+static void *bpf_inode_storage_lookup_elem(struct bpf_map *map, void *key)
+{
+ struct bpf_local_storage_data *sdata;
+ struct inode *inode;
+ int err = -EINVAL;
+
+ if (key) {
+ inode = *(struct inode **)(key);
+ sdata = inode_storage_lookup(inode, map, true);
+ return sdata ? sdata->data : NULL;
+ }
+
+ return ERR_PTR(err);
+}
+
+static int bpf_inode_storage_update_elem(struct bpf_map *map, void *key,
+ void *value, u64 map_flags)
+{
+ struct bpf_local_storage_data *sdata;
+ struct inode *inode;
+ int err = -EINVAL;
+
+ if (key) {
+ inode = *(struct inode **)(key);
+ sdata = map->ops->map_local_storage_update(inode, map, value,
+ map_flags);
+ return PTR_ERR_OR_ZERO(sdata);
+ }
+ return err;
+}
+
+static int inode_storage_delete(struct inode *inode, struct bpf_map *map)
+{
+ struct bpf_local_storage_data *sdata;
+
+ sdata = inode_storage_lookup(inode, map, false);
+ if (!sdata)
+ return -ENOENT;
+
+ bpf_selem_unlink_map_elem(SELEM(sdata));
+
+ return 0;
+}
+
+static int bpf_inode_storage_delete_elem(struct bpf_map *map, void *key)
+{
+ struct inode *inode;
+ int err = -EINVAL;
+
+ if (key) {
+ inode = *(struct inode **)(key);
+ err = inode_storage_delete(inode, map);
+ }
+
+ return err;
+}
+
+BPF_CALL_4(bpf_inode_storage_get, struct bpf_map *, map, struct inode *, inode,
+ void *, value, u64, flags)
+{
+ struct bpf_local_storage_data *sdata;
+
+ if (flags & ~(BPF_LOCAL_STORAGE_GET_F_CREATE))
+ return (unsigned long)NULL;
+
+ sdata = inode_storage_lookup(inode, map, true);
+ if (sdata)
+ return (unsigned long)sdata->data;
+
+ if (flags & BPF_LOCAL_STORAGE_GET_F_CREATE) {
+ sdata = inode_storage_update(inode, map, value, BPF_NOEXIST);
+ return IS_ERR(sdata) ?
+ (unsigned long)NULL : (unsigned long)sdata->data;
+ }
+
+ return (unsigned long)NULL;
+}
+
+BPF_CALL_2(bpf_inode_storage_delete,
+ struct bpf_map *, map, struct inode *, inode)
+{
+ return inode_storage_delete(inode, map);
+}
+
+static int notsupp_get_next_key(struct bpf_map *map, void *key,
+ void *next_key)
+{
+ return -ENOTSUPP;
+}
+
+DEFINE_BPF_STORAGE_CACHE(inode);
+
+struct bpf_map *inode_storage_map_alloc(union bpf_attr *attr)
+{
+ struct bpf_local_storage_map *smap;
+
+ smap = bpf_local_storage_map_alloc(attr);
+ if (IS_ERR(smap))
+ return ERR_CAST(smap);
+
+ smap->cache_idx = cache_idx_get_inode();
+ return &smap->map;
+}
+
+void inode_storage_map_free(struct bpf_map *map)
+{
+ struct bpf_local_storage_map *smap;
+
+ smap = (struct bpf_local_storage_map *)map;
+ cache_idx_free_inode(smap->cache_idx);
+ bpf_local_storage_map_free(smap);
+}
+
+static int inode_storage_map_btf_id;
+const struct bpf_map_ops inode_storage_map_ops = {
+ .map_alloc_check = bpf_local_storage_map_alloc_check,
+ .map_alloc = inode_storage_map_alloc,
+ .map_free = inode_storage_map_free,
+ .map_get_next_key = notsupp_get_next_key,
+ .map_lookup_elem = bpf_inode_storage_lookup_elem,
+ .map_update_elem = bpf_inode_storage_update_elem,
+ .map_delete_elem = bpf_inode_storage_delete_elem,
+ .map_check_btf = bpf_local_storage_map_check_btf,
+ .map_btf_name = "bpf_local_storage_map",
+ .map_btf_id = &inode_storage_map_btf_id,
+ .map_local_storage_alloc = inode_storage_alloc,
+ .map_selem_alloc = inode_selem_alloc,
+ .map_local_storage_update = inode_storage_update,
+ .map_local_storage_unlink = unlink_inode_storage,
+};
+
+static int bpf_inode_storage_get_btf_ids[4];
+const struct bpf_func_proto bpf_inode_storage_get_proto = {
+ .func = bpf_inode_storage_get,
+ .gpl_only = false,
+ .ret_type = RET_PTR_TO_MAP_VALUE_OR_NULL,
+ .arg1_type = ARG_CONST_MAP_PTR,
+ .arg2_type = ARG_PTR_TO_BTF_ID,
+ .arg3_type = ARG_PTR_TO_MAP_VALUE_OR_NULL,
+ .arg4_type = ARG_ANYTHING,
+ .btf_id = bpf_inode_storage_get_btf_ids,
+};
+
+static int bpf_inode_storage_delete_btf_ids[2];
+const struct bpf_func_proto bpf_inode_storage_delete_proto = {
+ .func = bpf_inode_storage_delete,
+ .gpl_only = false,
+ .ret_type = RET_INTEGER,
+ .arg1_type = ARG_CONST_MAP_PTR,
+ .arg2_type = ARG_PTR_TO_BTF_ID,
+ .btf_id = bpf_inode_storage_delete_btf_ids,
+};
diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
index 156f51ffada2..3fc7586408f7 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -768,7 +768,8 @@ static int map_check_btf(struct bpf_map *map, const struct btf *btf,
if (map->map_type != BPF_MAP_TYPE_HASH &&
map->map_type != BPF_MAP_TYPE_ARRAY &&
map->map_type != BPF_MAP_TYPE_CGROUP_STORAGE &&
- map->map_type != BPF_MAP_TYPE_SK_STORAGE)
+ map->map_type != BPF_MAP_TYPE_SK_STORAGE &&
+ map->map_type != BPF_MAP_TYPE_INODE_STORAGE)
return -ENOTSUPP;
if (map->spin_lock_off + sizeof(struct bpf_spin_lock) >
map->value_size) {
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index b608185e1ffd..9080a62b583a 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -4164,6 +4164,11 @@ static int check_map_func_compatibility(struct bpf_verifier_env *env,
func_id != BPF_FUNC_sk_storage_delete)
goto error;
break;
+ case BPF_MAP_TYPE_INODE_STORAGE:
+ if (func_id != BPF_FUNC_inode_storage_get &&
+ func_id != BPF_FUNC_inode_storage_delete)
+ goto error;
+ break;
default:
break;
}
@@ -4237,6 +4242,11 @@ static int check_map_func_compatibility(struct bpf_verifier_env *env,
if (map->map_type != BPF_MAP_TYPE_SK_STORAGE)
goto error;
break;
+ case BPF_FUNC_inode_storage_get:
+ case BPF_FUNC_inode_storage_delete:
+ if (map->map_type != BPF_MAP_TYPE_INODE_STORAGE)
+ goto error;
+ break;
default:
break;
}
diff --git a/security/bpf/hooks.c b/security/bpf/hooks.c
index 32d32d485451..35f9b19259e5 100644
--- a/security/bpf/hooks.c
+++ b/security/bpf/hooks.c
@@ -3,6 +3,7 @@
/*
* Copyright (C) 2020 Google LLC.
*/
+#include <linux/bpf_local_storage.h>
#include <linux/lsm_hooks.h>
#include <linux/bpf_lsm.h>
@@ -11,6 +12,7 @@ static struct security_hook_list bpf_lsm_hooks[] __lsm_ro_after_init = {
LSM_HOOK_INIT(NAME, bpf_lsm_##NAME),
#include <linux/lsm_hook_defs.h>
#undef LSM_HOOK
+ LSM_HOOK_INIT(inode_free_security, bpf_inode_storage_free),
};
static int __init bpf_lsm_init(void)
@@ -20,7 +22,12 @@ static int __init bpf_lsm_init(void)
return 0;
}
+struct lsm_blob_sizes bpf_lsm_blob_sizes __lsm_ro_after_init = {
+ .lbs_inode = sizeof(struct bpf_storage_blob),
+};
+
DEFINE_LSM(bpf) = {
.name = "bpf",
.init = bpf_lsm_init,
+ .blobs = &bpf_lsm_blob_sizes
};
diff --git a/tools/bpf/bpftool/Documentation/bpftool-map.rst b/tools/bpf/bpftool/Documentation/bpftool-map.rst
index 41e2a74252d0..083db6c2fc67 100644
--- a/tools/bpf/bpftool/Documentation/bpftool-map.rst
+++ b/tools/bpf/bpftool/Documentation/bpftool-map.rst
@@ -49,7 +49,7 @@ MAP COMMANDS
| | **lru_percpu_hash** | **lpm_trie** | **array_of_maps** | **hash_of_maps**
| | **devmap** | **devmap_hash** | **sockmap** | **cpumap** | **xskmap** | **sockhash**
| | **cgroup_storage** | **reuseport_sockarray** | **percpu_cgroup_storage**
-| | **queue** | **stack** | **sk_storage** | **struct_ops** | **ringbuf** }
+| | **queue** | **stack** | **sk_storage** | **struct_ops** | **ringbuf** | **inode_storage** }
DESCRIPTION
===========
diff --git a/tools/bpf/bpftool/bash-completion/bpftool b/tools/bpf/bpftool/bash-completion/bpftool
index 25b25aca1112..34cadc081a78 100644
--- a/tools/bpf/bpftool/bash-completion/bpftool
+++ b/tools/bpf/bpftool/bash-completion/bpftool
@@ -688,7 +688,8 @@ _bpftool()
lru_percpu_hash lpm_trie array_of_maps \
hash_of_maps devmap devmap_hash sockmap cpumap \
xskmap sockhash cgroup_storage reuseport_sockarray \
- percpu_cgroup_storage queue stack' -- \
+ percpu_cgroup_storage queue stack sk_storage \
+ struct_ops inode_storage' -- \
"$cur" ) )
return 0
;;
diff --git a/tools/bpf/bpftool/map.c b/tools/bpf/bpftool/map.c
index 3a27d31a1856..bc0071228f88 100644
--- a/tools/bpf/bpftool/map.c
+++ b/tools/bpf/bpftool/map.c
@@ -50,6 +50,7 @@ const char * const map_type_name[] = {
[BPF_MAP_TYPE_SK_STORAGE] = "sk_storage",
[BPF_MAP_TYPE_STRUCT_OPS] = "struct_ops",
[BPF_MAP_TYPE_RINGBUF] = "ringbuf",
+ [BPF_MAP_TYPE_INODE_STORAGE] = "inode_storage",
};
const size_t map_type_name_size = ARRAY_SIZE(map_type_name);
@@ -1442,7 +1443,7 @@ static int do_help(int argc, char **argv)
" lru_percpu_hash | lpm_trie | array_of_maps | hash_of_maps |\n"
" devmap | devmap_hash | sockmap | cpumap | xskmap | sockhash |\n"
" cgroup_storage | reuseport_sockarray | percpu_cgroup_storage |\n"
- " queue | stack | sk_storage | struct_ops | ringbuf }\n"
+ " queue | stack | sk_storage | struct_ops | ringbuf | inode_storage }\n"
" " HELP_SPEC_OPTIONS "\n"
"",
bin_name, argv[-2]);
diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
index 1f3e831c4813..42fc442f4586 100644
--- a/tools/include/uapi/linux/bpf.h
+++ b/tools/include/uapi/linux/bpf.h
@@ -148,6 +148,7 @@ enum bpf_map_type {
BPF_MAP_TYPE_DEVMAP_HASH,
BPF_MAP_TYPE_STRUCT_OPS,
BPF_MAP_TYPE_RINGBUF,
+ BPF_MAP_TYPE_INODE_STORAGE,
};
/* Note that tracing related programs such as
@@ -3319,6 +3320,41 @@ union bpf_attr {
* A non-negative value equal to or less than *size* on success,
* or a negative error in case of failure.
*
+ * void *bpf_inode_storage_get(struct bpf_map *map, void *inode, void *value, u64 flags)
+ * Description
+ * Get a bpf_local_storage from an *inode*.
+ *
+ * Logically, it could be thought of as getting the value from
+ * a *map* with *inode* as the **key**. From this
+ * perspective, the usage is not much different from
+ * **bpf_map_lookup_elem**\ (*map*, **&**\ *inode*) except this
+ * helper enforces the key must be an inode and the map must also
+ * be a **BPF_MAP_TYPE_INODE_STORAGE**.
+ *
+ * Underneath, the value is stored locally at *inode* instead of
+ * the *map*. The *map* is used as the bpf-local-storage
+ * "type". The bpf-local-storage "type" (i.e. the *map*) is
+ * searched against all bpf_local_storage residing at *inode*.
+ *
+ * An optional *flags* (**BPF_LOCAL_STORAGE_GET_F_CREATE**) can be
+ * used such that a new bpf_local_storage will be
+ * created if one does not exist. *value* can be used
+ * together with **BPF_LOCAL_STORAGE_GET_F_CREATE** to specify
+ * the initial value of a bpf_local_storage. If *value* is
+ * **NULL**, the new bpf_local_storage will be zero initialized.
+ * Return
+ * A bpf_local_storage pointer is returned on success.
+ *
+ * **NULL** if not found or there was an error in adding
+ * a new bpf_local_storage.
+ *
+ * int bpf_inode_storage_delete(struct bpf_map *map, void *inode)
+ * Description
+ * Delete a bpf_local_storage from an *inode*.
+ * Return
+ * 0 on success.
+ *
+ * **-ENOENT** if the bpf_local_storage cannot be found.
*/
#define __BPF_FUNC_MAPPER(FN) \
FN(unspec), \
@@ -3463,6 +3499,8 @@ union bpf_attr {
FN(skc_to_tcp_request_sock), \
FN(skc_to_udp6_sock), \
FN(get_task_stack), \
+ FN(inode_storage_get), \
+ FN(inode_storage_delete), \
/* */
/* integer value in 'imm' field of BPF_CALL instruction selects which helper
diff --git a/tools/lib/bpf/libbpf_probes.c b/tools/lib/bpf/libbpf_probes.c
index 10cd8d1891f5..b859558ce290 100644
--- a/tools/lib/bpf/libbpf_probes.c
+++ b/tools/lib/bpf/libbpf_probes.c
@@ -170,7 +170,7 @@ int libbpf__load_raw_btf(const char *raw_types, size_t types_len,
return btf_fd;
}
-static int load_sk_storage_btf(void)
+static int load_local_storage_btf(void)
{
const char strs[] = "\0bpf_spin_lock\0val\0cnt\0l";
/* struct bpf_spin_lock {
@@ -229,12 +229,13 @@ bool bpf_probe_map_type(enum bpf_map_type map_type, __u32 ifindex)
key_size = 0;
break;
case BPF_MAP_TYPE_SK_STORAGE:
+ case BPF_MAP_TYPE_INODE_STORAGE:
btf_key_type_id = 1;
btf_value_type_id = 3;
value_size = 8;
max_entries = 0;
map_flags = BPF_F_NO_PREALLOC;
- btf_fd = load_sk_storage_btf();
+ btf_fd = load_local_storage_btf();
if (btf_fd < 0)
return false;
break;
--
2.27.0.389.gc38d7665816-goog
^ permalink raw reply related
* Re: [PATCH AUTOSEL 5.7 03/30] ima: extend boot_aggregate with kernel measurements
From: Sasha Levin @ 2020-07-09 1:27 UTC (permalink / raw)
To: Mimi Zohar
Cc: linux-kernel, stable, Maurizio Drocco, Bruno Meneguele,
linux-integrity, linux-security-module
In-Reply-To: <1594224793.23056.251.camel@linux.ibm.com>
On Wed, Jul 08, 2020 at 12:13:13PM -0400, Mimi Zohar wrote:
>Hi Sasha,
>
>On Wed, 2020-07-08 at 11:40 -0400, Sasha Levin wrote:
>> From: Maurizio Drocco <maurizio.drocco@ibm.com>
>>
>> [ Upstream commit 20c59ce010f84300f6c655d32db2610d3433f85c ]
>>
>> Registers 8-9 are used to store measurements of the kernel and its
>> command line (e.g., grub2 bootloader with tpm module enabled). IMA
>> should include them in the boot aggregate. Registers 8-9 should be
>> only included in non-SHA1 digests to avoid ambiguity.
>
>Prior to Linux 5.8, the SHA1 template data hashes were padded before
>being extended into the TPM. Support for calculating and extending
>the per TPM bank template data digests is only being upstreamed in
>Linux 5.8.
>
>How will attestation servers know whether to include PCRs 8 & 9 in the
>the boot_aggregate calculation? Now, there is a direct relationship
>between the template data SHA1 padded digest not including PCRs 8 & 9,
>and the new per TPM bank template data digest including them.
Got it, I'll drop it then, thank you!
--
Thanks,
Sasha
^ permalink raw reply
* Re: [PATCH v18 22/23] LSM: Add /proc attr entry for full LSM context
From: Jann Horn @ 2020-07-09 1:30 UTC (permalink / raw)
To: Casey Schaufler
Cc: Casey Schaufler, James Morris, linux-security-module,
SElinux list, Kees Cook, John Johansen, Tetsuo Handa, Paul Moore,
Stephen Smalley, Linux API
In-Reply-To: <20200709001234.9719-23-casey@schaufler-ca.com>
On Thu, Jul 9, 2020 at 2:42 AM Casey Schaufler <casey@schaufler-ca.com> wrote:
> Add an entry /proc/.../attr/context which displays the full
> process security "context" in compound format:
> lsm1\0value\0lsm2\0value\0...
> This entry is not writable.
>
> Reviewed-by: Kees Cook <keescook@chromium.org>
> Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
> Cc: linux-api@vger.kernel.org
[...]
> diff --git a/security/security.c b/security/security.c
[...]
> +/**
> + * append_ctx - append a lsm/context pair to a compound context
> + * @ctx: the existing compound context
> + * @ctxlen: size of the old context, including terminating nul byte
> + * @lsm: new lsm name, nul terminated
> + * @new: new context, possibly nul terminated
> + * @newlen: maximum size of @new
> + *
> + * replace @ctx with a new compound context, appending @newlsm and @new
> + * to @ctx. On exit the new data replaces the old, which is freed.
> + * @ctxlen is set to the new size, which includes a trailing nul byte.
> + *
> + * Returns 0 on success, -ENOMEM if no memory is available.
> + */
> +static int append_ctx(char **ctx, int *ctxlen, const char *lsm, char *new,
> + int newlen)
> +{
> + char *final;
> + int llen;
Please use size_t to represent object sizes, instead of implicitly
truncating them and assuming that that doesn't wrap. Using "int" here
not only makes it harder to statically reason about this code, it
actually can also make the generated code worse:
$ cat numtrunc.c
#include <stddef.h>
size_t my_strlen(char *p);
void *my_alloc(size_t len);
void *blah_trunc(char *p) {
int len = my_strlen(p) + 1;
return my_alloc(len);
}
void *blah_notrunc(char *p) {
size_t len = my_strlen(p) + 1;
return my_alloc(len);
}
$ gcc -O2 -c -o numtrunc.o numtrunc.c
$ objdump -d numtrunc.o
[...]
0000000000000000 <blah_trunc>:
0: 48 83 ec 08 sub $0x8,%rsp
4: e8 00 00 00 00 callq 9 <blah_trunc+0x9>
9: 48 83 c4 08 add $0x8,%rsp
d: 8d 78 01 lea 0x1(%rax),%edi
10: 48 63 ff movslq %edi,%rdi <<<<<<<<unnecessary instruction
13: e9 00 00 00 00 jmpq 18 <blah_trunc+0x18>
[...]
0000000000000020 <blah_notrunc>:
20: 48 83 ec 08 sub $0x8,%rsp
24: e8 00 00 00 00 callq 29 <blah_notrunc+0x9>
29: 48 83 c4 08 add $0x8,%rsp
2d: 48 8d 78 01 lea 0x1(%rax),%rdi
31: e9 00 00 00 00 jmpq 36 <blah_notrunc+0x16>
$
This is because GCC documents
(https://gcc.gnu.org/onlinedocs/gcc/Integers-implementation.html) that
for integer conversions where the value does not fit into the signed
target type, "the value is reduced modulo 2^N to be within range of
the type"; so the compiler has to assume that you are actually
intentionally trying to truncate the more significant bits from the
length, and therefore may have to insert extra code to ensure that
this truncation happens.
> + llen = strlen(lsm) + 1;
> + newlen = strnlen(new, newlen) + 1;
This strnlen() call seems dodgy. If an LSM can return a string that
already contains null bytes, shouldn't that be considered a bug, given
that it can't be displayed properly? Would it be more appropriate to
have a WARN_ON(memchr(new, '\0', newlen)) check here and bail out if
that happens?
> + final = kzalloc(*ctxlen + llen + newlen, GFP_KERNEL);
> + if (final == NULL)
> + return -ENOMEM;
> + if (*ctxlen)
> + memcpy(final, *ctx, *ctxlen);
> + memcpy(final + *ctxlen, lsm, llen);
> + memcpy(final + *ctxlen + llen, new, newlen);
> + kfree(*ctx);
> + *ctx = final;
> + *ctxlen = *ctxlen + llen + newlen;
> + return 0;
> +}
> +
> /*
> * The default value of the LSM hook is defined in linux/lsm_hook_defs.h and
> * can be accessed with:
> @@ -2109,6 +2145,10 @@ int security_getprocattr(struct task_struct *p, const char *lsm, char *name,
> char **value)
> {
> struct security_hook_list *hp;
> + char *final = NULL;
> + char *cp;
> + int rc = 0;
> + int finallen = 0;
> int display = lsm_task_display(current);
> int slot = 0;
>
[...]
> return -ENOMEM;
> }
>
> + if (!strcmp(name, "context")) {
> + hlist_for_each_entry(hp, &security_hook_heads.getprocattr,
> + list) {
> + rc = hp->hook.getprocattr(p, "context", &cp);
> + if (rc == -EINVAL)
> + continue;
> + if (rc < 0) {
> + kfree(final);
> + return rc;
> + }
This means that if SELinux refuses to give the caller PROCESS__GETATTR
access to the target process, the entire "context" file will refuse to
show anything, even if e.g. an AppArmor label would be visible through
the LSM-specific attribute directory, right? That seems awkward. Can
you maybe omit context elements for which the access check failed
instead, or embed an extra flag byte to signal for each element
whether the lookup failed, or something along those lines?
If this is an intentional design limitation, it should probably be
documented in the commit message or so.
> + rc = append_ctx(&final, &finallen, hp->lsmid->lsm,
> + cp, rc);
> + if (rc < 0) {
> + kfree(final);
> + return rc;
> + }
Isn't there a memory leak here? `cp` points to memory that was
allocated by hp->hook.getprocattr(), and you're not freeing it after
append_ctx(). (And append_ctx() also doesn't free it.)
> + }
> + if (final == NULL)
> + return -EINVAL;
> + *value = final;
> + return finallen;
> + }
> +
> hlist_for_each_entry(hp, &security_hook_heads.getprocattr, list) {
> if (lsm != NULL && strcmp(lsm, hp->lsmid->lsm))
> continue;
^ permalink raw reply
* Re: [PATCH 0/4] Fix misused kernel_read_file() enums
From: Kees Cook @ 2020-07-09 2:00 UTC (permalink / raw)
To: Luis Chamberlain
Cc: Hans de Goede, James Morris, Mimi Zohar, Scott Branden,
Greg Kroah-Hartman, Rafael J. Wysocki, Alexander Viro, Jessica Yu,
Dmitry Kasatkin, Serge E. Hallyn, Casey Schaufler,
Eric W. Biederman, Peter Zijlstra, Matthew Garrett, David Howells,
Mauro Carvalho Chehab, Randy Dunlap, Joel Fernandes (Google),
KP Singh, Dave Olsthoorn, Peter Jones, Andrew Morton,
Stephen Boyd, Paul Moore, linux-kernel, linux-fsdevel,
linux-integrity, linux-security-module
In-Reply-To: <20200708133004.GG4332@42.do-not-panic.com>
On Wed, Jul 08, 2020 at 01:30:04PM +0000, Luis Chamberlain wrote:
> On Wed, Jul 08, 2020 at 01:58:47PM +0200, Hans de Goede wrote:
> > Hi,
> >
> > On 7/8/20 1:55 PM, Luis Chamberlain wrote:
> > > On Wed, Jul 08, 2020 at 01:37:41PM +0200, Hans de Goede wrote:
> > > > Hi,
> > > >
> > > > On 7/8/20 1:01 PM, Hans de Goede wrote:
> > > > > Hi,
> > > > >
> > > > > On 7/7/20 10:19 AM, Kees Cook wrote:
> > > > > > Hi,
> > > > > >
> > > > > > In looking for closely at the additions that got made to the
> > > > > > kernel_read_file() enums, I noticed that FIRMWARE_PREALLOC_BUFFER
> > > > > > and FIRMWARE_EFI_EMBEDDED were added, but they are not appropriate
> > > > > > *kinds* of files for the LSM to reason about. They are a "how" and
> > > > > > "where", respectively. Remove these improper aliases and refactor the
> > > > > > code to adapt to the changes.
> > > > > >
> > > > > > Additionally adds in missing calls to security_kernel_post_read_file()
> > > > > > in the platform firmware fallback path (to match the sysfs firmware
> > > > > > fallback path) and in module loading. I considered entirely removing
> > > > > > security_kernel_post_read_file() hook since it is technically unused,
> > > > > > but IMA probably wants to be able to measure EFI-stored firmware images,
> > > > > > so I wired it up and matched it for modules, in case anyone wants to
> > > > > > move the module signature checks out of the module core and into an LSM
> > > > > > to avoid the current layering violations.
> > > > > >
> > > > > > This touches several trees, and I suspect it would be best to go through
> > > > > > James's LSM tree.
> > > > > >
> > > > > > Thanks!
> > > > >
> > > > >
> > > > > I've done some quick tests on this series to make sure that
> > > > > the efi embedded-firmware support did not regress.
> > > > > That still works fine, so this series is;
> > > > >
> > > > > Tested-by: Hans de Goede <hdegoede@redhat.com>
> > > >
> > > > I made a mistake during testing I was not actually running the
> > > > kernel with the patches added.
> > > >
> > > > After fixing that I did find a problem, patch 4/4:
> > > > "module: Add hook for security_kernel_post_read_file()"
> > > >
> > > > Breaks module-loading for me. This is with the 4 patches
> > > > on top of 5.8.0-rc4, so this might just be because I'm
> > > > not using the right base.
> > > >
> > > > With patch 4/4 reverted things work fine for me.
> > > >
> > > > So, please only add my Tested-by to patches 1-3.
> > >
> > > BTW is there any testing covered by the selftests for the firmware
> > > laoder which would have caputured this? If not can you extend
> > > it with something to capture this case you ran into?
> >
> > This was not a firmware-loading issue. For me in my tests,
> > which were limited to 1 device, patch 4/4, which only touches
> > the module-loading code, stopped module loading from working.
> >
> > Since my test device has / on an eMMC and the kernel config
> > I'm using has mmc-block as a module, things just hung in the
> > initrd since no modules could be loaded, so I did not debug
> > this any further. Dropping patch 4/4 from my local tree
> > solved this.
>
> Thanks Hans!
>
> Kees, would test_kmod.c and the respective selftest would have picked
> this issue up?
I need to check -- I got a (possibly related) 0day report on it too.
Since I have to clean it up further based on Mimi's comments, and adapt
it a bit for Scott's series, I'll need to get a v2 spun for sure. :)
--
Kees Cook
^ permalink raw reply
* Re: [PATCH bpf-next v3 2/4] bpf: Implement bpf_local_storage for inodes
From: kernel test robot @ 2020-07-09 4:37 UTC (permalink / raw)
To: KP Singh, linux-kernel, bpf, linux-security-module
Cc: kbuild-all, Alexei Starovoitov, Daniel Borkmann, Paul Turner,
Jann Horn, Florent Revest
In-Reply-To: <20200709005654.3324272-3-kpsingh@chromium.org>
[-- Attachment #1: Type: text/plain, Size: 2114 bytes --]
Hi KP,
I love your patch! Perhaps something to improve:
[auto build test WARNING on bpf-next/master]
url: https://github.com/0day-ci/linux/commits/KP-Singh/Generalizing-bpf_local_storage/20200709-085810
base: https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next.git master
config: mips-allyesconfig (attached as .config)
compiler: mips-linux-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=mips
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All warnings (new ones prefixed by >>):
>> kernel/bpf/bpf_inode_storage.c:274:17: warning: no previous prototype for 'inode_storage_map_alloc' [-Wmissing-prototypes]
274 | struct bpf_map *inode_storage_map_alloc(union bpf_attr *attr)
| ^~~~~~~~~~~~~~~~~~~~~~~
>> kernel/bpf/bpf_inode_storage.c:286:6: warning: no previous prototype for 'inode_storage_map_free' [-Wmissing-prototypes]
286 | void inode_storage_map_free(struct bpf_map *map)
| ^~~~~~~~~~~~~~~~~~~~~~
vim +/inode_storage_map_alloc +274 kernel/bpf/bpf_inode_storage.c
273
> 274 struct bpf_map *inode_storage_map_alloc(union bpf_attr *attr)
275 {
276 struct bpf_local_storage_map *smap;
277
278 smap = bpf_local_storage_map_alloc(attr);
279 if (IS_ERR(smap))
280 return ERR_CAST(smap);
281
282 smap->cache_idx = cache_idx_get_inode();
283 return &smap->map;
284 }
285
> 286 void inode_storage_map_free(struct bpf_map *map)
287 {
288 struct bpf_local_storage_map *smap;
289
290 smap = (struct bpf_local_storage_map *)map;
291 cache_idx_free_inode(smap->cache_idx);
292 bpf_local_storage_map_free(smap);
293 }
294
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 67069 bytes --]
^ permalink raw reply
* [PATCH v3 00/12] ima: Fix rule parsing bugs and extend KEXEC_CMDLINE rule support
From: Tyler Hicks @ 2020-07-09 6:18 UTC (permalink / raw)
To: Mimi Zohar, Dmitry Kasatkin
Cc: James Morris, Serge E . Hallyn, Lakshmi Ramasubramanian,
Prakhar Srivastava, linux-kernel, linux-integrity,
linux-security-module, Janne Karhunen, Eric Biederman, kexec,
Casey Schaufler, Nayna Jain
This series ultimately extends the supported IMA rule conditionals for
the KEXEC_CMDLINE hook function. As of today, there's an imbalance in
IMA language conditional support for KEXEC_CMDLINE rules in comparison
to KEXEC_KERNEL_CHECK and KEXEC_INITRAMFS_CHECK rules. The KEXEC_CMDLINE
rules do not support *any* conditionals so you cannot have a sequence of
rules like this:
dont_measure func=KEXEC_KERNEL_CHECK obj_type=foo_t
dont_measure func=KEXEC_INITRAMFS_CHECK obj_type=foo_t
dont_measure func=KEXEC_CMDLINE obj_type=foo_t
measure func=KEXEC_KERNEL_CHECK
measure func=KEXEC_INITRAMFS_CHECK
measure func=KEXEC_CMDLINE
Instead, KEXEC_CMDLINE rules can only be measured or not measured and
there's no additional flexibility in today's implementation of the
KEXEC_CMDLINE hook function.
With this series, the above sequence of rules becomes valid and any
calls to kexec_file_load() with a kernel and initramfs inode type of
foo_t will not be measured (that includes the kernel cmdline buffer)
while all other objects given to a kexec_file_load() syscall will be
measured. There's obviously not an inode directly associated with the
kernel cmdline buffer but this patch series ties the inode based
decision making for KEXEC_CMDLINE to the kernel's inode. I think this
will be intuitive to policy authors.
While reading IMA code and preparing to make this change, I realized
that the buffer based hook functions (KEXEC_CMDLINE and KEY_CHECK) are
quite special in comparison to longer standing hook functions. These
buffer based hook functions can only support measure actions and there
are some restrictions on the conditionals that they support. However,
the rule parser isn't enforcing any of those restrictions and IMA policy
authors wouldn't have any immediate way of knowing that the policy that
they wrote is invalid. For example, the sequence of rules above parses
successfully in today's kernel but the
"dont_measure func=KEXEC_CMDLINE ..." rule is incorrectly handled in
ima_match_rules(). The dont_measure rule is *always* considered to be a
match so, surprisingly, no KEXEC_CMDLINE measurements are made.
While making the rule parser more strict, I realized that the parser
does not correctly free all of the allocated memory associated with an
ima_rule_entry when going down some error paths. Invalid policy loaded
by the policy administrator could result in small memory leaks.
I envision patches 1-7 going to stable. The series is ordered in a way
that has all the fixes up front, followed by cleanups, followed by the
feature patch. The breakdown of patches looks like so:
Memory leak fixes: 1-3
Parser strictness fixes: 4-7
Code cleanups made possible by the fixes: 8-11
Extend KEXEC_CMDLINE rule support: 12
Perhaps the most logical ordering for code review is:
1, 2, 3, 8, 9, 4, 5, 6, 7, 10, 11, 12
If you'd like me to re-order or split up the series, just let me know.
Thanks for considering these patches!
* Series-wide v3 changes
- Indentation changes in patch #4 which caused some churn
- Added patch #7
- Significant changes to patch #10 to address Mimi's requests
* Series-wide v2 changes
- Rebased onto next-integrity-testing
- Squashed patches 2 and 3 from v1
+ Updated this cover letter to account for changes to patch index
changes
+ See patch 2 for specific code changes
Tyler
Tyler Hicks (12):
ima: Have the LSM free its audit rule
ima: Free the entire rule when deleting a list of rules
ima: Free the entire rule if it fails to parse
ima: Fail rule parsing when buffer hook functions have an invalid
action
ima: Fail rule parsing when the KEXEC_CMDLINE hook is combined with an
invalid cond
ima: Fail rule parsing when the KEY_CHECK hook is combined with an
invalid cond
ima: Fail rule parsing when appraise_flag=blacklist is unsupportable
ima: Shallow copy the args_p member of ima_rule_entry.lsm elements
ima: Use correct type for the args_p member of ima_rule_entry.lsm
elements
ima: Move comprehensive rule validation checks out of the token parser
ima: Use the common function to detect LSM conditionals in a rule
ima: Support additional conditionals in the KEXEC_CMDLINE hook
function
include/linux/ima.h | 4 +-
kernel/kexec_file.c | 2 +-
security/integrity/ima/ima.h | 13 +-
security/integrity/ima/ima_api.c | 2 +-
security/integrity/ima/ima_appraise.c | 2 +-
security/integrity/ima/ima_asymmetric_keys.c | 2 +-
security/integrity/ima/ima_main.c | 23 ++-
security/integrity/ima/ima_modsig.c | 20 --
security/integrity/ima/ima_policy.c | 206 ++++++++++++++-----
security/integrity/ima/ima_queue_keys.c | 2 +-
10 files changed, 182 insertions(+), 94 deletions(-)
--
2.25.1
^ permalink raw reply
* [PATCH v3 01/12] ima: Have the LSM free its audit rule
From: Tyler Hicks @ 2020-07-09 6:19 UTC (permalink / raw)
To: Mimi Zohar, Dmitry Kasatkin
Cc: James Morris, Serge E . Hallyn, Lakshmi Ramasubramanian,
Prakhar Srivastava, linux-kernel, linux-integrity,
linux-security-module, Janne Karhunen, Casey Schaufler
In-Reply-To: <20200709061911.954326-1-tyhicks@linux.microsoft.com>
Ask the LSM to free its audit rule rather than directly calling kfree().
Both AppArmor and SELinux do additional work in their audit_rule_free()
hooks. Fix memory leaks by allowing the LSMs to perform necessary work.
Fixes: b16942455193 ("ima: use the lsm policy update notifier")
Signed-off-by: Tyler Hicks <tyhicks@linux.microsoft.com>
Cc: Janne Karhunen <janne.karhunen@gmail.com>
Cc: Casey Schaufler <casey@schaufler-ca.com>
Reviewed-by: Mimi Zohar <zohar@linux.ibm.com>
---
* v3
- No change
* v2
- Fixed build warning by dropping the 'return -EINVAL' from
the stubbed out security_filter_rule_free() since it has a void
return type
- Added Mimi's Reviewed-by
- Developed a follow-on patch to rename security_filter_rule_*()
functions, to address Casey's request, but I'll submit it
independently of this patch series since it is somewhat unrelated
security/integrity/ima/ima.h | 5 +++++
security/integrity/ima/ima_policy.c | 2 +-
2 files changed, 6 insertions(+), 1 deletion(-)
diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index 4515975cc540..59ec28f5c117 100644
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h
@@ -420,6 +420,7 @@ static inline void ima_free_modsig(struct modsig *modsig)
#ifdef CONFIG_IMA_LSM_RULES
#define security_filter_rule_init security_audit_rule_init
+#define security_filter_rule_free security_audit_rule_free
#define security_filter_rule_match security_audit_rule_match
#else
@@ -430,6 +431,10 @@ static inline int security_filter_rule_init(u32 field, u32 op, char *rulestr,
return -EINVAL;
}
+static inline void security_filter_rule_free(void *lsmrule)
+{
+}
+
static inline int security_filter_rule_match(u32 secid, u32 field, u32 op,
void *lsmrule)
{
diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
index 66aa3e17a888..d7c268c2b0ce 100644
--- a/security/integrity/ima/ima_policy.c
+++ b/security/integrity/ima/ima_policy.c
@@ -258,7 +258,7 @@ static void ima_lsm_free_rule(struct ima_rule_entry *entry)
int i;
for (i = 0; i < MAX_LSM_RULES; i++) {
- kfree(entry->lsm[i].rule);
+ security_filter_rule_free(entry->lsm[i].rule);
kfree(entry->lsm[i].args_p);
}
kfree(entry);
--
2.25.1
^ permalink raw reply related
* [PATCH v3 02/12] ima: Free the entire rule when deleting a list of rules
From: Tyler Hicks @ 2020-07-09 6:19 UTC (permalink / raw)
To: Mimi Zohar, Dmitry Kasatkin
Cc: James Morris, Serge E . Hallyn, Lakshmi Ramasubramanian,
Prakhar Srivastava, linux-kernel, linux-integrity,
linux-security-module
In-Reply-To: <20200709061911.954326-1-tyhicks@linux.microsoft.com>
Create a function, ima_free_rule(), to free all memory associated with
an ima_rule_entry. Use the new function to fix memory leaks of allocated
ima_rule_entry members, such as .fsname and .keyrings, when deleting a
list of rules.
Make the existing ima_lsm_free_rule() function specific to the LSM
audit rule array of an ima_rule_entry and require that callers make an
additional call to kfree to free the ima_rule_entry itself.
This fixes a memory leak seen when loading by a valid rule that contains
an additional piece of allocated memory, such as an fsname, followed by
an invalid rule that triggers a policy load failure:
# echo -e "dont_measure fsname=securityfs\nbad syntax" > \
/sys/kernel/security/ima/policy
-bash: echo: write error: Invalid argument
# echo scan > /sys/kernel/debug/kmemleak
# cat /sys/kernel/debug/kmemleak
unreferenced object 0xffff9bab67ca12c0 (size 16):
comm "bash", pid 684, jiffies 4295212803 (age 252.344s)
hex dump (first 16 bytes):
73 65 63 75 72 69 74 79 66 73 00 6b 6b 6b 6b a5 securityfs.kkkk.
backtrace:
[<00000000adc80b1b>] kstrdup+0x2e/0x60
[<00000000d504cb0d>] ima_parse_add_rule+0x7d4/0x1020
[<00000000444825ac>] ima_write_policy+0xab/0x1d0
[<000000002b7f0d6c>] vfs_write+0xde/0x1d0
[<0000000096feedcf>] ksys_write+0x68/0xe0
[<0000000052b544a2>] do_syscall_64+0x56/0xa0
[<000000007ead1ba7>] entry_SYSCALL_64_after_hwframe+0x44/0xa9
Fixes: f1b08bbcbdaf ("ima: define a new policy condition based on the filesystem name")
Fixes: 2b60c0ecedf8 ("IMA: Read keyrings= option from the IMA policy")
Signed-off-by: Tyler Hicks <tyhicks@linux.microsoft.com>
---
* v3
- No change
* v2
- Collapsed patch #2 from v1 of this series, into this patch. This
patch now introduces ima_free_rule().
- Existing callers of ima_lsm_free_rule() are doing so to free rules
after a successful or failed ima_lsm_copy_rule() and those callers
continue to directly call ima_lsm_copy_rule() rather than doing
explicit reference ownership and calling ima_free_rule().
- The kfree(entry) of ima_lsm_free_rule() was removed from that
function to make it focused on freeing the LSM references. Direct
callers of ima_lsm_free_rule() must now call kfree(entry) after
ima_lsm_free_rule().
- A comment was added in ima_lsm_update_rule() to clarify why
ima_free_rule() isn't being used.
security/integrity/ima/ima_policy.c | 29 ++++++++++++++++++++++++-----
1 file changed, 24 insertions(+), 5 deletions(-)
diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
index d7c268c2b0ce..bf00b966e87f 100644
--- a/security/integrity/ima/ima_policy.c
+++ b/security/integrity/ima/ima_policy.c
@@ -261,6 +261,21 @@ static void ima_lsm_free_rule(struct ima_rule_entry *entry)
security_filter_rule_free(entry->lsm[i].rule);
kfree(entry->lsm[i].args_p);
}
+}
+
+static void ima_free_rule(struct ima_rule_entry *entry)
+{
+ if (!entry)
+ return;
+
+ /*
+ * entry->template->fields may be allocated in ima_parse_rule() but that
+ * reference is owned by the corresponding ima_template_desc element in
+ * the defined_templates list and cannot be freed here
+ */
+ kfree(entry->fsname);
+ kfree(entry->keyrings);
+ ima_lsm_free_rule(entry);
kfree(entry);
}
@@ -302,6 +317,7 @@ static struct ima_rule_entry *ima_lsm_copy_rule(struct ima_rule_entry *entry)
out_err:
ima_lsm_free_rule(nentry);
+ kfree(nentry);
return NULL;
}
@@ -315,7 +331,14 @@ static int ima_lsm_update_rule(struct ima_rule_entry *entry)
list_replace_rcu(&entry->list, &nentry->list);
synchronize_rcu();
+ /*
+ * ima_lsm_copy_rule() shallow copied all references, except for the
+ * LSM references, from entry to nentry so we only want to free the LSM
+ * references and the entry itself. All other memory refrences will now
+ * be owned by nentry.
+ */
ima_lsm_free_rule(entry);
+ kfree(entry);
return 0;
}
@@ -1402,15 +1425,11 @@ ssize_t ima_parse_add_rule(char *rule)
void ima_delete_rules(void)
{
struct ima_rule_entry *entry, *tmp;
- int i;
temp_ima_appraise = 0;
list_for_each_entry_safe(entry, tmp, &ima_temp_rules, list) {
- for (i = 0; i < MAX_LSM_RULES; i++)
- kfree(entry->lsm[i].args_p);
-
list_del(&entry->list);
- kfree(entry);
+ ima_free_rule(entry);
}
}
--
2.25.1
^ permalink raw reply related
* [PATCH v3 07/12] ima: Fail rule parsing when appraise_flag=blacklist is unsupportable
From: Tyler Hicks @ 2020-07-09 6:19 UTC (permalink / raw)
To: Mimi Zohar, Dmitry Kasatkin
Cc: James Morris, Serge E . Hallyn, Lakshmi Ramasubramanian,
Prakhar Srivastava, linux-kernel, linux-integrity,
linux-security-module, Nayna Jain
In-Reply-To: <20200709061911.954326-1-tyhicks@linux.microsoft.com>
The "appraise_flag" option is only appropriate for appraise actions
and its "blacklist" value is only appropriate when
CONFIG_IMA_APPRAISE_MODSIG is enabled and "appraise_flag=blacklist" is
only appropriate when "appraise_type=imasig|modsig" is also present.
Make this clear at policy load so that IMA policy authors don't assume
that other uses of "appraise_flag=blacklist" are supported.
Fixes: 273df864cf74 ("ima: Check against blacklisted hashes for files with modsig")
Signed-off-by: Tyler Hicks <tyhicks@linux.microsoft.com>
Cc: Nayna Jain <nayna@linux.ibm.com>
---
* v3
- New patch
security/integrity/ima/ima_policy.c | 13 ++++++++++++-
1 file changed, 12 insertions(+), 1 deletion(-)
diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
index 81da02071d41..9842e2e0bc6d 100644
--- a/security/integrity/ima/ima_policy.c
+++ b/security/integrity/ima/ima_policy.c
@@ -1035,6 +1035,11 @@ static bool ima_validate_rule(struct ima_rule_entry *entry)
return false;
}
+ /* Ensure that combinations of flags are compatible with each other */
+ if (entry->flags & IMA_CHECK_BLACKLIST &&
+ !(entry->flags & IMA_MODSIG_ALLOWED))
+ return false;
+
return true;
}
@@ -1371,8 +1376,14 @@ static int ima_parse_rule(char *rule, struct ima_rule_entry *entry)
result = -EINVAL;
break;
case Opt_appraise_flag:
+ if (entry->action != APPRAISE) {
+ result = -EINVAL;
+ break;
+ }
+
ima_log_string(ab, "appraise_flag", args[0].from);
- if (strstr(args[0].from, "blacklist"))
+ if (IS_ENABLED(CONFIG_IMA_APPRAISE_MODSIG) &&
+ strstr(args[0].from, "blacklist"))
entry->flags |= IMA_CHECK_BLACKLIST;
break;
case Opt_permit_directio:
--
2.25.1
^ permalink raw reply related
* [PATCH v3 12/12] ima: Support additional conditionals in the KEXEC_CMDLINE hook function
From: Tyler Hicks @ 2020-07-09 6:19 UTC (permalink / raw)
To: Mimi Zohar, Dmitry Kasatkin
Cc: James Morris, Serge E . Hallyn, Lakshmi Ramasubramanian,
Prakhar Srivastava, linux-kernel, linux-integrity,
linux-security-module, Eric Biederman, kexec
In-Reply-To: <20200709061911.954326-1-tyhicks@linux.microsoft.com>
Take the properties of the kexec kernel's inode and the current task
ownership into consideration when matching a KEXEC_CMDLINE operation to
the rules in the IMA policy. This allows for some uniformity when
writing IMA policy rules for KEXEC_KERNEL_CHECK, KEXEC_INITRAMFS_CHECK,
and KEXEC_CMDLINE operations.
Prior to this patch, it was not possible to write a set of rules like
this:
dont_measure func=KEXEC_KERNEL_CHECK obj_type=foo_t
dont_measure func=KEXEC_INITRAMFS_CHECK obj_type=foo_t
dont_measure func=KEXEC_CMDLINE obj_type=foo_t
measure func=KEXEC_KERNEL_CHECK
measure func=KEXEC_INITRAMFS_CHECK
measure func=KEXEC_CMDLINE
The inode information associated with the kernel being loaded by a
kexec_kernel_load(2) syscall can now be included in the decision to
measure or not
Additonally, the uid, euid, and subj_* conditionals can also now be
used in KEXEC_CMDLINE rules. There was no technical reason as to why
those conditionals weren't being considered previously other than
ima_match_rules() didn't have a valid inode to use so it immediately
bailed out for KEXEC_CMDLINE operations rather than going through the
full list of conditional comparisons.
Signed-off-by: Tyler Hicks <tyhicks@linux.microsoft.com>
Cc: Eric Biederman <ebiederm@xmission.com>
Cc: kexec@lists.infradead.org
Reviewed-by: Lakshmi Ramasubramanian <nramas@linux.microsoft.com>
---
* v3
- Added Lakshmi's Reviewed-by
- Adjust for the indentation change introduced in patch #4
* v2
- Moved the inode parameter of process_buffer_measurement() to be the
first parameter so that it more closely matches process_masurement()
include/linux/ima.h | 4 ++--
kernel/kexec_file.c | 2 +-
security/integrity/ima/ima.h | 2 +-
security/integrity/ima/ima_api.c | 2 +-
security/integrity/ima/ima_appraise.c | 2 +-
security/integrity/ima/ima_asymmetric_keys.c | 2 +-
security/integrity/ima/ima_main.c | 23 +++++++++++++++-----
security/integrity/ima/ima_policy.c | 17 +++++----------
security/integrity/ima/ima_queue_keys.c | 2 +-
9 files changed, 31 insertions(+), 25 deletions(-)
diff --git a/include/linux/ima.h b/include/linux/ima.h
index 9164e1534ec9..d15100de6cdd 100644
--- a/include/linux/ima.h
+++ b/include/linux/ima.h
@@ -25,7 +25,7 @@ extern int ima_post_read_file(struct file *file, void *buf, loff_t size,
enum kernel_read_file_id id);
extern void ima_post_path_mknod(struct dentry *dentry);
extern int ima_file_hash(struct file *file, char *buf, size_t buf_size);
-extern void ima_kexec_cmdline(const void *buf, int size);
+extern void ima_kexec_cmdline(int kernel_fd, const void *buf, int size);
#ifdef CONFIG_IMA_KEXEC
extern void ima_add_kexec_buffer(struct kimage *image);
@@ -103,7 +103,7 @@ static inline int ima_file_hash(struct file *file, char *buf, size_t buf_size)
return -EOPNOTSUPP;
}
-static inline void ima_kexec_cmdline(const void *buf, int size) {}
+static inline void ima_kexec_cmdline(int kernel_fd, const void *buf, int size) {}
#endif /* CONFIG_IMA */
#ifndef CONFIG_IMA_KEXEC
diff --git a/kernel/kexec_file.c b/kernel/kexec_file.c
index bb05fd52de85..07df431c1f21 100644
--- a/kernel/kexec_file.c
+++ b/kernel/kexec_file.c
@@ -287,7 +287,7 @@ kimage_file_prepare_segments(struct kimage *image, int kernel_fd, int initrd_fd,
goto out;
}
- ima_kexec_cmdline(image->cmdline_buf,
+ ima_kexec_cmdline(kernel_fd, image->cmdline_buf,
image->cmdline_buf_len - 1);
}
diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index ea7e77536f3c..576ae2c6d418 100644
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h
@@ -265,7 +265,7 @@ void ima_store_measurement(struct integrity_iint_cache *iint, struct file *file,
struct evm_ima_xattr_data *xattr_value,
int xattr_len, const struct modsig *modsig, int pcr,
struct ima_template_desc *template_desc);
-void process_buffer_measurement(const void *buf, int size,
+void process_buffer_measurement(struct inode *inode, const void *buf, int size,
const char *eventname, enum ima_hooks func,
int pcr, const char *keyring);
void ima_audit_measurement(struct integrity_iint_cache *iint,
diff --git a/security/integrity/ima/ima_api.c b/security/integrity/ima/ima_api.c
index bf22de8b7ce0..4f39fb93f278 100644
--- a/security/integrity/ima/ima_api.c
+++ b/security/integrity/ima/ima_api.c
@@ -162,7 +162,7 @@ void ima_add_violation(struct file *file, const unsigned char *filename,
/**
* ima_get_action - appraise & measure decision based on policy.
- * @inode: pointer to inode to measure
+ * @inode: pointer to the inode associated with the object being validated
* @cred: pointer to credentials structure to validate
* @secid: secid of the task being validated
* @mask: contains the permission mask (MAY_READ, MAY_WRITE, MAY_EXEC,
diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c
index a9649b04b9f1..6c52bf7ea7f0 100644
--- a/security/integrity/ima/ima_appraise.c
+++ b/security/integrity/ima/ima_appraise.c
@@ -328,7 +328,7 @@ int ima_check_blacklist(struct integrity_iint_cache *iint,
rc = is_binary_blacklisted(digest, digestsize);
if ((rc == -EPERM) && (iint->flags & IMA_MEASURE))
- process_buffer_measurement(digest, digestsize,
+ process_buffer_measurement(NULL, digest, digestsize,
"blacklisted-hash", NONE,
pcr, NULL);
}
diff --git a/security/integrity/ima/ima_asymmetric_keys.c b/security/integrity/ima/ima_asymmetric_keys.c
index aaae80c4e376..1c68c500c26f 100644
--- a/security/integrity/ima/ima_asymmetric_keys.c
+++ b/security/integrity/ima/ima_asymmetric_keys.c
@@ -58,7 +58,7 @@ void ima_post_key_create_or_update(struct key *keyring, struct key *key,
* if the IMA policy is configured to measure a key linked
* to the given keyring.
*/
- process_buffer_measurement(payload, payload_len,
+ process_buffer_measurement(NULL, payload, payload_len,
keyring->description, KEY_CHECK, 0,
keyring->description);
}
diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
index 8351b2fd48e0..8a91711ca79b 100644
--- a/security/integrity/ima/ima_main.c
+++ b/security/integrity/ima/ima_main.c
@@ -726,6 +726,7 @@ int ima_load_data(enum kernel_load_data_id id)
/*
* process_buffer_measurement - Measure the buffer to ima log.
+ * @inode: inode associated with the object being measured (NULL for KEY_CHECK)
* @buf: pointer to the buffer that needs to be added to the log.
* @size: size of buffer(in bytes).
* @eventname: event name to be used for the buffer entry.
@@ -735,7 +736,7 @@ int ima_load_data(enum kernel_load_data_id id)
*
* Based on policy, the buffer is measured into the ima log.
*/
-void process_buffer_measurement(const void *buf, int size,
+void process_buffer_measurement(struct inode *inode, const void *buf, int size,
const char *eventname, enum ima_hooks func,
int pcr, const char *keyring)
{
@@ -768,7 +769,7 @@ void process_buffer_measurement(const void *buf, int size,
*/
if (func) {
security_task_getsecid(current, &secid);
- action = ima_get_action(NULL, current_cred(), secid, 0, func,
+ action = ima_get_action(inode, current_cred(), secid, 0, func,
&pcr, &template, keyring);
if (!(action & IMA_MEASURE))
return;
@@ -823,16 +824,26 @@ void process_buffer_measurement(const void *buf, int size,
/**
* ima_kexec_cmdline - measure kexec cmdline boot args
+ * @kernel_fd: file descriptor of the kexec kernel being loaded
* @buf: pointer to buffer
* @size: size of buffer
*
* Buffers can only be measured, not appraised.
*/
-void ima_kexec_cmdline(const void *buf, int size)
+void ima_kexec_cmdline(int kernel_fd, const void *buf, int size)
{
- if (buf && size != 0)
- process_buffer_measurement(buf, size, "kexec-cmdline",
- KEXEC_CMDLINE, 0, NULL);
+ struct fd f;
+
+ if (!buf || !size)
+ return;
+
+ f = fdget(kernel_fd);
+ if (!f.file)
+ return;
+
+ process_buffer_measurement(file_inode(f.file), buf, size,
+ "kexec-cmdline", KEXEC_CMDLINE, 0, NULL);
+ fdput(f);
}
static int __init init_ima(void)
diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
index 81ee8fd1d83a..2e87154c9296 100644
--- a/security/integrity/ima/ima_policy.c
+++ b/security/integrity/ima/ima_policy.c
@@ -443,13 +443,9 @@ static bool ima_match_rules(struct ima_rule_entry *rule, struct inode *inode,
{
int i;
- if ((func == KEXEC_CMDLINE) || (func == KEY_CHECK)) {
- if ((rule->flags & IMA_FUNC) && (rule->func == func)) {
- if (func == KEY_CHECK)
- return ima_match_keyring(rule, keyring, cred);
- return true;
- }
- return false;
+ if (func == KEY_CHECK) {
+ return (rule->flags & IMA_FUNC) && (rule->func == func) &&
+ ima_match_keyring(rule, keyring, cred);
}
if ((rule->flags & IMA_FUNC) &&
(rule->func != func && func != POST_SETATTR))
@@ -1035,10 +1031,9 @@ static bool ima_validate_rule(struct ima_rule_entry *entry)
if (entry->action & ~(MEASURE | DONT_MEASURE))
return false;
- if (entry->flags & ~(IMA_FUNC | IMA_PCR))
- return false;
-
- if (ima_rule_contains_lsm_cond(entry))
+ if (entry->flags & ~(IMA_FUNC | IMA_FSMAGIC | IMA_UID |
+ IMA_FOWNER | IMA_FSUUID | IMA_EUID |
+ IMA_PCR | IMA_FSNAME))
return false;
break;
diff --git a/security/integrity/ima/ima_queue_keys.c b/security/integrity/ima/ima_queue_keys.c
index 56ce24a18b66..69a8626a35c0 100644
--- a/security/integrity/ima/ima_queue_keys.c
+++ b/security/integrity/ima/ima_queue_keys.c
@@ -158,7 +158,7 @@ void ima_process_queued_keys(void)
list_for_each_entry_safe(entry, tmp, &ima_keys, list) {
if (!timer_expired)
- process_buffer_measurement(entry->payload,
+ process_buffer_measurement(NULL, entry->payload,
entry->payload_len,
entry->keyring_name,
KEY_CHECK, 0,
--
2.25.1
^ permalink raw reply related
* [PATCH v3 10/12] ima: Move comprehensive rule validation checks out of the token parser
From: Tyler Hicks @ 2020-07-09 6:19 UTC (permalink / raw)
To: Mimi Zohar, Dmitry Kasatkin
Cc: James Morris, Serge E . Hallyn, Lakshmi Ramasubramanian,
Prakhar Srivastava, linux-kernel, linux-integrity,
linux-security-module
In-Reply-To: <20200709061911.954326-1-tyhicks@linux.microsoft.com>
Use ima_validate_rule(), at the end of the token parsing stage, to
verify combinations of actions, hooks, and flags. This is useful to
increase readability by consolidating such checks into a single function
and also because rule conditionals can be specified in arbitrary order
making it difficult to do comprehensive rule validation until the entire
rule has been parsed.
This allows for the check that ties together the "keyrings" conditional
with the KEY_CHECK function hook to be moved into the final rule
validation.
The modsig check no longer needs to compiled conditionally because the
token parser will ensure that modsig support is enabled before accepting
"imasig|modsig" appraise type values. The final rule validation will
ensure that appraise_type and appraise_flag options are only present in
appraise rules.
Finally, this allows for the check that ties together the "pcr"
conditional with the measure action to be moved into the final rule
validation.
Signed-off-by: Tyler Hicks <tyhicks@linux.microsoft.com>
---
* v3
- Significant broadening of the patch's scope along with renaming and
re-describing the patch. ima_validate_rule() is now the consolidated
location for checking combinations of
actions/functions/conditionals and the existing checks are now
removed from the token parsing code.
+ Ensure that the IMA_FUNC flag is only set when a function hook is
specified, and vice versa, which allows us to use the NONE case in
the switch statement to enforce that "keyrings=",
"appraise_type=imasig|modsig", and "appraise_flag=blacklist"
cannot be specified on a rule without an appropriate hook function
- Adjust for the indentation change introduced in patch #4
- Adjust for new fixes introduced in patch #7
* v2
- Allowed IMA_DIGSIG_REQUIRED, IMA_PERMIT_DIRECTIO,
IMA_MODSIG_ALLOWED, and IMA_CHECK_BLACKLIST conditionals to be
present in the rule entry flags for non-buffer hook functions.
security/integrity/ima/ima.h | 6 ---
security/integrity/ima/ima_modsig.c | 20 ----------
security/integrity/ima/ima_policy.c | 57 +++++++++++++++++++----------
3 files changed, 37 insertions(+), 46 deletions(-)
diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index 59ec28f5c117..ea7e77536f3c 100644
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h
@@ -372,7 +372,6 @@ static inline int ima_read_xattr(struct dentry *dentry,
#endif /* CONFIG_IMA_APPRAISE */
#ifdef CONFIG_IMA_APPRAISE_MODSIG
-bool ima_hook_supports_modsig(enum ima_hooks func);
int ima_read_modsig(enum ima_hooks func, const void *buf, loff_t buf_len,
struct modsig **modsig);
void ima_collect_modsig(struct modsig *modsig, const void *buf, loff_t size);
@@ -382,11 +381,6 @@ int ima_get_raw_modsig(const struct modsig *modsig, const void **data,
u32 *data_len);
void ima_free_modsig(struct modsig *modsig);
#else
-static inline bool ima_hook_supports_modsig(enum ima_hooks func)
-{
- return false;
-}
-
static inline int ima_read_modsig(enum ima_hooks func, const void *buf,
loff_t buf_len, struct modsig **modsig)
{
diff --git a/security/integrity/ima/ima_modsig.c b/security/integrity/ima/ima_modsig.c
index d106885cc495..fb25723c65bc 100644
--- a/security/integrity/ima/ima_modsig.c
+++ b/security/integrity/ima/ima_modsig.c
@@ -32,26 +32,6 @@ struct modsig {
u8 raw_pkcs7[];
};
-/**
- * ima_hook_supports_modsig - can the policy allow modsig for this hook?
- *
- * modsig is only supported by hooks using ima_post_read_file(), because only
- * they preload the contents of the file in a buffer. FILE_CHECK does that in
- * some cases, but not when reached from vfs_open(). POLICY_CHECK can support
- * it, but it's not useful in practice because it's a text file so deny.
- */
-bool ima_hook_supports_modsig(enum ima_hooks func)
-{
- switch (func) {
- case KEXEC_KERNEL_CHECK:
- case KEXEC_INITRAMFS_CHECK:
- case MODULE_CHECK:
- return true;
- default:
- return false;
- }
-}
-
/*
* ima_read_modsig - Read modsig from buf.
*
diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
index 13a178c70b44..c4d0a0c1f896 100644
--- a/security/integrity/ima/ima_policy.c
+++ b/security/integrity/ima/ima_policy.c
@@ -984,10 +984,27 @@ static void check_template_modsig(const struct ima_template_desc *template)
static bool ima_validate_rule(struct ima_rule_entry *entry)
{
- /* Ensure that the action is set */
+ /* Ensure that the action is set and is compatible with the flags */
if (entry->action == UNKNOWN)
return false;
+ if (entry->action != MEASURE && entry->flags & IMA_PCR)
+ return false;
+
+ if (entry->action != APPRAISE &&
+ entry->flags & (IMA_DIGSIG_REQUIRED | IMA_MODSIG_ALLOWED | IMA_CHECK_BLACKLIST))
+ return false;
+
+ /*
+ * The IMA_FUNC bit must be set if and only if there's a valid hook
+ * function specified, and vice versa. Enforcing this property allows
+ * for the NONE case below to validate a rule without an explicit hook
+ * function.
+ */
+ if (((entry->flags & IMA_FUNC) && entry->func == NONE) ||
+ (!(entry->flags & IMA_FUNC) && entry->func != NONE))
+ return false;
+
/*
* Ensure that the hook function is compatible with the other
* components of the rule
@@ -999,12 +1016,27 @@ static bool ima_validate_rule(struct ima_rule_entry *entry)
case BPRM_CHECK:
case CREDS_CHECK:
case POST_SETATTR:
- case MODULE_CHECK:
case FIRMWARE_CHECK:
+ case POLICY_CHECK:
+ if (entry->flags & ~(IMA_FUNC | IMA_MASK | IMA_FSMAGIC |
+ IMA_UID | IMA_FOWNER | IMA_FSUUID |
+ IMA_INMASK | IMA_EUID | IMA_PCR |
+ IMA_FSNAME | IMA_DIGSIG_REQUIRED |
+ IMA_PERMIT_DIRECTIO))
+ return false;
+
+ break;
+ case MODULE_CHECK:
case KEXEC_KERNEL_CHECK:
case KEXEC_INITRAMFS_CHECK:
- case POLICY_CHECK:
- /* Validation of these hook functions is in ima_parse_rule() */
+ if (entry->flags & ~(IMA_FUNC | IMA_MASK | IMA_FSMAGIC |
+ IMA_UID | IMA_FOWNER | IMA_FSUUID |
+ IMA_INMASK | IMA_EUID | IMA_PCR |
+ IMA_FSNAME | IMA_DIGSIG_REQUIRED |
+ IMA_PERMIT_DIRECTIO | IMA_MODSIG_ALLOWED |
+ IMA_CHECK_BLACKLIST))
+ return false;
+
break;
case KEXEC_CMDLINE:
if (entry->action & ~(MEASURE | DONT_MEASURE))
@@ -1218,7 +1250,6 @@ static int ima_parse_rule(char *rule, struct ima_rule_entry *entry)
keyrings_len = strlen(args[0].from) + 1;
if ((entry->keyrings) ||
- (entry->func != KEY_CHECK) ||
(keyrings_len < 2)) {
result = -EINVAL;
break;
@@ -1358,15 +1389,10 @@ static int ima_parse_rule(char *rule, struct ima_rule_entry *entry)
AUDIT_SUBJ_TYPE);
break;
case Opt_appraise_type:
- if (entry->action != APPRAISE) {
- result = -EINVAL;
- break;
- }
-
ima_log_string(ab, "appraise_type", args[0].from);
if ((strcmp(args[0].from, "imasig")) == 0)
entry->flags |= IMA_DIGSIG_REQUIRED;
- else if (ima_hook_supports_modsig(entry->func) &&
+ else if (IS_ENABLED(CONFIG_IMA_APPRAISE_MODSIG) &&
strcmp(args[0].from, "imasig|modsig") == 0)
entry->flags |= IMA_DIGSIG_REQUIRED |
IMA_MODSIG_ALLOWED;
@@ -1374,11 +1400,6 @@ static int ima_parse_rule(char *rule, struct ima_rule_entry *entry)
result = -EINVAL;
break;
case Opt_appraise_flag:
- if (entry->action != APPRAISE) {
- result = -EINVAL;
- break;
- }
-
ima_log_string(ab, "appraise_flag", args[0].from);
if (IS_ENABLED(CONFIG_IMA_APPRAISE_MODSIG) &&
strstr(args[0].from, "blacklist"))
@@ -1388,10 +1409,6 @@ static int ima_parse_rule(char *rule, struct ima_rule_entry *entry)
entry->flags |= IMA_PERMIT_DIRECTIO;
break;
case Opt_pcr:
- if (entry->action != MEASURE) {
- result = -EINVAL;
- break;
- }
ima_log_string(ab, "pcr", args[0].from);
result = kstrtoint(args[0].from, 10, &entry->pcr);
--
2.25.1
^ permalink raw reply related
* [PATCH v3 09/12] ima: Use correct type for the args_p member of ima_rule_entry.lsm elements
From: Tyler Hicks @ 2020-07-09 6:19 UTC (permalink / raw)
To: Mimi Zohar, Dmitry Kasatkin
Cc: James Morris, Serge E . Hallyn, Lakshmi Ramasubramanian,
Prakhar Srivastava, linux-kernel, linux-integrity,
linux-security-module
In-Reply-To: <20200709061911.954326-1-tyhicks@linux.microsoft.com>
Make args_p be of the char pointer type rather than have it be a void
pointer that gets casted to char pointer when it is used. It is a simple
NUL-terminated string as returned by match_strdup().
Signed-off-by: Tyler Hicks <tyhicks@linux.microsoft.com>
---
* v3
- No change
* v2
- No change
security/integrity/ima/ima_policy.c | 18 +++++++++---------
1 file changed, 9 insertions(+), 9 deletions(-)
diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
index b02e1ffd10c9..13a178c70b44 100644
--- a/security/integrity/ima/ima_policy.c
+++ b/security/integrity/ima/ima_policy.c
@@ -74,7 +74,7 @@ struct ima_rule_entry {
int pcr;
struct {
void *rule; /* LSM file metadata specific */
- void *args_p; /* audit value */
+ char *args_p; /* audit value */
int type; /* audit type */
} lsm[MAX_LSM_RULES];
char *fsname;
@@ -314,7 +314,7 @@ static struct ima_rule_entry *ima_lsm_copy_rule(struct ima_rule_entry *entry)
&nentry->lsm[i].rule);
if (!nentry->lsm[i].rule)
pr_warn("rule for LSM \'%s\' is undefined\n",
- (char *)entry->lsm[i].args_p);
+ entry->lsm[i].args_p);
}
return nentry;
}
@@ -918,7 +918,7 @@ static int ima_lsm_rule_init(struct ima_rule_entry *entry,
&entry->lsm[lsm_rule].rule);
if (!entry->lsm[lsm_rule].rule) {
pr_warn("rule for LSM \'%s\' is undefined\n",
- (char *)entry->lsm[lsm_rule].args_p);
+ entry->lsm[lsm_rule].args_p);
if (ima_rules == &ima_default_rules) {
kfree(entry->lsm[lsm_rule].args_p);
@@ -1682,27 +1682,27 @@ int ima_policy_show(struct seq_file *m, void *v)
switch (i) {
case LSM_OBJ_USER:
seq_printf(m, pt(Opt_obj_user),
- (char *)entry->lsm[i].args_p);
+ entry->lsm[i].args_p);
break;
case LSM_OBJ_ROLE:
seq_printf(m, pt(Opt_obj_role),
- (char *)entry->lsm[i].args_p);
+ entry->lsm[i].args_p);
break;
case LSM_OBJ_TYPE:
seq_printf(m, pt(Opt_obj_type),
- (char *)entry->lsm[i].args_p);
+ entry->lsm[i].args_p);
break;
case LSM_SUBJ_USER:
seq_printf(m, pt(Opt_subj_user),
- (char *)entry->lsm[i].args_p);
+ entry->lsm[i].args_p);
break;
case LSM_SUBJ_ROLE:
seq_printf(m, pt(Opt_subj_role),
- (char *)entry->lsm[i].args_p);
+ entry->lsm[i].args_p);
break;
case LSM_SUBJ_TYPE:
seq_printf(m, pt(Opt_subj_type),
- (char *)entry->lsm[i].args_p);
+ entry->lsm[i].args_p);
break;
}
seq_puts(m, " ");
--
2.25.1
^ permalink raw reply related
* [PATCH v3 11/12] ima: Use the common function to detect LSM conditionals in a rule
From: Tyler Hicks @ 2020-07-09 6:19 UTC (permalink / raw)
To: Mimi Zohar, Dmitry Kasatkin
Cc: James Morris, Serge E . Hallyn, Lakshmi Ramasubramanian,
Prakhar Srivastava, linux-kernel, linux-integrity,
linux-security-module
In-Reply-To: <20200709061911.954326-1-tyhicks@linux.microsoft.com>
Make broader use of ima_rule_contains_lsm_cond() to check if a given
rule contains an LSM conditional. This is a code cleanup and has no
user-facing change.
Signed-off-by: Tyler Hicks <tyhicks@linux.microsoft.com>
Reviewed-by: Mimi Zohar <zohar@linux.ibm.com>
---
* v3
- No change
* v2
- No change
security/integrity/ima/ima_policy.c | 11 ++---------
1 file changed, 2 insertions(+), 9 deletions(-)
diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
index c4d0a0c1f896..81ee8fd1d83a 100644
--- a/security/integrity/ima/ima_policy.c
+++ b/security/integrity/ima/ima_policy.c
@@ -360,17 +360,10 @@ static bool ima_rule_contains_lsm_cond(struct ima_rule_entry *entry)
static void ima_lsm_update_rules(void)
{
struct ima_rule_entry *entry, *e;
- int i, result, needs_update;
+ int result;
list_for_each_entry_safe(entry, e, &ima_policy_rules, list) {
- needs_update = 0;
- for (i = 0; i < MAX_LSM_RULES; i++) {
- if (entry->lsm[i].args_p) {
- needs_update = 1;
- break;
- }
- }
- if (!needs_update)
+ if (!ima_rule_contains_lsm_cond(entry))
continue;
result = ima_lsm_update_rule(entry);
--
2.25.1
^ permalink raw reply related
* [PATCH v3 08/12] ima: Shallow copy the args_p member of ima_rule_entry.lsm elements
From: Tyler Hicks @ 2020-07-09 6:19 UTC (permalink / raw)
To: Mimi Zohar, Dmitry Kasatkin
Cc: James Morris, Serge E . Hallyn, Lakshmi Ramasubramanian,
Prakhar Srivastava, linux-kernel, linux-integrity,
linux-security-module
In-Reply-To: <20200709061911.954326-1-tyhicks@linux.microsoft.com>
The args_p member is a simple string that is allocated by
ima_rule_init(). Shallow copy it like other non-LSM references in
ima_rule_entry structs.
There are no longer any necessary error path cleanups to do in
ima_lsm_copy_rule().
Signed-off-by: Tyler Hicks <tyhicks@linux.microsoft.com>
---
* v3
- No change
* v2
- Adjusted context to account for ima_lsm_copy_rule() directly calling
ima_lsm_free_rule() and the lack of explicit reference ownership
transfers
- Added comment to ima_lsm_copy_rule() to document the args_p
reference ownership transfer
security/integrity/ima/ima_policy.c | 16 +++++++---------
1 file changed, 7 insertions(+), 9 deletions(-)
diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
index 9842e2e0bc6d..b02e1ffd10c9 100644
--- a/security/integrity/ima/ima_policy.c
+++ b/security/integrity/ima/ima_policy.c
@@ -300,10 +300,13 @@ static struct ima_rule_entry *ima_lsm_copy_rule(struct ima_rule_entry *entry)
continue;
nentry->lsm[i].type = entry->lsm[i].type;
- nentry->lsm[i].args_p = kstrdup(entry->lsm[i].args_p,
- GFP_KERNEL);
- if (!nentry->lsm[i].args_p)
- goto out_err;
+ nentry->lsm[i].args_p = entry->lsm[i].args_p;
+ /*
+ * Remove the reference from entry so that the associated
+ * memory will not be freed during a later call to
+ * ima_lsm_free_rule(entry).
+ */
+ entry->lsm[i].args_p = NULL;
security_filter_rule_init(nentry->lsm[i].type,
Audit_equal,
@@ -314,11 +317,6 @@ static struct ima_rule_entry *ima_lsm_copy_rule(struct ima_rule_entry *entry)
(char *)entry->lsm[i].args_p);
}
return nentry;
-
-out_err:
- ima_lsm_free_rule(nentry);
- kfree(nentry);
- return NULL;
}
static int ima_lsm_update_rule(struct ima_rule_entry *entry)
--
2.25.1
^ permalink raw reply related
* [PATCH v3 06/12] ima: Fail rule parsing when the KEY_CHECK hook is combined with an invalid cond
From: Tyler Hicks @ 2020-07-09 6:19 UTC (permalink / raw)
To: Mimi Zohar, Dmitry Kasatkin
Cc: James Morris, Serge E . Hallyn, Lakshmi Ramasubramanian,
Prakhar Srivastava, linux-kernel, linux-integrity,
linux-security-module
In-Reply-To: <20200709061911.954326-1-tyhicks@linux.microsoft.com>
The KEY_CHECK function only supports the uid, pcr, and keyrings
conditionals. Make this clear at policy load so that IMA policy authors
don't assume that other conditionals are supported.
Fixes: 5808611cccb2 ("IMA: Add KEY_CHECK func to measure keys")
Signed-off-by: Tyler Hicks <tyhicks@linux.microsoft.com>
Reviewed-by: Lakshmi Ramasubramanian <nramas@linux.microsoft.com>
---
* v3
- Added Lakshmi's Reviewed-by
- Adjust for the indentation change introduced in patch #4
* v2
- No change
security/integrity/ima/ima_policy.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
index 1c64bd6f1728..81da02071d41 100644
--- a/security/integrity/ima/ima_policy.c
+++ b/security/integrity/ima/ima_policy.c
@@ -1023,6 +1023,13 @@ static bool ima_validate_rule(struct ima_rule_entry *entry)
if (entry->action & ~(MEASURE | DONT_MEASURE))
return false;
+ if (entry->flags & ~(IMA_FUNC | IMA_UID | IMA_PCR |
+ IMA_KEYRINGS))
+ return false;
+
+ if (ima_rule_contains_lsm_cond(entry))
+ return false;
+
break;
default:
return false;
--
2.25.1
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox