* [PATCH 1/4] KEYS: trusted: allow users to use kernel RNG for key material
From: Ahmad Fatoum @ 2021-07-21 16:48 UTC (permalink / raw)
To: James Bottomley, Jarkko Sakkinen, Mimi Zohar, David Howells
Cc: kernel, Ahmad Fatoum, James Morris, Serge E. Hallyn,
Horia Geantă, Aymen Sghaier, Herbert Xu, David S. Miller,
Udit Agarwal, Jan Luebbe, Eric Biggers, David Gstir,
Richard Weinberger, Franck LENORMAND, Sumit Garg, keyrings,
linux-crypto, linux-integrity, linux-kernel,
linux-security-module
In-Reply-To: <cover.9fc9298fd9d63553491871d043a18affc2dbc8a8.1626885907.git-series.a.fatoum@pengutronix.de>
The two existing trusted key sources don't make use of the kernel RNG,
but instead let the hardware doing the sealing/unsealing also
generate the random key material. However, Users may want to place
less trust into the quality of the trust source's random number
generator and instead use the kernel entropy pool, which can be
seeded from multiple entropy sources.
Make this possible by adding a new trusted.kernel_rng parameter,
that will force use of the kernel RNG. In its absence, it's up
to the trust source to decide, which random numbers to use,
maintaining the existing behavior.
Suggested-by: Jarkko Sakkinen <jarkko@kernel.org>
Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
To: James Bottomley <jejb@linux.ibm.com>
To: Jarkko Sakkinen <jarkko@kernel.org>
To: Mimi Zohar <zohar@linux.ibm.com>
To: David Howells <dhowells@redhat.com>
Cc: James Morris <jmorris@namei.org>
Cc: "Serge E. Hallyn" <serge@hallyn.com>
Cc: "Horia Geantă" <horia.geanta@nxp.com>
Cc: Aymen Sghaier <aymen.sghaier@nxp.com>
Cc: Herbert Xu <herbert@gondor.apana.org.au>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Udit Agarwal <udit.agarwal@nxp.com>
Cc: Jan Luebbe <j.luebbe@pengutronix.de>
Cc: Eric Biggers <ebiggers@kernel.org>
Cc: David Gstir <david@sigma-star.at>
Cc: Richard Weinberger <richard@nod.at>
Cc: Franck LENORMAND <franck.lenormand@nxp.com>
Cc: Sumit Garg <sumit.garg@linaro.org>
Cc: keyrings@vger.kernel.org
Cc: linux-crypto@vger.kernel.org
Cc: linux-integrity@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Cc: linux-security-module@vger.kernel.org
---
Documentation/admin-guide/kernel-parameters.txt | 7 ++++++-
Documentation/security/keys/trusted-encrypted.rst | 20 +++++++++-------
security/keys/trusted-keys/trusted_core.c | 17 +++++++++++++-
3 files changed, 35 insertions(+), 9 deletions(-)
diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index bdb22006f713..0267ead88902 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -5734,6 +5734,13 @@
first trust source as a backend which is initialized
successfully during iteration.
+ trusted.kernel_rng = [KEYS]
+ Format: <bool>
+ When set to true (1), the kernel random number pool
+ is used to generate key material for trusted keys.
+ The default is to leave the RNG's choice to each
+ individual trust source.
+
tsc= Disable clocksource stability checks for TSC.
Format: <string>
[x86] reliable: mark tsc clocksource as reliable, this
diff --git a/Documentation/security/keys/trusted-encrypted.rst b/Documentation/security/keys/trusted-encrypted.rst
index 80d5a5af62a1..1d4b4b8f12f0 100644
--- a/Documentation/security/keys/trusted-encrypted.rst
+++ b/Documentation/security/keys/trusted-encrypted.rst
@@ -87,22 +87,26 @@ Key Generation
Trusted Keys
------------
-New keys are created from random numbers generated in the trust source. They
-are encrypted/decrypted using a child key in the storage key hierarchy.
-Encryption and decryption of the child key must be protected by a strong
-access control policy within the trust source.
+New keys are created from random numbers. They are encrypted/decrypted using
+a child key in the storage key hierarchy. Encryption and decryption of the
+child key must be protected by a strong access control policy within the
+trust source. The random number generator in use differs according to the
+selected trust source:
- * TPM (hardware device) based RNG
+ * TPM: hardware device based RNG
- Strength of random numbers may vary from one device manufacturer to
- another.
+ Keys are generated within the TPM. Strength of random numbers may vary
+ from one device manufacturer to another.
- * TEE (OP-TEE based on Arm TrustZone) based RNG
+ * TEE: OP-TEE based on Arm TrustZone based RNG
RNG is customizable as per platform needs. It can either be direct output
from platform specific hardware RNG or a software based Fortuna CSPRNG
which can be seeded via multiple entropy sources.
+Optionally, users may specify ``trusted.kernel_rng=1`` on the kernel
+command-line to override the used RNG with the kernel's random number pool.
+
Encrypted Keys
--------------
diff --git a/security/keys/trusted-keys/trusted_core.c b/security/keys/trusted-keys/trusted_core.c
index 8cab69e5d0da..569af9af8df0 100644
--- a/security/keys/trusted-keys/trusted_core.c
+++ b/security/keys/trusted-keys/trusted_core.c
@@ -16,12 +16,17 @@
#include <linux/key-type.h>
#include <linux/module.h>
#include <linux/parser.h>
+#include <linux/random.h>
#include <linux/rcupdate.h>
#include <linux/slab.h>
#include <linux/static_call.h>
#include <linux/string.h>
#include <linux/uaccess.h>
+static bool trusted_kernel_rng;
+module_param_named(kernel_rng, trusted_kernel_rng, bool, 0);
+MODULE_PARM_DESC(kernel_rng, "Generate key material from kernel RNG");
+
static char *trusted_key_source;
module_param_named(source, trusted_key_source, charp, 0);
MODULE_PARM_DESC(source, "Select trusted keys source (tpm or tee)");
@@ -312,8 +317,14 @@ struct key_type key_type_trusted = {
};
EXPORT_SYMBOL_GPL(key_type_trusted);
+static int kernel_get_random(unsigned char *key, size_t key_len)
+{
+ return get_random_bytes_wait(key, key_len) ?: key_len;
+}
+
static int __init init_trusted(void)
{
+ int (*get_random)(unsigned char *key, size_t key_len);
int i, ret = 0;
for (i = 0; i < ARRAY_SIZE(trusted_key_sources); i++) {
@@ -322,6 +333,10 @@ static int __init init_trusted(void)
strlen(trusted_key_sources[i].name)))
continue;
+ get_random = trusted_key_sources[i].ops->get_random;
+ if (trusted_kernel_rng)
+ get_random = kernel_get_random;
+
static_call_update(trusted_key_init,
trusted_key_sources[i].ops->init);
static_call_update(trusted_key_seal,
@@ -329,7 +344,7 @@ static int __init init_trusted(void)
static_call_update(trusted_key_unseal,
trusted_key_sources[i].ops->unseal);
static_call_update(trusted_key_get_random,
- trusted_key_sources[i].ops->get_random);
+ get_random);
static_call_update(trusted_key_exit,
trusted_key_sources[i].ops->exit);
migratable = trusted_key_sources[i].ops->migratable;
--
git-series 0.9.1
^ permalink raw reply related
* [PATCH 0/4] KEYS: trusted: Introduce support for NXP CAAM-based trusted keys
From: Ahmad Fatoum @ 2021-07-21 16:48 UTC (permalink / raw)
To: Jarkko Sakkinen, Horia Geantă, Mimi Zohar, Aymen Sghaier,
Herbert Xu, David S. Miller, James Bottomley
Cc: kernel, David Howells, James Morris, Serge E. Hallyn,
Steffen Trumtrar, Udit Agarwal, Jan Luebbe, David Gstir,
Eric Biggers, Richard Weinberger, Franck LENORMAND, Sumit Garg,
linux-integrity, keyrings, linux-crypto, linux-kernel,
linux-security-module
Series applies on top of
https://lore.kernel.org/linux-integrity/20210721160258.7024-1-a.fatoum@pengutronix.de/T/#u
v2 -> v3:
- Split off first Kconfig preparation patch. It fixes a regression,
so sent that out, so it can be applied separately (Sumit)
- Split off second key import patch. I'll send that out separately
as it's a development aid and not required within the CAAM series
- add MAINTAINERS entry
v1 -> v2:
- Added new commit to make trusted key Kconfig option independent
of TPM and added new Kconfig file for trusted keys
- Add new commit for importing existing key material
- Allow users to force use of kernel RNG (Jarkko)
- Enforce maximum keymod size (Horia)
- Use append_seq_(in|out)_ptr_intlen instead of append_seq_(in|out)_ptr
(Horia)
- Make blobifier handle private to CAAM glue code file (Horia)
- Extend trusted keys documentation for CAAM
- Rebased and updated original cover letter:
The Cryptographic Acceleration and Assurance Module (CAAM) is an IP core
built into many newer i.MX and QorIQ SoCs by NXP.
Its blob mechanism can AES encrypt/decrypt user data using a unique
never-disclosed device-specific key.
There has been multiple discussions on how to represent this within the kernel:
The Cryptographic Acceleration and Assurance Module (CAAM) is an IP core
built into many newer i.MX and QorIQ SoCs by NXP.
Its blob mechanism can AES encrypt/decrypt user data using a unique
never-disclosed device-specific key. There has been multiple
discussions on how to represent this within the kernel:
- [RFC] crypto: caam - add red blobifier
Steffen implemented[1] a PoC sysfs driver to start a discussion on how to
best integrate the blob mechanism.
Mimi suggested that it could be used to implement trusted keys.
Trusted keys back then were a TPM-only feature.
- security/keys/secure_key: Adds the secure key support based on CAAM.
Udit added[2] a new "secure" key type with the CAAM as backend. The key
material stays within the kernel only.
Mimi and James agreed that this needs a generic interface, not specific
to CAAM. Mimi suggested trusted keys. Jan noted that this could serve as
basis for TEE-backed keys.
- [RFC] drivers: crypto: caam: key: Add caam_tk key type
Franck added[3] a new "caam_tk" key type based on Udit's work. This time
it uses CAAM "black blobs" instead of "red blobs", so key material stays
within the CAAM and isn't exposed to kernel in plaintext.
James voiced the opinion that there should be just one user-facing generic
wrap/unwrap key type with multiple possible handlers.
David suggested trusted keys.
- Introduce TEE based Trusted Keys support
Sumit reworked[4] trusted keys to support multiple possible backends with
one chosen at boot time and added a new TEE backend along with TPM.
This now sits in Jarkko's master branch to be sent out for v5.13
This patch series builds on top of Sumit's rework to have the CAAM as yet another
trusted key backend.
The CAAM bits are based on Steffen's initial patch from 2015. His work had been
used in the field for some years now, so I preferred not to deviate too much from it.
This series has been tested with dmcrypt[5] on an i.MX6DL.
Looking forward to your feedback.
Cheers,
Ahmad
[1]: https://lore.kernel.org/linux-crypto/1447082306-19946-2-git-send-email-s.trumtrar@pengutronix.de/
[2]: https://lore.kernel.org/linux-integrity/20180723111432.26830-1-udit.agarwal@nxp.com/
[3]: https://lore.kernel.org/lkml/1551456599-10603-2-git-send-email-franck.lenormand@nxp.com/
[4]: https://lore.kernel.org/lkml/1604419306-26105-1-git-send-email-sumit.garg@linaro.org/
[5]: https://lore.kernel.org/linux-integrity/20210122084321.24012-2-a.fatoum@pengutronix.de/
---
To: Jarkko Sakkinen <jarkko@kernel.org>
To: "Horia Geantă" <horia.geanta@nxp.com>
To: Mimi Zohar <zohar@linux.ibm.com>
To: Aymen Sghaier <aymen.sghaier@nxp.com>
To: Herbert Xu <herbert@gondor.apana.org.au>
To: "David S. Miller" <davem@davemloft.net>
To: James Bottomley <jejb@linux.ibm.com>
Cc: David Howells <dhowells@redhat.com>
Cc: James Morris <jmorris@namei.org>
Cc: "Serge E. Hallyn" <serge@hallyn.com>
Cc: Steffen Trumtrar <s.trumtrar@pengutronix.de>
Cc: Udit Agarwal <udit.agarwal@nxp.com>
Cc: Jan Luebbe <j.luebbe@pengutronix.de>
Cc: David Gstir <david@sigma-star.at>
Cc: Eric Biggers <ebiggers@kernel.org>
Cc: Richard Weinberger <richard@nod.at>
Cc: Franck LENORMAND <franck.lenormand@nxp.com>
Cc: Sumit Garg <sumit.garg@linaro.org>
Cc: linux-integrity@vger.kernel.org
Cc: keyrings@vger.kernel.org
Cc: linux-crypto@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Cc: linux-security-module@vger.kernel.org
Ahmad Fatoum (4):
KEYS: trusted: allow users to use kernel RNG for key material
KEYS: trusted: allow trust sources to use kernel RNG for key material
crypto: caam - add in-kernel interface for blob generator
KEYS: trusted: Introduce support for NXP CAAM-based trusted keys
Documentation/admin-guide/kernel-parameters.txt | 8 +-
Documentation/security/keys/trusted-encrypted.rst | 60 +++-
MAINTAINERS | 9 +-
drivers/crypto/caam/Kconfig | 3 +-
drivers/crypto/caam/Makefile | 1 +-
drivers/crypto/caam/blob_gen.c | 230 +++++++++++++++-
include/keys/trusted-type.h | 2 +-
include/keys/trusted_caam.h | 11 +-
include/soc/fsl/caam-blob.h | 56 ++++-
security/keys/trusted-keys/Kconfig | 11 +-
security/keys/trusted-keys/Makefile | 2 +-
security/keys/trusted-keys/trusted_caam.c | 74 +++++-
security/keys/trusted-keys/trusted_core.c | 23 +-
13 files changed, 477 insertions(+), 13 deletions(-)
create mode 100644 drivers/crypto/caam/blob_gen.c
create mode 100644 include/keys/trusted_caam.h
create mode 100644 include/soc/fsl/caam-blob.h
create mode 100644 security/keys/trusted-keys/trusted_caam.c
base-commit: 97408d81ed533b953326c580ff2c3f1948b3fcee
--
git-series 0.9.1
^ permalink raw reply
* [PATCH RFC 6/9] veth: use skb_prepare_for_gro()
From: Paolo Abeni @ 2021-07-21 16:44 UTC (permalink / raw)
To: netdev
Cc: David S. Miller, Jakub Kicinski, Florian Westphal, Eric Dumazet,
linux-security-module, selinux
In-Reply-To: <cover.1626882513.git.pabeni@redhat.com>
Leveraging the previous patch we can now avoid orphaning the
skb in the veth gro path, allowing correct backpressure.
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
---
drivers/net/veth.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/veth.c b/drivers/net/veth.c
index 381670c08ba7..50eb43e5bf45 100644
--- a/drivers/net/veth.c
+++ b/drivers/net/veth.c
@@ -713,7 +713,7 @@ static struct sk_buff *veth_xdp_rcv_skb(struct veth_rq *rq,
int mac_len, delta, off;
struct xdp_buff xdp;
- skb_orphan_partial(skb);
+ skb_prepare_for_gro(skb);
rcu_read_lock();
xdp_prog = rcu_dereference(rq->xdp_prog);
--
2.26.3
^ permalink raw reply related
* [PATCH RFC 7/9] sk_buff: move inner header fields after tail
From: Paolo Abeni @ 2021-07-21 16:44 UTC (permalink / raw)
To: netdev
Cc: David S. Miller, Jakub Kicinski, Florian Westphal, Eric Dumazet,
linux-security-module, selinux
In-Reply-To: <cover.1626882513.git.pabeni@redhat.com>
all the inner header fields are valid only if the 'encaspulation'
flag is set, and the relevant fields are always initialized when
the field is set: we don't need to initialize them at skb allocation
time
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
---
v1 -> v2:
- add CHECK_SKB_FIELD(__encapsulation_offset) in __copy_skb_header
---
include/linux/skbuff.h | 31 ++++++++++++++++++++++---------
net/core/skbuff.c | 6 ++----
2 files changed, 24 insertions(+), 13 deletions(-)
diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index ea9fdcc7c7ca..a3e756575aa7 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -822,6 +822,9 @@ struct sk_buff {
__u8 ip_summed:2;
__u8 ooo_okay:1;
+ /* private: */
+ __u8 __pkt_encapsulation_offset[0];
+ /* public: */
__u8 l4_hash:1;
__u8 sw_hash:1;
__u8 wifi_acked_valid:1;
@@ -911,15 +914,6 @@ struct sk_buff {
__u32 reserved_tailroom;
};
- union {
- __be16 inner_protocol;
- __u8 inner_ipproto;
- };
-
- __u16 inner_transport_header;
- __u16 inner_network_header;
- __u16 inner_mac_header;
-
__be16 protocol;
__u16 transport_header;
__u16 network_header;
@@ -948,6 +942,19 @@ struct sk_buff {
#if defined(CONFIG_NF_CONNTRACK) || defined(CONFIG_NF_CONNTRACK_MODULE)
unsigned long _nfct;
#endif
+ union {
+ struct {
+ union {
+ __be16 inner_protocol;
+ __u8 inner_ipproto;
+ };
+
+ __u16 inner_transport_header;
+ __u16 inner_network_header;
+ __u16 inner_mac_header;
+ };
+ __u64 inner_headers;
+ };
};
#ifdef __KERNEL__
@@ -2449,6 +2456,12 @@ static inline void skb_tailroom_reserve(struct sk_buff *skb, unsigned int mtu,
#define ENCAP_TYPE_ETHER 0
#define ENCAP_TYPE_IPPROTO 1
+static inline void __skb_copy_inner_headers(struct sk_buff *dst, const struct sk_buff *src)
+{
+ if (src->encapsulation)
+ dst->inner_headers = src->inner_headers;
+}
+
static inline void skb_set_inner_protocol(struct sk_buff *skb,
__be16 protocol)
{
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index 9ed754da6e13..53b8db10e567 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -995,6 +995,7 @@ static void __copy_skb_header(struct sk_buff *new, const struct sk_buff *old)
skb_dst_copy(new, old);
__skb_ext_copy(new, old);
__nf_copy(new, old, false);
+ __skb_copy_inner_headers(new, old);
/* Note : this field could be in headers_start/headers_end section
* It is not yet because we do not want to have a 16 bit hole
@@ -1005,6 +1006,7 @@ static void __copy_skb_header(struct sk_buff *new, const struct sk_buff *old)
offsetof(struct sk_buff, headers_end) -
offsetof(struct sk_buff, headers_start));
CHECK_SKB_FIELD(_state);
+ CHECK_SKB_FIELD(__pkt_encapsulation_offset);
CHECK_SKB_FIELD(protocol);
CHECK_SKB_FIELD(csum);
CHECK_SKB_FIELD(hash);
@@ -1015,10 +1017,6 @@ static void __copy_skb_header(struct sk_buff *new, const struct sk_buff *old)
CHECK_SKB_FIELD(transport_header);
CHECK_SKB_FIELD(network_header);
CHECK_SKB_FIELD(mac_header);
- CHECK_SKB_FIELD(inner_protocol);
- CHECK_SKB_FIELD(inner_transport_header);
- CHECK_SKB_FIELD(inner_network_header);
- CHECK_SKB_FIELD(inner_mac_header);
CHECK_SKB_FIELD(mark);
#ifdef CONFIG_NETWORK_SECMARK
CHECK_SKB_FIELD(secmark);
--
2.26.3
^ permalink raw reply related
* [PATCH RFC 8/9] sk_buff: move vlan field after tail.
From: Paolo Abeni @ 2021-07-21 16:44 UTC (permalink / raw)
To: netdev
Cc: David S. Miller, Jakub Kicinski, Florian Westphal, Eric Dumazet,
linux-security-module, selinux
In-Reply-To: <cover.1626882513.git.pabeni@redhat.com>
Such field validity is already tracked by the existing
'vlan_present' bit. Move them after tail and conditinally copy
as needed.
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
---
include/linux/skbuff.h | 10 ++++++++--
net/core/skbuff.c | 5 +++--
2 files changed, 11 insertions(+), 4 deletions(-)
diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index a3e756575aa7..7acf2a203918 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -897,8 +897,6 @@ struct sk_buff {
__u32 priority;
int skb_iif;
__u32 hash;
- __be16 vlan_proto;
- __u16 vlan_tci;
#if defined(CONFIG_NET_RX_BUSY_POLL) || defined(CONFIG_XPS)
union {
unsigned int napi_id;
@@ -955,6 +953,14 @@ struct sk_buff {
};
__u64 inner_headers;
};
+
+ union {
+ struct {
+ __be16 vlan_proto;
+ __u16 vlan_tci;
+ };
+ __u32 vlan_info;
+ };
};
#ifdef __KERNEL__
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index 53b8db10e567..c59e90db80d5 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -996,6 +996,8 @@ static void __copy_skb_header(struct sk_buff *new, const struct sk_buff *old)
__skb_ext_copy(new, old);
__nf_copy(new, old, false);
__skb_copy_inner_headers(new, old);
+ if (old->vlan_present)
+ new->vlan_info = old->vlan_info;
/* Note : this field could be in headers_start/headers_end section
* It is not yet because we do not want to have a 16 bit hole
@@ -1007,13 +1009,12 @@ static void __copy_skb_header(struct sk_buff *new, const struct sk_buff *old)
offsetof(struct sk_buff, headers_start));
CHECK_SKB_FIELD(_state);
CHECK_SKB_FIELD(__pkt_encapsulation_offset);
+ CHECK_SKB_FIELD(__pkt_vlan_present_offset);
CHECK_SKB_FIELD(protocol);
CHECK_SKB_FIELD(csum);
CHECK_SKB_FIELD(hash);
CHECK_SKB_FIELD(priority);
CHECK_SKB_FIELD(skb_iif);
- CHECK_SKB_FIELD(vlan_proto);
- CHECK_SKB_FIELD(vlan_tci);
CHECK_SKB_FIELD(transport_header);
CHECK_SKB_FIELD(network_header);
CHECK_SKB_FIELD(mac_header);
--
2.26.3
^ permalink raw reply related
* [PATCH RFC 9/9] sk_buff: access secmark via getter/setter
From: Paolo Abeni @ 2021-07-21 16:44 UTC (permalink / raw)
To: netdev
Cc: David S. Miller, Jakub Kicinski, Florian Westphal, Eric Dumazet,
linux-security-module, selinux
In-Reply-To: <cover.1626882513.git.pabeni@redhat.com>
So we can track the field status and move it after tail.
After this commit the skb lifecycle for simple cases (no ct, no secmark,
no vlan, no UDP tunnel) uses 3 cacheline instead of 4 cachelines required
before this series.
e.g. GRO for non vlan traffic will consistently uses 3 cacheline for
each packet.
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
---
include/linux/skbuff.h | 40 ++++++++++++++++++++++----------
net/core/skbuff.c | 7 +++---
net/netfilter/nfnetlink_queue.c | 6 +++--
net/netfilter/nft_meta.c | 6 ++---
net/netfilter/xt_CONNSECMARK.c | 8 +++----
net/netfilter/xt_SECMARK.c | 2 +-
security/apparmor/lsm.c | 15 +++++++-----
security/selinux/hooks.c | 10 ++++----
security/smack/smack_lsm.c | 4 ++--
security/smack/smack_netfilter.c | 4 ++--
10 files changed, 62 insertions(+), 40 deletions(-)
diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index 7acf2a203918..941c0f858c65 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -688,6 +688,7 @@ typedef unsigned char *sk_buff_data_t;
* CHECKSUM_UNNECESSARY (max 3)
* @dst_pending_confirm: need to confirm neighbour
* @decrypted: Decrypted SKB
+ * @secmark_present: the secmark tag is present
* @_state: bitmap reporting the presence of some skb state info
* @has_nfct: @_state bit for nfct info
* @has_dst: @_state bit for dst pointer
@@ -695,7 +696,7 @@ typedef unsigned char *sk_buff_data_t;
* @active_extensions: @_state bits for active extensions (skb_ext_id types)
* @napi_id: id of the NAPI struct this skb came from
* @sender_cpu: (aka @napi_id) source CPU in XPS
- * @secmark: security marking
+ * @_secmark: security marking
* @mark: Generic packet mark
* @reserved_tailroom: (aka @mark) number of bytes of free space available
* at the tail of an sk_buff
@@ -870,6 +871,9 @@ struct sk_buff {
#endif
#ifdef CONFIG_TLS_DEVICE
__u8 decrypted:1;
+#endif
+#ifdef CONFIG_NETWORK_SECMARK
+ __u8 secmark_present:1;
#endif
union {
__u8 _state; /* state of extended fields */
@@ -903,9 +907,6 @@ struct sk_buff {
unsigned int sender_cpu;
};
#endif
-#ifdef CONFIG_NETWORK_SECMARK
- __u32 secmark;
-#endif
union {
__u32 mark;
@@ -961,6 +962,9 @@ struct sk_buff {
};
__u32 vlan_info;
};
+#ifdef CONFIG_NETWORK_SECMARK
+ __u32 _secmark;
+#endif
};
#ifdef __KERNEL__
@@ -4228,6 +4232,23 @@ static inline void skb_remcsum_process(struct sk_buff *skb, void *ptr,
skb->csum = csum_add(skb->csum, delta);
}
+static inline __u32 skb_secmark(const struct sk_buff *skb)
+{
+#if IS_ENABLED(CONFIG_NETWORK_SECMARK)
+ return skb->secmark_present ? skb->_secmark : 0;
+#else
+ return NULL;
+#endif
+}
+
+static inline void skb_set_secmark(struct sk_buff *skb, __u32 secmark)
+{
+#if IS_ENABLED(CONFIG_NETWORK_SECMARK)
+ skb->secmark_present = 1;
+ skb->_secmark = secmark;
+#endif
+}
+
static inline struct nf_conntrack *skb_nfct(const struct sk_buff *skb)
{
#if IS_ENABLED(CONFIG_NF_CONNTRACK)
@@ -4414,19 +4435,14 @@ static inline void nf_copy(struct sk_buff *dst, const struct sk_buff *src)
#ifdef CONFIG_NETWORK_SECMARK
static inline void skb_copy_secmark(struct sk_buff *to, const struct sk_buff *from)
{
- to->secmark = from->secmark;
-}
-
-static inline void skb_init_secmark(struct sk_buff *skb)
-{
- skb->secmark = 0;
+ to->secmark_present = from->secmark_present;
+ if (from->_secmark)
+ to->_secmark = from->_secmark;
}
#else
static inline void skb_copy_secmark(struct sk_buff *to, const struct sk_buff *from)
{ }
-static inline void skb_init_secmark(struct sk_buff *skb)
-{ }
#endif
static inline int secpath_exists(const struct sk_buff *skb)
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index c59e90db80d5..704aecbde60d 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -998,6 +998,10 @@ static void __copy_skb_header(struct sk_buff *new, const struct sk_buff *old)
__skb_copy_inner_headers(new, old);
if (old->vlan_present)
new->vlan_info = old->vlan_info;
+#ifdef CONFIG_NETWORK_SECMARK
+ if (old->_secmark)
+ new->_secmark = old->_secmark;
+#endif
/* Note : this field could be in headers_start/headers_end section
* It is not yet because we do not want to have a 16 bit hole
@@ -1019,9 +1023,6 @@ static void __copy_skb_header(struct sk_buff *new, const struct sk_buff *old)
CHECK_SKB_FIELD(network_header);
CHECK_SKB_FIELD(mac_header);
CHECK_SKB_FIELD(mark);
-#ifdef CONFIG_NETWORK_SECMARK
- CHECK_SKB_FIELD(secmark);
-#endif
#ifdef CONFIG_NET_RX_BUSY_POLL
CHECK_SKB_FIELD(napi_id);
#endif
diff --git a/net/netfilter/nfnetlink_queue.c b/net/netfilter/nfnetlink_queue.c
index f774de0fc24f..cf00d4286187 100644
--- a/net/netfilter/nfnetlink_queue.c
+++ b/net/netfilter/nfnetlink_queue.c
@@ -304,14 +304,16 @@ static int nfqnl_put_sk_uidgid(struct sk_buff *skb, struct sock *sk)
static u32 nfqnl_get_sk_secctx(struct sk_buff *skb, char **secdata)
{
u32 seclen = 0;
+ u32 secmark;
#if IS_ENABLED(CONFIG_NETWORK_SECMARK)
if (!skb || !sk_fullsock(skb->sk))
return 0;
read_lock_bh(&skb->sk->sk_callback_lock);
- if (skb->secmark)
- security_secid_to_secctx(skb->secmark, secdata, &seclen);
+ secmark = skb_secmark(skb);
+ if (secmark)
+ security_secid_to_secctx(secmark, secdata, &seclen);
read_unlock_bh(&skb->sk->sk_callback_lock);
#endif
diff --git a/net/netfilter/nft_meta.c b/net/netfilter/nft_meta.c
index a7e01e9952f1..da4bc455d8bd 100644
--- a/net/netfilter/nft_meta.c
+++ b/net/netfilter/nft_meta.c
@@ -363,7 +363,7 @@ void nft_meta_get_eval(const struct nft_expr *expr,
#endif
#ifdef CONFIG_NETWORK_SECMARK
case NFT_META_SECMARK:
- *dest = skb->secmark;
+ *dest = skb_secmark(skb);
break;
#endif
case NFT_META_PKTTYPE:
@@ -451,7 +451,7 @@ void nft_meta_set_eval(const struct nft_expr *expr,
break;
#ifdef CONFIG_NETWORK_SECMARK
case NFT_META_SECMARK:
- skb->secmark = value;
+ skb_set_secmark(skb, value);
break;
#endif
default:
@@ -833,7 +833,7 @@ static void nft_secmark_obj_eval(struct nft_object *obj, struct nft_regs *regs,
const struct nft_secmark *priv = nft_obj_data(obj);
struct sk_buff *skb = pkt->skb;
- skb->secmark = priv->secid;
+ skb_set_secmark(skb, priv->secid);
}
static int nft_secmark_obj_init(const struct nft_ctx *ctx,
diff --git a/net/netfilter/xt_CONNSECMARK.c b/net/netfilter/xt_CONNSECMARK.c
index 76acecf3e757..26f4fbc04c0b 100644
--- a/net/netfilter/xt_CONNSECMARK.c
+++ b/net/netfilter/xt_CONNSECMARK.c
@@ -31,13 +31,13 @@ MODULE_ALIAS("ip6t_CONNSECMARK");
*/
static void secmark_save(const struct sk_buff *skb)
{
- if (skb->secmark) {
+ if (skb_secmark(skb)) {
struct nf_conn *ct;
enum ip_conntrack_info ctinfo;
ct = nf_ct_get(skb, &ctinfo);
if (ct && !ct->secmark) {
- ct->secmark = skb->secmark;
+ ct->secmark = skb_secmark(skb);
nf_conntrack_event_cache(IPCT_SECMARK, ct);
}
}
@@ -49,13 +49,13 @@ static void secmark_save(const struct sk_buff *skb)
*/
static void secmark_restore(struct sk_buff *skb)
{
- if (!skb->secmark) {
+ if (!skb_secmark(skb)) {
const struct nf_conn *ct;
enum ip_conntrack_info ctinfo;
ct = nf_ct_get(skb, &ctinfo);
if (ct && ct->secmark)
- skb->secmark = ct->secmark;
+ skb_set_secmark(skb, ct->secmark);
}
}
diff --git a/net/netfilter/xt_SECMARK.c b/net/netfilter/xt_SECMARK.c
index 498a0bf6f044..bc383bc2bba9 100644
--- a/net/netfilter/xt_SECMARK.c
+++ b/net/netfilter/xt_SECMARK.c
@@ -36,7 +36,7 @@ secmark_tg(struct sk_buff *skb, const struct xt_secmark_target_info_v1 *info)
BUG();
}
- skb->secmark = secmark;
+ skb_set_secmark(skb, secmark);
return XT_CONTINUE;
}
diff --git a/security/apparmor/lsm.c b/security/apparmor/lsm.c
index f72406fe1bf2..afbae187b920 100644
--- a/security/apparmor/lsm.c
+++ b/security/apparmor/lsm.c
@@ -1053,12 +1053,13 @@ static int apparmor_socket_shutdown(struct socket *sock, int how)
static int apparmor_socket_sock_rcv_skb(struct sock *sk, struct sk_buff *skb)
{
struct aa_sk_ctx *ctx = SK_CTX(sk);
+ u32 secmark = skb_secmark(skb);
- if (!skb->secmark)
+ if (!secmark)
return 0;
return apparmor_secmark_check(ctx->label, OP_RECVMSG, AA_MAY_RECEIVE,
- skb->secmark, sk);
+ secmark, sk);
}
#endif
@@ -1160,12 +1161,13 @@ static int apparmor_inet_conn_request(const struct sock *sk, struct sk_buff *skb
struct request_sock *req)
{
struct aa_sk_ctx *ctx = SK_CTX(sk);
+ u32 secmark = skb_secmark(skb);
- if (!skb->secmark)
+ if (!secmark)
return 0;
return apparmor_secmark_check(ctx->label, OP_CONNECT, AA_MAY_CONNECT,
- skb->secmark, sk);
+ secmark, sk);
}
#endif
@@ -1754,10 +1756,11 @@ static unsigned int apparmor_ip_postroute(void *priv,
struct sk_buff *skb,
const struct nf_hook_state *state)
{
+ u32 secmark = skb_secmark(skb);
struct aa_sk_ctx *ctx;
struct sock *sk;
- if (!skb->secmark)
+ if (!secmark)
return NF_ACCEPT;
sk = skb_to_full_sk(skb);
@@ -1766,7 +1769,7 @@ static unsigned int apparmor_ip_postroute(void *priv,
ctx = SK_CTX(sk);
if (!apparmor_secmark_check(ctx->label, OP_SENDMSG, AA_MAY_SEND,
- skb->secmark, sk))
+ secmark, sk))
return NF_ACCEPT;
return NF_DROP_ERR(-ECONNREFUSED);
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index b0032c42333e..898b81ba7566 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -5138,7 +5138,7 @@ static int selinux_sock_rcv_skb_compat(struct sock *sk, struct sk_buff *skb,
if (selinux_secmark_enabled()) {
err = avc_has_perm(&selinux_state,
- sk_sid, skb->secmark, SECCLASS_PACKET,
+ sk_sid, skb_secmark(skb), SECCLASS_PACKET,
PACKET__RECV, &ad);
if (err)
return err;
@@ -5214,7 +5214,7 @@ static int selinux_socket_sock_rcv_skb(struct sock *sk, struct sk_buff *skb)
if (secmark_active) {
err = avc_has_perm(&selinux_state,
- sk_sid, skb->secmark, SECCLASS_PACKET,
+ sk_sid, skb_secmark(skb), SECCLASS_PACKET,
PACKET__RECV, &ad);
if (err)
return err;
@@ -5727,7 +5727,7 @@ static unsigned int selinux_ip_forward(struct sk_buff *skb,
if (secmark_active)
if (avc_has_perm(&selinux_state,
- peer_sid, skb->secmark,
+ peer_sid, skb_secmark(skb),
SECCLASS_PACKET, PACKET__FORWARD_IN, &ad))
return NF_DROP;
@@ -5840,7 +5840,7 @@ static unsigned int selinux_ip_postroute_compat(struct sk_buff *skb,
if (selinux_secmark_enabled())
if (avc_has_perm(&selinux_state,
- sksec->sid, skb->secmark,
+ sksec->sid, skb_secmark(skb),
SECCLASS_PACKET, PACKET__SEND, &ad))
return NF_DROP_ERR(-ECONNREFUSED);
@@ -5964,7 +5964,7 @@ static unsigned int selinux_ip_postroute(struct sk_buff *skb,
if (secmark_active)
if (avc_has_perm(&selinux_state,
- peer_sid, skb->secmark,
+ peer_sid, skb_secmark(skb),
SECCLASS_PACKET, secmark_perm, &ad))
return NF_DROP_ERR(-ECONNREFUSED);
diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
index 223a6da0e6dc..2ed19e2db66a 100644
--- a/security/smack/smack_lsm.c
+++ b/security/smack/smack_lsm.c
@@ -3840,10 +3840,10 @@ static int smk_skb_to_addr_ipv6(struct sk_buff *skb, struct sockaddr_in6 *sip)
#ifdef CONFIG_NETWORK_SECMARK
static struct smack_known *smack_from_skb(struct sk_buff *skb)
{
- if (skb == NULL || skb->secmark == 0)
+ if (skb == NULL || skb_secmark(skb) == 0)
return NULL;
- return smack_from_secid(skb->secmark);
+ return smack_from_secid(skb_secmark(skb));
}
#else
static inline struct smack_known *smack_from_skb(struct sk_buff *skb)
diff --git a/security/smack/smack_netfilter.c b/security/smack/smack_netfilter.c
index fc7399b45373..881143e62eb4 100644
--- a/security/smack/smack_netfilter.c
+++ b/security/smack/smack_netfilter.c
@@ -31,7 +31,7 @@ static unsigned int smack_ipv6_output(void *priv,
if (sk && sk->sk_security) {
ssp = sk->sk_security;
skp = ssp->smk_out;
- skb->secmark = skp->smk_secid;
+ skb_set_secmark(skb, skp->smk_secid);
}
return NF_ACCEPT;
@@ -49,7 +49,7 @@ static unsigned int smack_ipv4_output(void *priv,
if (sk && sk->sk_security) {
ssp = sk->sk_security;
skp = ssp->smk_out;
- skb->secmark = skp->smk_secid;
+ skb_set_secmark(skb, skp->smk_secid);
}
return NF_ACCEPT;
--
2.26.3
^ permalink raw reply related
* [PATCH RFC 5/9] skbuff: introduce has_sk state bit.
From: Paolo Abeni @ 2021-07-21 16:44 UTC (permalink / raw)
To: netdev
Cc: David S. Miller, Jakub Kicinski, Florian Westphal, Eric Dumazet,
linux-security-module, selinux
In-Reply-To: <cover.1626882513.git.pabeni@redhat.com>
This change leverages the infrastructure introduced by the previous
patches to allow soft devices passing to the GRO engine owned skbs
without impacting the fast-path.
It's up to the GRO caller ensuring the bit validity before
invoking the GRO engine with the new helper skb_prepare_for_gro().
If the bit is set only skb with equal sk will be aggregated.
Additionally, skb truesize on GRO recycle and free is correctly
updated so that sk wmem is not changed by the GRO processing.
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
---
include/linux/skbuff.h | 2 ++
include/net/sock.h | 9 +++++++++
net/core/dev.c | 2 ++
net/core/skbuff.c | 13 +++++++++++--
4 files changed, 24 insertions(+), 2 deletions(-)
diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index 03be9a774c58..ea9fdcc7c7ca 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -691,6 +691,7 @@ typedef unsigned char *sk_buff_data_t;
* @_state: bitmap reporting the presence of some skb state info
* @has_nfct: @_state bit for nfct info
* @has_dst: @_state bit for dst pointer
+ * @has_sk: @_state bit for sk pointer, only relevant at GRO time
* @active_extensions: @_state bits for active extensions (skb_ext_id types)
* @napi_id: id of the NAPI struct this skb came from
* @sender_cpu: (aka @napi_id) source CPU in XPS
@@ -872,6 +873,7 @@ struct sk_buff {
struct {
__u8 has_nfct:1;
__u8 has_dst:1;
+ __u8 has_sk:1;
#ifdef CONFIG_SKB_EXTENSIONS
__u8 active_extensions:5;
#endif
diff --git a/include/net/sock.h b/include/net/sock.h
index f23cb259b0e2..c1f2d896794b 100644
--- a/include/net/sock.h
+++ b/include/net/sock.h
@@ -2249,6 +2249,15 @@ static inline __must_check bool skb_set_owner_sk_safe(struct sk_buff *skb, struc
return false;
}
+static inline void skb_prepare_for_gro(struct sk_buff *skb)
+{
+ if (skb->destructor != sock_wfree) {
+ skb_orphan(skb);
+ return;
+ }
+ skb->has_sk = 1;
+}
+
void sk_reset_timer(struct sock *sk, struct timer_list *timer,
unsigned long expires);
diff --git a/net/core/dev.c b/net/core/dev.c
index 70c24ed9ca67..2ef087958fc9 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -6022,6 +6022,7 @@ static void gro_list_prepare(const struct list_head *head,
struct tc_skb_ext *p_ext;
#endif
+ diffs |= p->sk != skb->sk;
diffs |= skb_metadata_dst_cmp(p, skb);
diffs |= skb_get_nfct(p) ^ skb_get_nfct(skb);
@@ -6299,6 +6300,7 @@ static void napi_reuse_skb(struct napi_struct *napi, struct sk_buff *skb)
skb_shinfo(skb)->gso_type = 0;
skb->truesize = SKB_TRUESIZE(skb_end_offset(skb));
if (unlikely(skb->_state)) {
+ skb_orphan(skb);
skb_ext_reset(skb);
nf_reset_ct(skb);
}
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index befb49d1a756..9ed754da6e13 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -947,6 +947,7 @@ void napi_skb_free_stolen_head(struct sk_buff *skb)
nf_reset_ct(skb);
skb_dst_drop(skb);
skb_ext_put(skb);
+ skb_orphan(skb);
}
napi_skb_cache_put(skb);
}
@@ -3884,6 +3885,9 @@ int skb_gro_receive_list(struct sk_buff *p, struct sk_buff *skb)
NAPI_GRO_CB(p)->last = skb;
NAPI_GRO_CB(p)->count++;
p->data_len += skb->len;
+
+ /* sk owenrship - if any - completely transferred to the aggregated packet */
+ skb->destructor = NULL;
p->truesize += skb->truesize;
p->len += skb->len;
@@ -4285,7 +4289,7 @@ int skb_gro_receive(struct sk_buff *p, struct sk_buff *skb)
delta_truesize = skb->truesize -
SKB_TRUESIZE(skb_end_offset(skb));
- skb->truesize -= skb->data_len;
+ /* napi_reuse_skb() will always re-init 'truesize' */
skb->len -= skb->data_len;
skb->data_len = 0;
@@ -4297,6 +4301,7 @@ int skb_gro_receive(struct sk_buff *p, struct sk_buff *skb)
struct page *page = virt_to_head_page(skb->head);
unsigned int first_size = headlen - offset;
unsigned int first_offset;
+ unsigned int new_truesize;
if (nr_frags + 1 + skbinfo->nr_frags > MAX_SKB_FRAGS)
goto merge;
@@ -4314,12 +4319,16 @@ int skb_gro_receive(struct sk_buff *p, struct sk_buff *skb)
memcpy(frag + 1, skbinfo->frags, sizeof(*frag) * skbinfo->nr_frags);
/* We dont need to clear skbinfo->nr_frags here */
- delta_truesize = skb->truesize - SKB_DATA_ALIGN(sizeof(struct sk_buff));
+ new_truesize = SKB_TRUESIZE(sizeof(struct sk_buff));
+ delta_truesize = skb->truesize - new_truesize;
+ skb->truesize = new_truesize;
NAPI_GRO_CB(skb)->free = NAPI_GRO_FREE_STOLEN_HEAD;
goto done;
}
merge:
+ /* sk owenrship - if any - completely transferred to the aggregated packet */
+ skb->destructor = NULL;
delta_truesize = skb->truesize;
if (offset > headlen) {
unsigned int eat = offset - headlen;
--
2.26.3
^ permalink raw reply related
* [PATCH RFC 4/9] net: optimize GRO for the common case.
From: Paolo Abeni @ 2021-07-21 16:44 UTC (permalink / raw)
To: netdev
Cc: David S. Miller, Jakub Kicinski, Florian Westphal, Eric Dumazet,
linux-security-module, selinux
In-Reply-To: <cover.1626882513.git.pabeni@redhat.com>
After the previous patches, at GRO time, skb->_state is
usually 0, unless the packets comes from some H/W offload
slowpath or tunnel without rx checksum offload.
We can optimize the GRO code assuming !skb->_state is likely.
This remove multiple conditionals in the fast-path, at the
price of an additional one when we hit the above "slow-paths".
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
---
net/core/dev.c | 29 +++++++++++++++++++++--------
net/core/skbuff.c | 8 +++++---
2 files changed, 26 insertions(+), 11 deletions(-)
diff --git a/net/core/dev.c b/net/core/dev.c
index 3ee58876e8f5..70c24ed9ca67 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -6002,7 +6002,6 @@ static void gro_list_prepare(const struct list_head *head,
diffs |= skb_vlan_tag_present(p) ^ skb_vlan_tag_present(skb);
if (skb_vlan_tag_present(p))
diffs |= skb_vlan_tag_get(p) ^ skb_vlan_tag_get(skb);
- diffs |= skb_metadata_dst_cmp(p, skb);
diffs |= skb_metadata_differs(p, skb);
if (maclen == ETH_HLEN)
diffs |= compare_ether_header(skb_mac_header(p),
@@ -6012,17 +6011,29 @@ static void gro_list_prepare(const struct list_head *head,
skb_mac_header(skb),
maclen);
- diffs |= skb_get_nfct(p) ^ skb_get_nfct(skb);
+ /* in most common scenarions _state is 0
+ * otherwise we are already on some slower paths
+ * either skip all the infrequent tests altogether or
+ * avoid trying too hard to skip each of them individually
+ */
+ if (!diffs && unlikely(skb->_state | p->_state)) {
+#if IS_ENABLED(CONFIG_SKB_EXTENSIONS) && IS_ENABLED(CONFIG_NET_TC_SKB_EXT)
+ struct tc_skb_ext *skb_ext;
+ struct tc_skb_ext *p_ext;
+#endif
+
+ diffs |= skb_metadata_dst_cmp(p, skb);
+ diffs |= skb_get_nfct(p) ^ skb_get_nfct(skb);
+
#if IS_ENABLED(CONFIG_SKB_EXTENSIONS) && IS_ENABLED(CONFIG_NET_TC_SKB_EXT)
- if (!diffs) {
- struct tc_skb_ext *skb_ext = skb_ext_find(skb, TC_SKB_EXT);
- struct tc_skb_ext *p_ext = skb_ext_find(p, TC_SKB_EXT);
+ skb_ext = skb_ext_find(skb, TC_SKB_EXT);
+ p_ext = skb_ext_find(p, TC_SKB_EXT);
diffs |= (!!p_ext) ^ (!!skb_ext);
if (!diffs && unlikely(skb_ext))
diffs |= p_ext->chain ^ skb_ext->chain;
- }
#endif
+ }
NAPI_GRO_CB(p)->same_flow = !diffs;
}
@@ -6287,8 +6298,10 @@ static void napi_reuse_skb(struct napi_struct *napi, struct sk_buff *skb)
skb->encapsulation = 0;
skb_shinfo(skb)->gso_type = 0;
skb->truesize = SKB_TRUESIZE(skb_end_offset(skb));
- skb_ext_reset(skb);
- nf_reset_ct(skb);
+ if (unlikely(skb->_state)) {
+ skb_ext_reset(skb);
+ nf_reset_ct(skb);
+ }
napi->skb = skb;
}
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index 2ffe18595635..befb49d1a756 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -943,9 +943,11 @@ void __kfree_skb_defer(struct sk_buff *skb)
void napi_skb_free_stolen_head(struct sk_buff *skb)
{
- nf_reset_ct(skb);
- skb_dst_drop(skb);
- skb_ext_put(skb);
+ if (unlikely(skb->_state)) {
+ nf_reset_ct(skb);
+ skb_dst_drop(skb);
+ skb_ext_put(skb);
+ }
napi_skb_cache_put(skb);
}
--
2.26.3
^ permalink raw reply related
* [PATCH RFC 3/9] sk_buff: move the active_extensions into the state bitfield
From: Paolo Abeni @ 2021-07-21 16:44 UTC (permalink / raw)
To: netdev
Cc: David S. Miller, Jakub Kicinski, Florian Westphal, Eric Dumazet,
linux-security-module, selinux
In-Reply-To: <cover.1626882513.git.pabeni@redhat.com>
No functional change intended
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
---
v1 -> v2:
- add CHECK_SKB_FIELD(_state) in __copy_skb_header
2 problems:
- this restrict the storage for new skb extensions to 0 or at most 1
- can't provide a build time check to ensure SKB_EXT do not exceed
active_extensions
I'm wondering about moving 2 random bits from the header section to
the old active_extensions location (and explicitly copy them on clone)
so that we can keep using 1 byte for extension and 1 byte for other
state things
---
include/linux/skbuff.h | 11 +++++------
net/core/skbuff.c | 1 +
2 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index 1b811585f6fc..03be9a774c58 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -670,7 +670,6 @@ typedef unsigned char *sk_buff_data_t;
* @pfmemalloc: skbuff was allocated from PFMEMALLOC reserves
* @pp_recycle: mark the packet for recycling instead of freeing (implies
* page_pool support on driver)
- * @active_extensions: active extensions (skb_ext_id types)
* @ndisc_nodetype: router type (from link layer)
* @ooo_okay: allow the mapping of a socket to a queue to be changed
* @l4_hash: indicate hash is a canonical 4-tuple hash over transport
@@ -692,6 +691,7 @@ typedef unsigned char *sk_buff_data_t;
* @_state: bitmap reporting the presence of some skb state info
* @has_nfct: @_state bit for nfct info
* @has_dst: @_state bit for dst pointer
+ * @active_extensions: @_state bits for active extensions (skb_ext_id types)
* @napi_id: id of the NAPI struct this skb came from
* @sender_cpu: (aka @napi_id) source CPU in XPS
* @secmark: security marking
@@ -796,9 +796,6 @@ struct sk_buff {
head_frag:1,
pfmemalloc:1,
pp_recycle:1; /* page_pool recycle indicator */
-#ifdef CONFIG_SKB_EXTENSIONS
- __u8 active_extensions;
-#endif
/* fields enclosed in headers_start/headers_end are copied
* using a single memcpy() in __copy_skb_header()
@@ -875,6 +872,9 @@ struct sk_buff {
struct {
__u8 has_nfct:1;
__u8 has_dst:1;
+#ifdef CONFIG_SKB_EXTENSIONS
+ __u8 active_extensions:5;
+#endif
};
};
@@ -4283,8 +4283,6 @@ static inline void skb_ext_put(struct sk_buff *skb)
static inline void __skb_ext_copy(struct sk_buff *dst,
const struct sk_buff *src)
{
- dst->active_extensions = src->active_extensions;
-
if (src->active_extensions) {
struct skb_ext *ext = src->extensions;
@@ -4296,6 +4294,7 @@ static inline void __skb_ext_copy(struct sk_buff *dst,
static inline void skb_ext_copy(struct sk_buff *dst, const struct sk_buff *src)
{
skb_ext_put(dst);
+ dst->active_extensions = src->active_extensions;
__skb_ext_copy(dst, src);
}
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index e94805bd8656..2ffe18595635 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -1001,6 +1001,7 @@ static void __copy_skb_header(struct sk_buff *new, const struct sk_buff *old)
memcpy(&new->headers_start, &old->headers_start,
offsetof(struct sk_buff, headers_end) -
offsetof(struct sk_buff, headers_start));
+ CHECK_SKB_FIELD(_state);
CHECK_SKB_FIELD(protocol);
CHECK_SKB_FIELD(csum);
CHECK_SKB_FIELD(hash);
--
2.26.3
^ permalink raw reply related
* [PATCH RFC 2/9] sk_buff: track dst status in skb->_state
From: Paolo Abeni @ 2021-07-21 16:44 UTC (permalink / raw)
To: netdev
Cc: David S. Miller, Jakub Kicinski, Florian Westphal, Eric Dumazet,
linux-security-module, selinux
In-Reply-To: <cover.1626882513.git.pabeni@redhat.com>
Similar to the previous patch, covering the dst field,
but limited to tracking only the dst status.
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
---
include/linux/skbuff.h | 4 ++++
include/net/dst.h | 3 +++
2 files changed, 7 insertions(+)
diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index ec3d34d8022f..1b811585f6fc 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -691,6 +691,7 @@ typedef unsigned char *sk_buff_data_t;
* @decrypted: Decrypted SKB
* @_state: bitmap reporting the presence of some skb state info
* @has_nfct: @_state bit for nfct info
+ * @has_dst: @_state bit for dst pointer
* @napi_id: id of the NAPI struct this skb came from
* @sender_cpu: (aka @napi_id) source CPU in XPS
* @secmark: security marking
@@ -873,6 +874,7 @@ struct sk_buff {
__u8 _state; /* state of extended fields */
struct {
__u8 has_nfct:1;
+ __u8 has_dst:1;
};
};
@@ -998,6 +1000,7 @@ static inline struct dst_entry *skb_dst(const struct sk_buff *skb)
*/
static inline void skb_dst_set(struct sk_buff *skb, struct dst_entry *dst)
{
+ skb->has_dst = !!dst;
skb->_skb_refdst = (unsigned long)dst;
}
@@ -1014,6 +1017,7 @@ static inline void skb_dst_set(struct sk_buff *skb, struct dst_entry *dst)
static inline void skb_dst_set_noref(struct sk_buff *skb, struct dst_entry *dst)
{
WARN_ON(!rcu_read_lock_held() && !rcu_read_lock_bh_held());
+ skb->has_dst = !!dst;
skb->_skb_refdst = (unsigned long)dst | SKB_DST_NOREF;
}
diff --git a/include/net/dst.h b/include/net/dst.h
index 75b1e734e9c2..2cb765dabc6f 100644
--- a/include/net/dst.h
+++ b/include/net/dst.h
@@ -272,11 +272,13 @@ static inline void skb_dst_drop(struct sk_buff *skb)
if (skb->_skb_refdst) {
refdst_drop(skb->_skb_refdst);
skb->_skb_refdst = 0UL;
+ skb->has_dst = 0;
}
}
static inline void __skb_dst_copy(struct sk_buff *nskb, unsigned long refdst)
{
+ nskb->has_dst = !!refdst;
nskb->_skb_refdst = refdst;
if (!(nskb->_skb_refdst & SKB_DST_NOREF))
dst_clone(skb_dst(nskb));
@@ -316,6 +318,7 @@ static inline bool skb_dst_force(struct sk_buff *skb)
dst = NULL;
skb->_skb_refdst = (unsigned long)dst;
+ skb->has_dst = !!dst;
}
return skb->_skb_refdst != 0UL;
--
2.26.3
^ permalink raw reply related
* [PATCH RFC 1/9] sk_buff: track nfct status in newly added skb->_state
From: Paolo Abeni @ 2021-07-21 16:44 UTC (permalink / raw)
To: netdev
Cc: David S. Miller, Jakub Kicinski, Florian Westphal, Eric Dumazet,
linux-security-module, selinux
In-Reply-To: <cover.1626882513.git.pabeni@redhat.com>
so that we can skip initizialzing such field at skb
allocation and move such field after 'tail'.
_state uses one byte hole in the header section.
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
---
v1 -> v2:
- : NULL
- has_nfct = !!nfct -> ovs uses skb_set_nfct(NULL, 0) to clear skb->_nfct
should skb_nfct()/skb_get_nfct() return IP_CT_UNTRACKED
if SKB_HAS_NFCT is not set?
---
include/linux/skbuff.h | 19 ++++++++++++++-----
1 file changed, 14 insertions(+), 5 deletions(-)
diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index f19190820e63..ec3d34d8022f 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -689,6 +689,8 @@ typedef unsigned char *sk_buff_data_t;
* CHECKSUM_UNNECESSARY (max 3)
* @dst_pending_confirm: need to confirm neighbour
* @decrypted: Decrypted SKB
+ * @_state: bitmap reporting the presence of some skb state info
+ * @has_nfct: @_state bit for nfct info
* @napi_id: id of the NAPI struct this skb came from
* @sender_cpu: (aka @napi_id) source CPU in XPS
* @secmark: security marking
@@ -765,9 +767,6 @@ struct sk_buff {
#endif
};
-#if defined(CONFIG_NF_CONNTRACK) || defined(CONFIG_NF_CONNTRACK_MODULE)
- unsigned long _nfct;
-#endif
unsigned int len,
data_len;
__u16 mac_len,
@@ -870,6 +869,12 @@ struct sk_buff {
#ifdef CONFIG_TLS_DEVICE
__u8 decrypted:1;
#endif
+ union {
+ __u8 _state; /* state of extended fields */
+ struct {
+ __u8 has_nfct:1;
+ };
+ };
#ifdef CONFIG_NET_SCHED
__u16 tc_index; /* traffic control index */
@@ -936,6 +941,9 @@ struct sk_buff {
/* only useable after checking ->active_extensions != 0 */
struct skb_ext *extensions;
#endif
+#if defined(CONFIG_NF_CONNTRACK) || defined(CONFIG_NF_CONNTRACK_MODULE)
+ unsigned long _nfct;
+#endif
};
#ifdef __KERNEL__
@@ -4198,7 +4206,7 @@ static inline void skb_remcsum_process(struct sk_buff *skb, void *ptr,
static inline struct nf_conntrack *skb_nfct(const struct sk_buff *skb)
{
#if IS_ENABLED(CONFIG_NF_CONNTRACK)
- return (void *)(skb->_nfct & NFCT_PTRMASK);
+ return skb->has_nfct ? (void *)(skb->_nfct & NFCT_PTRMASK) : NULL;
#else
return NULL;
#endif
@@ -4207,7 +4215,7 @@ static inline struct nf_conntrack *skb_nfct(const struct sk_buff *skb)
static inline unsigned long skb_get_nfct(const struct sk_buff *skb)
{
#if IS_ENABLED(CONFIG_NF_CONNTRACK)
- return skb->_nfct;
+ return skb->has_nfct ? skb->_nfct : 0;
#else
return 0UL;
#endif
@@ -4216,6 +4224,7 @@ static inline unsigned long skb_get_nfct(const struct sk_buff *skb)
static inline void skb_set_nfct(struct sk_buff *skb, unsigned long nfct)
{
#if IS_ENABLED(CONFIG_NF_CONNTRACK)
+ skb->has_nfct = !!nfct;
skb->_nfct = nfct;
#endif
}
--
2.26.3
^ permalink raw reply related
* [PATCH RFC 0/9] sk_buff: optimize layout for GRO
From: Paolo Abeni @ 2021-07-21 16:44 UTC (permalink / raw)
To: netdev
Cc: David S. Miller, Jakub Kicinski, Florian Westphal, Eric Dumazet,
linux-security-module, selinux
This is a very early draft - in a different world would be
replaced by hallway discussion at in-person conference - aimed at
outlining some ideas and collect feedback on the overall outlook.
There are still bugs to be fixed, more test and benchmark need, etc.
There are 3 main goals:
- [try to] avoid the overhead for uncommon conditions at GRO time
(patches 1-4)
- enable backpressure for the veth GRO path (patches 5-6)
- reduce the number of cacheline used by the sk_buff lifecycle
from 4 to 3, at least in some common scenarios (patches 1,7-9).
The idea here is avoid the initialization of some fields and
control their validity with a bitmask, as presented by at least
Florian and Jesper in the past.
The above requires a bit of code churn in some places and, yes,
a few new bits in the sk_buff struct (using some existing holes)
Paolo Abeni (9):
sk_buff: track nfct status in newly added skb->_state
sk_buff: track dst status in skb->_state
sk_buff: move the active_extensions into the state bitfield
net: optimize GRO for the common case.
skbuff: introduce has_sk state bit.
veth: use skb_prepare_for_gro()
sk_buff: move inner header fields after tail
sk_buff: move vlan field after tail.
sk_buff: access secmark via getter/setter
drivers/net/veth.c | 2 +-
include/linux/skbuff.h | 117 ++++++++++++++++++++++---------
include/net/dst.h | 3 +
include/net/sock.h | 9 +++
net/core/dev.c | 31 +++++---
net/core/skbuff.c | 40 +++++++----
net/netfilter/nfnetlink_queue.c | 6 +-
net/netfilter/nft_meta.c | 6 +-
net/netfilter/xt_CONNSECMARK.c | 8 +--
net/netfilter/xt_SECMARK.c | 2 +-
security/apparmor/lsm.c | 15 ++--
security/selinux/hooks.c | 10 +--
security/smack/smack_lsm.c | 4 +-
security/smack/smack_netfilter.c | 4 +-
14 files changed, 175 insertions(+), 82 deletions(-)
--
2.26.3
^ permalink raw reply
* [PATCH v2] KEYS: trusted: fix use as module when CONFIG_TCG_TPM=m
From: Ahmad Fatoum @ 2021-07-21 16:02 UTC (permalink / raw)
To: Jarkko Sakkinen, James Morris, Serge E. Hallyn, James Bottomley,
Mimi Zohar, Sumit Garg, David Howells, Herbert Xu,
David S. Miller
Cc: kernel, Andreas Rammhold, Ahmad Fatoum, David Gstir,
Richard Weinberger, keyrings, linux-crypto, linux-kernel,
linux-security-module, linux-integrity
Since commit 5d0682be3189 ("KEYS: trusted: Add generic trusted keys
framework"), trusted.ko built with CONFIG_TCG_TPM=CONFIG_TRUSTED_KEYS=m
will not register the TPM trusted key type at runtime.
This is because, after that rework, CONFIG_DEPENDENCY of the TPM
and TEE backends were checked with #ifdef, but that's only true
when they're built-in.
Fix this by introducing two new boolean Kconfig symbols:
TRUSTED_KEYS_TPM and TRUSTED_KEYS_TEE with the appropriate
dependencies and use them to check which backends are available.
This also has a positive effect on user experience:
- It's now possible to use TEE trusted keys without CONFIG_TCG_TPM
- It's now possible to enable CONFIG_TCG_TPM, but exclude TPM from
available trust sources
- TEE=m && TRUSTED_KEYS=y no longer leads to TEE support
being silently dropped
Any code depending on the TPM trusted key backend or symbols exported
by it will now need to explicitly state that it
depends on TRUSTED_KEYS && TRUSTED_KEYS_TPM
The latter to ensure the dependency is built and the former to ensure
it's reachable for module builds. This currently only affects
CONFIG_ASYMMETRIC_TPM_KEY_SUBTYPE, so it's fixed up here as well.
Reported-by: Andreas Rammhold <andreas@rammhold.de>
Fixes: 5d0682be3189 ("KEYS: trusted: Add generic trusted keys framework")
Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
(Implicit) v1 was as a preparatory patch for CAAM trusted keys[1] with the
goal of fixing the Kconfig inflexibility after the TEE trusted key rework.
Unbeknownst to me, it also fixes a regression, which was later
reported by Andreas[2] along with a patch.
I split out the fix from the CAAM series and adjusted the commit
message to explain the regression.
v1 -> v2:
- Move rest of TPM-related selects from TRUSTED_KEYS to
TRUSTED_KEYS_TPM (Sumit)
- Remove left-over line in Makefile (Sumit)
- added Fixes: tag
- adjust commit message to reference the regression reported
by Andreas
- have ASYMMETRIC_TPM_KEY_SUBTYPE depend on TRUSTED_KEYS_TPM,
because it references global symbols that are exported
by the trusted key TPM backend.
[1]: https://lore.kernel.org/linux-integrity/f8285eb0135ba30c9d846cf9dd395d1f5f8b1efc.1624364386.git-series.a.fatoum@pengutronix.de/
[2]: https://lore.kernel.org/linux-integrity/20210719091335.vwfebcpkf4pag3wm@wrt/T/#t
To: Jarkko Sakkinen <jarkko@kernel.org>
To: James Morris <jmorris@namei.org>
To: "Serge E. Hallyn" <serge@hallyn.com>
To: James Bottomley <jejb@linux.ibm.com>
To: Mimi Zohar <zohar@linux.ibm.com>
To: Sumit Garg <sumit.garg@linaro.org>
To: David Howells <dhowells@redhat.com>
To: Herbert Xu <herbert@gondor.apana.org.au>
To: "David S. Miller" <davem@davemloft.net>
Cc: David Gstir <david@sigma-star.at>
Cc: Richard Weinberger <richard@nod.at>
Cc: keyrings@vger.kernel.org
Cc: linux-crypto@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Cc: linux-security-module@vger.kernel.org
Cc: linux-integrity@vger.kernel.org
---
crypto/asymmetric_keys/Kconfig | 2 +-
security/keys/Kconfig | 18 ++++++--------
security/keys/trusted-keys/Kconfig | 29 +++++++++++++++++++++++
security/keys/trusted-keys/Makefile | 8 +++----
security/keys/trusted-keys/trusted_core.c | 4 ++--
5 files changed, 43 insertions(+), 18 deletions(-)
create mode 100644 security/keys/trusted-keys/Kconfig
diff --git a/crypto/asymmetric_keys/Kconfig b/crypto/asymmetric_keys/Kconfig
index 1f1f004dc757..8886eddbf881 100644
--- a/crypto/asymmetric_keys/Kconfig
+++ b/crypto/asymmetric_keys/Kconfig
@@ -25,7 +25,7 @@ config ASYMMETRIC_PUBLIC_KEY_SUBTYPE
config ASYMMETRIC_TPM_KEY_SUBTYPE
tristate "Asymmetric TPM backed private key subtype"
depends on TCG_TPM
- depends on TRUSTED_KEYS
+ depends on TRUSTED_KEYS && TRUSTED_KEYS_TPM
select CRYPTO_HMAC
select CRYPTO_SHA1
select CRYPTO_HASH_INFO
diff --git a/security/keys/Kconfig b/security/keys/Kconfig
index 64b81abd087e..9ec302962fe2 100644
--- a/security/keys/Kconfig
+++ b/security/keys/Kconfig
@@ -70,23 +70,19 @@ config BIG_KEYS
config TRUSTED_KEYS
tristate "TRUSTED KEYS"
- depends on KEYS && TCG_TPM
- select CRYPTO
- select CRYPTO_HMAC
- select CRYPTO_SHA1
- select CRYPTO_HASH_INFO
- select ASN1_ENCODER
- select OID_REGISTRY
- select ASN1
+ depends on KEYS
help
This option provides support for creating, sealing, and unsealing
keys in the kernel. Trusted keys are random number symmetric keys,
- generated and RSA-sealed by the TPM. The TPM only unseals the keys,
- if the boot PCRs and other criteria match. Userspace will only ever
- see encrypted blobs.
+ generated and sealed by a trust source selected at kernel boot-time.
+ Userspace will only ever see encrypted blobs.
If you are unsure as to whether this is required, answer N.
+if TRUSTED_KEYS
+source "security/keys/trusted-keys/Kconfig"
+endif
+
config ENCRYPTED_KEYS
tristate "ENCRYPTED KEYS"
depends on KEYS
diff --git a/security/keys/trusted-keys/Kconfig b/security/keys/trusted-keys/Kconfig
new file mode 100644
index 000000000000..c163cfeedff6
--- /dev/null
+++ b/security/keys/trusted-keys/Kconfig
@@ -0,0 +1,29 @@
+config TRUSTED_KEYS_TPM
+ bool "TPM-based trusted keys"
+ depends on TCG_TPM >= TRUSTED_KEYS
+ default y
+ select CRYPTO
+ select CRYPTO_HMAC
+ select CRYPTO_SHA1
+ select CRYPTO_HASH_INFO
+ select ASN1_ENCODER
+ select OID_REGISTRY
+ select ASN1
+ help
+ Enable use of the Trusted Platform Module (TPM) as trusted key
+ backend. Trusted keys are are random number symmetric keys,
+ which will be generated and RSA-sealed by the TPM.
+ The TPM only unseals the keys, if the boot PCRs and other
+ criteria match.
+
+config TRUSTED_KEYS_TEE
+ bool "TEE-based trusted keys"
+ depends on TEE >= TRUSTED_KEYS
+ default y
+ help
+ Enable use of the Trusted Execution Environment (TEE) as trusted
+ key backend.
+
+if !TRUSTED_KEYS_TPM && !TRUSTED_KEYS_TEE
+comment "No trust source selected!"
+endif
diff --git a/security/keys/trusted-keys/Makefile b/security/keys/trusted-keys/Makefile
index feb8b6c3cc79..2e2371eae4d5 100644
--- a/security/keys/trusted-keys/Makefile
+++ b/security/keys/trusted-keys/Makefile
@@ -5,10 +5,10 @@
obj-$(CONFIG_TRUSTED_KEYS) += trusted.o
trusted-y += trusted_core.o
-trusted-y += trusted_tpm1.o
+trusted-$(CONFIG_TRUSTED_KEYS_TPM) += trusted_tpm1.o
$(obj)/trusted_tpm2.o: $(obj)/tpm2key.asn1.h
-trusted-y += trusted_tpm2.o
-trusted-y += tpm2key.asn1.o
+trusted-$(CONFIG_TRUSTED_KEYS_TPM) += trusted_tpm2.o
+trusted-$(CONFIG_TRUSTED_KEYS_TPM) += tpm2key.asn1.o
-trusted-$(CONFIG_TEE) += trusted_tee.o
+trusted-$(CONFIG_TRUSTED_KEYS_TEE) += trusted_tee.o
diff --git a/security/keys/trusted-keys/trusted_core.c b/security/keys/trusted-keys/trusted_core.c
index d5c891d8d353..8cab69e5d0da 100644
--- a/security/keys/trusted-keys/trusted_core.c
+++ b/security/keys/trusted-keys/trusted_core.c
@@ -27,10 +27,10 @@ module_param_named(source, trusted_key_source, charp, 0);
MODULE_PARM_DESC(source, "Select trusted keys source (tpm or tee)");
static const struct trusted_key_source trusted_key_sources[] = {
-#if defined(CONFIG_TCG_TPM)
+#if defined(CONFIG_TRUSTED_KEYS_TPM)
{ "tpm", &trusted_key_tpm_ops },
#endif
-#if defined(CONFIG_TEE)
+#if defined(CONFIG_TRUSTED_KEYS_TEE)
{ "tee", &trusted_key_tee_ops },
#endif
};
--
2.30.2
^ permalink raw reply related
* Re: issues about selinux namespace
From: xiujianfeng @ 2021-07-21 13:12 UTC (permalink / raw)
To: Paul Moore
Cc: Stephen Smalley, jamorris, Likun(OSLab), linux-security-module,
selinux
In-Reply-To: <CAHC9VhR3ZbcNM8awhJs9_NXmdUXHO4XoH8s2d3MjhMXwkgbh=Q@mail.gmail.com>
在 2021/7/20 10:56, Paul Moore 写道:
> On Mon, Jul 19, 2021 at 9:55 AM xiujianfeng <xiujianfeng@huawei.com> wrote:
>> thanks stepthen, I've found James's patch in
>> https://lwn.net/Articles/737949/,
>>
>> but it seems can't resolve my questions, so any futher discussion would
>> be helpfull and welcome.
>>
>> 在 2021/7/14 20:11, Stephen Smalley 写道:
>>> Please take your email to the selinux@vger.kernel.org. You are the
>>> second person to ask about selinux namespaces within the past week or
>>> so. I did upstream the refactoring and encapsulation of the data
>>> structures and code via the selinux_state patches, so those are in the
>>> mainline kernel these days, and Paul Moore and I have periodically
>>> re-based the remaining patches on top of upstream over in the
>>> https://github.com/SELinuxProject/selinux-kernel/tree/working-selinuxns
>>> branch. However, I had to drop the inode and superblock per-ns patches
>>> temporarily because of changes to LSM (inode blob management moved to
>>> the LSM framework out of the security modules), so that would need to
>>> be revisited. There was a separate patch from James Morris to support
>>> per-namespace security.selinux extended attributes; you can dig that
>>> out from the history or mailing lists if you want to revive that. I
>>> won't be able to look at it again until October at the earliest.
>>>
>>> On Wed, Jul 14, 2021 at 6:54 AM xiujianfeng <xiujianfeng@huawei.com> wrote:
>>>> Hi Stephen,
>>>>
>>>> I am writing to discuss about selinux namespace because I found your
>>>> previous work on github and I think selinux namespace is helpful to
>>>> harden container security. So I try to do further work but there are
>>>> some issues mentioned in the commit message and I have no idea how to
>>>> fix them, it would be great if I can get help from you.
>>>> First is about selinux hook functions, we need to update each hook to
>>>> perform its processing on current namespace and all of its ancestors,
>>>> for object, we can have different sid/tag in different namespace based
>>>> on inode namespace support, but for task, do we need to maintain each
>>>> security context generated in the corresponding namespace?
>>>> Second is the lifecycle management of on-disk inode labels. it's not
>>>> easy to handle this, should we clean all corresponding labels on disk
>>>> when namespace exit? if we do this, it may cost long time to iterate
>>>> inode on disk and must relabel files when container restart, if not, the
>>>> inode xattr space maybe full and cannot write label again when new
>>>> namespace starts.
>>>> BTW, do you have plan to finish the work?
>>>>
>>>> I look forward to receiving your reply.
>>>>
>>>> Best wishes.
> I understand that many mail clients do not encourage inline/bottom
> replies, but when posting to the various Linux Kernel mailing lists
> please make the effort to reply inline, or at the bottom, as
> appropriate.
>
> Namespacing the SELinux kernel code is a rather tricky thing, both
> with respect to the design and the mechanics of the implementation. I
> don't think we have a concrete idea yet on how we want to proceed in
> all of the areas mentioned; designs - and implementations - have been
> offered, but I think we are missing someone to drive the topic forward
> with demonstrations, sample implementations, etc. It is never a bad
> idea to ask how you can help a project, but in this case I think the
> answer is to step back for a moment, describe your use-case/problem,
> explain how you envision a namespaced SELinux helping you resolve
> this, and finally how you would want the namespaced SELinux
> implementation to work (how would you interact with it both via policy
> and runtime management).
thanks for you reply, I digged the history disscussion from
https://marc.info/?l=selinux&m=150696042210126&w=2
and find one use-case: Running multiple android instances on a single
host, this is the same as mine.
Anyway, I'll make a try.
>
> On a personal note, the regular rebasing of the SELinux namespace work
> has suffered lately due to other time commitments at work. I have
> recently (today) started a new position which should allow me to
> dedicate much more of my working hours to upstream development; it may
> take me a couple of weeks to get settled in, but you can expect the
> regular rebasing of selinux/working-selinuxns to resume in the future.
>
^ permalink raw reply
* Re: [PATCH] Smack: Fix wrong semantics in smk_access_entry()
From: Tianjia Zhang @ 2021-07-21 3:10 UTC (permalink / raw)
To: Casey Schaufler, James Morris, Serge E. Hallyn,
linux-security-module, linux-kernel
In-Reply-To: <20b7bc36-89fa-d004-74f0-629a42595f86@schaufler-ca.com>
On 7/21/21 12:32 AM, Casey Schaufler wrote:
> On 7/15/2021 8:15 AM, Casey Schaufler wrote:
>> On 7/15/2021 2:17 AM, Tianjia Zhang wrote:
>>> In the smk_access_entry() function, if no matching rule is found
>>> in the rust_list, a negative error code will be used to perform bit
>>> operations with the MAY_ enumeration value. This is semantically
>>> wrong. This patch fixes this issue.
>> Indeed, the code as written is functioning correctly by
>> sheer luck. I will take this patch. Thank you.
>>
>>> Signed-off-by: Tianjia Zhang <tianjia.zhang@linux.alibaba.com>
>
> Added to the Smack next branch.
>
Great, Thanks.
Tianjia
^ permalink raw reply
* Re: [PATCH] hardening: Clarify Kconfig text for auto-var-init
From: Gustavo A. R. Silva @ 2021-07-20 22:16 UTC (permalink / raw)
To: Kees Cook
Cc: linux-hardening, glider, linux-kernel, linux-security-module,
clang-built-linux
In-Reply-To: <20210720215957.3446719-1-keescook@chromium.org>
On Tue, Jul 20, 2021 at 02:59:57PM -0700, Kees Cook wrote:
> Clarify the details around the automatic variable initialization modes
> available. Specifically this details the values used for pattern init
> and expands on the rationale for zero init safety. Additionally makes
> zero init the default when available.
>
> Signed-off-by: Kees Cook <keescook@chromium.org>
Acked-by: Gustavo A. R. Silva <gustavoars@kernel.org>
Thanks!
--
Gustavo
> ---
> security/Kconfig.hardening | 52 +++++++++++++++++++++++---------------
> 1 file changed, 32 insertions(+), 20 deletions(-)
>
> diff --git a/security/Kconfig.hardening b/security/Kconfig.hardening
> index 023aea5e117c..90cbaff86e13 100644
> --- a/security/Kconfig.hardening
> +++ b/security/Kconfig.hardening
> @@ -29,6 +29,7 @@ choice
> prompt "Initialize kernel stack variables at function entry"
> default GCC_PLUGIN_STRUCTLEAK_BYREF_ALL if COMPILE_TEST && GCC_PLUGINS
> default INIT_STACK_ALL_PATTERN if COMPILE_TEST && CC_HAS_AUTO_VAR_INIT_PATTERN
> + default INIT_STACK_ALL_ZERO if CC_HAS_AUTO_VAR_INIT_PATTERN
> default INIT_STACK_NONE
> help
> This option enables initialization of stack variables at
> @@ -39,11 +40,11 @@ choice
> syscalls.
>
> This chooses the level of coverage over classes of potentially
> - uninitialized variables. The selected class will be
> + uninitialized variables. The selected class of variable will be
> initialized before use in a function.
>
> config INIT_STACK_NONE
> - bool "no automatic initialization (weakest)"
> + bool "no automatic stack variable initialization (weakest)"
> help
> Disable automatic stack variable initialization.
> This leaves the kernel vulnerable to the standard
> @@ -80,7 +81,7 @@ choice
> and is disallowed.
>
> config GCC_PLUGIN_STRUCTLEAK_BYREF_ALL
> - bool "zero-init anything passed by reference (very strong)"
> + bool "zero-init everything passed by reference (very strong)"
> depends on GCC_PLUGINS
> depends on !(KASAN && KASAN_STACK)
> select GCC_PLUGIN_STRUCTLEAK
> @@ -91,33 +92,44 @@ choice
> of uninitialized stack variable exploits and information
> exposures.
>
> + As a side-effect, this keeps a lot of variables on the
> + stack that can otherwise be optimized out, so combining
> + this with CONFIG_KASAN_STACK can lead to a stack overflow
> + and is disallowed.
> +
> config INIT_STACK_ALL_PATTERN
> - bool "0xAA-init everything on the stack (strongest)"
> + bool "pattern-init everything (strongest)"
> depends on CC_HAS_AUTO_VAR_INIT_PATTERN
> help
> - Initializes everything on the stack with a 0xAA
> - pattern. This is intended to eliminate all classes
> - of uninitialized stack variable exploits and information
> - exposures, even variables that were warned to have been
> - left uninitialized.
> + Initializes everything on the stack (including padding)
> + with a specific debug value. This is intended to eliminate
> + all classes of uninitialized stack variable exploits and
> + information exposures, even variables that were warned about
> + having been left uninitialized.
>
> Pattern initialization is known to provoke many existing bugs
> related to uninitialized locals, e.g. pointers receive
> - non-NULL values, buffer sizes and indices are very big.
> + non-NULL values, buffer sizes and indices are very big. The
> + pattern is situation-specific; Clang on 64-bit uses 0xAA
> + repeating for all types and padding except float and double
> + which use 0xFF repeating (-NaN). Clang on 32-bit uses 0xFF
> + repeating for all types and padding.
>
> config INIT_STACK_ALL_ZERO
> - bool "zero-init everything on the stack (strongest and safest)"
> + bool "zero-init everything (strongest and safest)"
> depends on CC_HAS_AUTO_VAR_INIT_ZERO
> help
> - Initializes everything on the stack with a zero
> - value. This is intended to eliminate all classes
> - of uninitialized stack variable exploits and information
> - exposures, even variables that were warned to have been
> - left uninitialized.
> -
> - Zero initialization provides safe defaults for strings,
> - pointers, indices and sizes, and is therefore
> - more suitable as a security mitigation measure.
> + Initializes everything on the stack (including padding)
> + with a zero value. This is intended to eliminate all
> + classes of uninitialized stack variable exploits and
> + information exposures, even variables that were warned
> + about having been left uninitialized.
> +
> + Zero initialization provides safe defaults for strings
> + (immediately NUL-terminated), pointers (NULL), indices
> + (index 0), and sizes (0 length), so it is therefore more
> + suitable as a production security mitigation than pattern
> + initialization.
>
> endchoice
>
> --
> 2.30.2
>
^ permalink raw reply
* [PATCH] hardening: Clarify Kconfig text for auto-var-init
From: Kees Cook @ 2021-07-20 21:59 UTC (permalink / raw)
To: linux-hardening
Cc: Kees Cook, glider, Gustavo A. R. Silva, linux-kernel,
linux-security-module, clang-built-linux
Clarify the details around the automatic variable initialization modes
available. Specifically this details the values used for pattern init
and expands on the rationale for zero init safety. Additionally makes
zero init the default when available.
Signed-off-by: Kees Cook <keescook@chromium.org>
---
security/Kconfig.hardening | 52 +++++++++++++++++++++++---------------
1 file changed, 32 insertions(+), 20 deletions(-)
diff --git a/security/Kconfig.hardening b/security/Kconfig.hardening
index 023aea5e117c..90cbaff86e13 100644
--- a/security/Kconfig.hardening
+++ b/security/Kconfig.hardening
@@ -29,6 +29,7 @@ choice
prompt "Initialize kernel stack variables at function entry"
default GCC_PLUGIN_STRUCTLEAK_BYREF_ALL if COMPILE_TEST && GCC_PLUGINS
default INIT_STACK_ALL_PATTERN if COMPILE_TEST && CC_HAS_AUTO_VAR_INIT_PATTERN
+ default INIT_STACK_ALL_ZERO if CC_HAS_AUTO_VAR_INIT_PATTERN
default INIT_STACK_NONE
help
This option enables initialization of stack variables at
@@ -39,11 +40,11 @@ choice
syscalls.
This chooses the level of coverage over classes of potentially
- uninitialized variables. The selected class will be
+ uninitialized variables. The selected class of variable will be
initialized before use in a function.
config INIT_STACK_NONE
- bool "no automatic initialization (weakest)"
+ bool "no automatic stack variable initialization (weakest)"
help
Disable automatic stack variable initialization.
This leaves the kernel vulnerable to the standard
@@ -80,7 +81,7 @@ choice
and is disallowed.
config GCC_PLUGIN_STRUCTLEAK_BYREF_ALL
- bool "zero-init anything passed by reference (very strong)"
+ bool "zero-init everything passed by reference (very strong)"
depends on GCC_PLUGINS
depends on !(KASAN && KASAN_STACK)
select GCC_PLUGIN_STRUCTLEAK
@@ -91,33 +92,44 @@ choice
of uninitialized stack variable exploits and information
exposures.
+ As a side-effect, this keeps a lot of variables on the
+ stack that can otherwise be optimized out, so combining
+ this with CONFIG_KASAN_STACK can lead to a stack overflow
+ and is disallowed.
+
config INIT_STACK_ALL_PATTERN
- bool "0xAA-init everything on the stack (strongest)"
+ bool "pattern-init everything (strongest)"
depends on CC_HAS_AUTO_VAR_INIT_PATTERN
help
- Initializes everything on the stack with a 0xAA
- pattern. This is intended to eliminate all classes
- of uninitialized stack variable exploits and information
- exposures, even variables that were warned to have been
- left uninitialized.
+ Initializes everything on the stack (including padding)
+ with a specific debug value. This is intended to eliminate
+ all classes of uninitialized stack variable exploits and
+ information exposures, even variables that were warned about
+ having been left uninitialized.
Pattern initialization is known to provoke many existing bugs
related to uninitialized locals, e.g. pointers receive
- non-NULL values, buffer sizes and indices are very big.
+ non-NULL values, buffer sizes and indices are very big. The
+ pattern is situation-specific; Clang on 64-bit uses 0xAA
+ repeating for all types and padding except float and double
+ which use 0xFF repeating (-NaN). Clang on 32-bit uses 0xFF
+ repeating for all types and padding.
config INIT_STACK_ALL_ZERO
- bool "zero-init everything on the stack (strongest and safest)"
+ bool "zero-init everything (strongest and safest)"
depends on CC_HAS_AUTO_VAR_INIT_ZERO
help
- Initializes everything on the stack with a zero
- value. This is intended to eliminate all classes
- of uninitialized stack variable exploits and information
- exposures, even variables that were warned to have been
- left uninitialized.
-
- Zero initialization provides safe defaults for strings,
- pointers, indices and sizes, and is therefore
- more suitable as a security mitigation measure.
+ Initializes everything on the stack (including padding)
+ with a zero value. This is intended to eliminate all
+ classes of uninitialized stack variable exploits and
+ information exposures, even variables that were warned
+ about having been left uninitialized.
+
+ Zero initialization provides safe defaults for strings
+ (immediately NUL-terminated), pointers (NULL), indices
+ (index 0), and sizes (0 length), so it is therefore more
+ suitable as a production security mitigation than pattern
+ initialization.
endchoice
--
2.30.2
^ permalink raw reply related
* Re: [PATCH v2 6/6] KEYS: trusted: Introduce support for NXP CAAM-based trusted keys
From: Richard Weinberger @ 2021-07-20 20:37 UTC (permalink / raw)
To: Mimi Zohar
Cc: Ahmad Fatoum, James Bottomley, Richard Weinberger,
Jonathan Corbet, David Howells, Jarkko Sakkinen, James Bottomley,
kernel, James Morris, Serge E. Hallyn, horia geanta,
aymen sghaier, Herbert Xu, davem, Udit Agarwal, Eric Biggers,
Jan Luebbe, david, Franck Lenormand, Sumit Garg,
open list, ASYMMETRIC KEYS, Linux Crypto Mailing List,
Linux Doc Mailing List, linux-integrity, linux-kernel, LSM
In-Reply-To: <40e167cca7b59fc4e11f45ba807486e11eade419.camel@linux.ibm.com>
On Tue, Jul 20, 2021 at 10:24 PM Mimi Zohar <zohar@linux.ibm.com> wrote:
> > I'm not sure how to proceed. Should I base my changes on this series
> > or do you plan to send an updated
> > version soon?
> > Maybe it makes also sense to base my DCP patch set on yours.
> >
> > Trusted Keys maintainers, what do you prefer?
>
> Jarkko sent an email saying he is on vacation for 2 weeks. James was
> on vacation as well. If there is something that needs immediate
> attention, please let me know.
Oh, let them enjoy their well deserved vacation.
There no need to hurry. :-)
--
Thanks,
//richard
^ permalink raw reply
* Re: [PATCH v2 6/6] KEYS: trusted: Introduce support for NXP CAAM-based trusted keys
From: Mimi Zohar @ 2021-07-20 20:24 UTC (permalink / raw)
To: Richard Weinberger, Ahmad Fatoum, James Bottomley
Cc: Richard Weinberger, Jonathan Corbet, David Howells,
Jarkko Sakkinen, James Bottomley, kernel, James Morris,
Serge E. Hallyn, horia geanta, aymen sghaier, Herbert Xu, davem,
Udit Agarwal, Eric Biggers, Jan Luebbe, david, Franck Lenormand,
Sumit Garg, open list, ASYMMETRIC KEYS, Linux Crypto Mailing List,
Linux Doc Mailing List, linux-integrity, linux-kernel, LSM
In-Reply-To: <CAFLxGvxr94apP2jaT0tB6JRDtv_ivrguXK2Ykd3zer_4xtJ+2w@mail.gmail.com>
HI -
On Tue, 2021-07-20 at 21:19 +0200, Richard Weinberger wrote:
> On Fri, Jul 2, 2021 at 2:37 PM Ahmad Fatoum <a.fatoum@pengutronix.de> wrote:
> > > Both is possible. If the string starts with "0x" it needs to be decoded to a
> > > 128 bit key. Otherwise it has to be a up to 16 byte string.
> >
> > Fine by me. Looking forward to your patches. :-)
>
> I'm not sure how to proceed. Should I base my changes on this series
> or do you plan to send an updated
> version soon?
> Maybe it makes also sense to base my DCP patch set on yours.
>
> Trusted Keys maintainers, what do you prefer?
Jarkko sent an email saying he is on vacation for 2 weeks. James was
on vacation as well. If there is something that needs immediate
attention, please let me know.
thanks,
Mimi
^ permalink raw reply
* Re: [PATCH v2 6/6] KEYS: trusted: Introduce support for NXP CAAM-based trusted keys
From: Richard Weinberger @ 2021-07-20 19:19 UTC (permalink / raw)
To: Ahmad Fatoum
Cc: Richard Weinberger, Jonathan Corbet, David Howells,
Jarkko Sakkinen, James Bottomley, Mimi Zohar, kernel,
James Morris, Serge E. Hallyn, horia geanta, aymen sghaier,
Herbert Xu, davem, Udit Agarwal, Eric Biggers, Jan Luebbe, david,
Franck Lenormand, Sumit Garg, open list, ASYMMETRIC KEYS,
Linux Crypto Mailing List, Linux Doc Mailing List,
linux-integrity, linux-kernel, LSM
In-Reply-To: <ac8ef66f-4d57-ead0-d1b3-e97220463241@pengutronix.de>
On Fri, Jul 2, 2021 at 2:37 PM Ahmad Fatoum <a.fatoum@pengutronix.de> wrote:
> > Both is possible. If the string starts with "0x" it needs to be decoded to a
> > 128 bit key. Otherwise it has to be a up to 16 byte string.
>
> Fine by me. Looking forward to your patches. :-)
I'm not sure how to proceed. Should I base my changes on this series
or do you plan to send an updated
version soon?
Maybe it makes also sense to base my DCP patch set on yours.
Trusted Keys maintainers, what do you prefer?
--
Thanks,
//richard
^ permalink raw reply
* Re: [PATCH] smack: mark 'smack_enabled' global variable as __initdata
From: Casey Schaufler @ 2021-07-20 17:36 UTC (permalink / raw)
To: Austin Kim, jmorris, serge
Cc: linux-security-module, linux-kernel, kernel-team, Casey Schaufler
In-Reply-To: <20210629134144.GA1168@raspberrypi>
On 6/29/2021 6:41 AM, Austin Kim wrote:
> From: Austin Kim <austin.kim@lge.com>
>
> Mark 'smack_enabled' as __initdata
> since it is only used during initialization code.
>
> Signed-off-by: Austin Kim <austin.kim@lge.com>
Added to Smack next.
> ---
> security/smack/smack.h | 2 +-
> security/smack/smack_lsm.c | 2 +-
> 2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/security/smack/smack.h b/security/smack/smack.h
> index c3cfbdf4944a..99c3422596ab 100644
> --- a/security/smack/smack.h
> +++ b/security/smack/smack.h
> @@ -302,7 +302,7 @@ int smack_populate_secattr(struct smack_known *skp);
> /*
> * Shared data.
> */
> -extern int smack_enabled;
> +extern int smack_enabled __initdata;
> 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 223a6da0e6dc..cacbe7518519 100644
> --- a/security/smack/smack_lsm.c
> +++ b/security/smack/smack_lsm.c
> @@ -54,7 +54,7 @@
> static DEFINE_MUTEX(smack_ipv6_lock);
> static LIST_HEAD(smk_ipv6_port_list);
> struct kmem_cache *smack_rule_cache;
> -int smack_enabled;
> +int smack_enabled __initdata;
>
> #define A(s) {"smack"#s, sizeof("smack"#s) - 1, Opt_##s}
> static struct {
^ permalink raw reply
* Re: [PATCH] Smack: Fix wrong semantics in smk_access_entry()
From: Casey Schaufler @ 2021-07-20 16:32 UTC (permalink / raw)
To: Tianjia Zhang, James Morris, Serge E. Hallyn,
linux-security-module, linux-kernel, Casey Schaufler
In-Reply-To: <ae938c7b-2f7a-27ec-7077-ceeb517ba97f@schaufler-ca.com>
On 7/15/2021 8:15 AM, Casey Schaufler wrote:
> On 7/15/2021 2:17 AM, Tianjia Zhang wrote:
>> In the smk_access_entry() function, if no matching rule is found
>> in the rust_list, a negative error code will be used to perform bit
>> operations with the MAY_ enumeration value. This is semantically
>> wrong. This patch fixes this issue.
> Indeed, the code as written is functioning correctly by
> sheer luck. I will take this patch. Thank you.
>
>> Signed-off-by: Tianjia Zhang <tianjia.zhang@linux.alibaba.com>
Added to the Smack next branch.
>> ---
>> security/smack/smack_access.c | 17 ++++++++---------
>> 1 file changed, 8 insertions(+), 9 deletions(-)
>>
>> diff --git a/security/smack/smack_access.c b/security/smack/smack_access.c
>> index 1f391f6a3d47..d2186e2757be 100644
>> --- a/security/smack/smack_access.c
>> +++ b/security/smack/smack_access.c
>> @@ -81,23 +81,22 @@ int log_policy = SMACK_AUDIT_DENIED;
>> int smk_access_entry(char *subject_label, char *object_label,
>> struct list_head *rule_list)
>> {
>> - int may = -ENOENT;
>> struct smack_rule *srp;
>>
>> list_for_each_entry_rcu(srp, rule_list, list) {
>> if (srp->smk_object->smk_known == object_label &&
>> srp->smk_subject->smk_known == subject_label) {
>> - may = srp->smk_access;
>> - break;
>> + int may = srp->smk_access;
>> + /*
>> + * MAY_WRITE implies MAY_LOCK.
>> + */
>> + if ((may & MAY_WRITE) == MAY_WRITE)
>> + may |= MAY_LOCK;
>> + return may;
>> }
>> }
>>
>> - /*
>> - * MAY_WRITE implies MAY_LOCK.
>> - */
>> - if ((may & MAY_WRITE) == MAY_WRITE)
>> - may |= MAY_LOCK;
>> - return may;
>> + return -ENOENT;
>> }
>>
>> /**
^ permalink raw reply
* Re: [PATCH v3 2/3] ima: Return int in the functions to measure a buffer
From: Mimi Zohar @ 2021-07-20 13:01 UTC (permalink / raw)
To: Roberto Sassu, paul@paul-moore.com
Cc: stephen.smalley.work@gmail.com, prsriva02@gmail.com,
tusharsu@linux.microsoft.com, nramas@linux.microsoft.com,
linux-integrity@vger.kernel.org,
linux-security-module@vger.kernel.org,
linux-kernel@vger.kernel.org, selinux@vger.kernel.org
In-Reply-To: <bd953894da3041d5969da645db2f982e@huawei.com>
On Tue, 2021-07-20 at 12:38 +0000, Roberto Sassu wrote:
> > > This patch modifies the return type from void to int, and returns 0 if the
> > > buffer has been successfully measured, a negative value otherwise.
> >
> > Needed here is an explanation as to why ima_measure_critical_data() is
> > special.
>
> We don't want to unnecessarily calculate the digest twice.
That's what the "iint" cache is for. . This needs more a of an
explaintion as to why ima_measure_critical_data() is special.
thanks,
Mimi
^ permalink raw reply
* RE: [PATCH v3 2/3] ima: Return int in the functions to measure a buffer
From: Roberto Sassu @ 2021-07-20 12:38 UTC (permalink / raw)
To: Mimi Zohar, paul@paul-moore.com
Cc: stephen.smalley.work@gmail.com, prsriva02@gmail.com,
tusharsu@linux.microsoft.com, nramas@linux.microsoft.com,
linux-integrity@vger.kernel.org,
linux-security-module@vger.kernel.org,
linux-kernel@vger.kernel.org, selinux@vger.kernel.org
In-Reply-To: <2f4920dbdb16156e1af5cf78f592a5cf07ec3176.camel@linux.ibm.com>
> From: Mimi Zohar [mailto:zohar@linux.ibm.com]
> Sent: Monday, July 19, 2021 10:28 PM
> Hi Roberto,
>
> On Mon, 2021-07-05 at 11:09 +0200, Roberto Sassu wrote:
> > ima_measure_critical_data() and process_buffer_measurement() currently
> > don't return a result. A caller wouldn't be able to know whether those
> > functions were executed successfully.
>
> Missing is an explanation as to why these functions aren't currently
> returning a result. The LSM/IMA hooks only return a negative result
> for failure to appraise a file's integrity, not measure a file. Only
> failure to appraise a file's integrity results in preventing the file
> from being read/executed/mmaped. Other failures are only audited.
Hi Mimi
ok, will add it.
> > This patch modifies the return type from void to int, and returns 0 if the
> > buffer has been successfully measured, a negative value otherwise.
>
> Needed here is an explanation as to why ima_measure_critical_data() is
> special.
We don't want to unnecessarily calculate the digest twice.
> > Also, this patch does not modify the behavior of existing callers by
> > processing the returned value. For those, the return value is ignored.
>
> I agree that the existing behavior shouldn't change, but will this
> result in the bots complaining?
If I remember correctly, I didn't get any error even with W=1.
Thanks
Roberto
HUAWEI TECHNOLOGIES Duesseldorf GmbH, HRB 56063
Managing Director: Li Peng, Li Jian, Shi Yanli
> thanks,
>
> Mimi
^ permalink raw reply
* Re: issues about selinux namespace
From: Paul Moore @ 2021-07-20 2:56 UTC (permalink / raw)
To: xiujianfeng
Cc: Stephen Smalley, jamorris, Likun(OSLab), linux-security-module,
selinux
In-Reply-To: <ec36e53f-5a6d-b86e-790c-d58b7b503aae@huawei.com>
On Mon, Jul 19, 2021 at 9:55 AM xiujianfeng <xiujianfeng@huawei.com> wrote:
>
> thanks stepthen, I've found James's patch in
> https://lwn.net/Articles/737949/,
>
> but it seems can't resolve my questions, so any futher discussion would
> be helpfull and welcome.
>
> 在 2021/7/14 20:11, Stephen Smalley 写道:
> > Please take your email to the selinux@vger.kernel.org. You are the
> > second person to ask about selinux namespaces within the past week or
> > so. I did upstream the refactoring and encapsulation of the data
> > structures and code via the selinux_state patches, so those are in the
> > mainline kernel these days, and Paul Moore and I have periodically
> > re-based the remaining patches on top of upstream over in the
> > https://github.com/SELinuxProject/selinux-kernel/tree/working-selinuxns
> > branch. However, I had to drop the inode and superblock per-ns patches
> > temporarily because of changes to LSM (inode blob management moved to
> > the LSM framework out of the security modules), so that would need to
> > be revisited. There was a separate patch from James Morris to support
> > per-namespace security.selinux extended attributes; you can dig that
> > out from the history or mailing lists if you want to revive that. I
> > won't be able to look at it again until October at the earliest.
> >
> > On Wed, Jul 14, 2021 at 6:54 AM xiujianfeng <xiujianfeng@huawei.com> wrote:
> >> Hi Stephen,
> >>
> >> I am writing to discuss about selinux namespace because I found your
> >> previous work on github and I think selinux namespace is helpful to
> >> harden container security. So I try to do further work but there are
> >> some issues mentioned in the commit message and I have no idea how to
> >> fix them, it would be great if I can get help from you.
> >> First is about selinux hook functions, we need to update each hook to
> >> perform its processing on current namespace and all of its ancestors,
> >> for object, we can have different sid/tag in different namespace based
> >> on inode namespace support, but for task, do we need to maintain each
> >> security context generated in the corresponding namespace?
> >> Second is the lifecycle management of on-disk inode labels. it's not
> >> easy to handle this, should we clean all corresponding labels on disk
> >> when namespace exit? if we do this, it may cost long time to iterate
> >> inode on disk and must relabel files when container restart, if not, the
> >> inode xattr space maybe full and cannot write label again when new
> >> namespace starts.
> >> BTW, do you have plan to finish the work?
> >>
> >> I look forward to receiving your reply.
> >>
> >> Best wishes.
I understand that many mail clients do not encourage inline/bottom
replies, but when posting to the various Linux Kernel mailing lists
please make the effort to reply inline, or at the bottom, as
appropriate.
Namespacing the SELinux kernel code is a rather tricky thing, both
with respect to the design and the mechanics of the implementation. I
don't think we have a concrete idea yet on how we want to proceed in
all of the areas mentioned; designs - and implementations - have been
offered, but I think we are missing someone to drive the topic forward
with demonstrations, sample implementations, etc. It is never a bad
idea to ask how you can help a project, but in this case I think the
answer is to step back for a moment, describe your use-case/problem,
explain how you envision a namespaced SELinux helping you resolve
this, and finally how you would want the namespaced SELinux
implementation to work (how would you interact with it both via policy
and runtime management).
On a personal note, the regular rebasing of the SELinux namespace work
has suffered lately due to other time commitments at work. I have
recently (today) started a new position which should allow me to
dedicate much more of my working hours to upstream development; it may
take me a couple of weeks to get settled in, but you can expect the
regular rebasing of selinux/working-selinuxns to resume in the future.
--
paul moore
www.paul-moore.com
^ permalink raw reply
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