* [PATCH 7/8] ima: use ima_hash_algo for collision detection in the measurement list
From: Roberto Sassu @ 2020-01-27 17:04 UTC (permalink / raw)
To: zohar, jarkko.sakkinen, james.bottomley, linux-integrity
Cc: linux-security-module, linux-kernel, silviu.vlasceanu,
Roberto Sassu
In-Reply-To: <20200127170443.21538-1-roberto.sassu@huawei.com>
Before calculating a digest for each PCR bank, collisions were detected
with a SHA1 digest. This patch includes ima_hash_algo among the algorithms
used to calculate the template digest and checks collisions on that digest.
Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
---
security/integrity/ima/ima.h | 1 +
security/integrity/ima/ima_api.c | 2 +-
security/integrity/ima/ima_crypto.c | 25 ++++++++++++++++++-------
security/integrity/ima/ima_main.c | 1 +
security/integrity/ima/ima_queue.c | 8 ++++----
security/integrity/ima/ima_template.c | 2 +-
6 files changed, 26 insertions(+), 13 deletions(-)
diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index d959dab5bcce..3e1afa5150bc 100644
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h
@@ -51,6 +51,7 @@ extern int ima_policy_flag;
/* set during initialization */
extern int ima_hash_algo;
extern int ima_sha1_idx;
+extern int ima_hash_algo_idx;
extern int ima_appraise;
extern struct tpm_chip *ima_tpm_chip;
diff --git a/security/integrity/ima/ima_api.c b/security/integrity/ima/ima_api.c
index ebaf0056735c..a9bb45de6db9 100644
--- a/security/integrity/ima/ima_api.c
+++ b/security/integrity/ima/ima_api.c
@@ -51,7 +51,7 @@ int ima_alloc_init_template(struct ima_event_data *event_data,
if (!*entry)
return -ENOMEM;
- (*entry)->digests = kcalloc(ima_tpm_chip->nr_allocated_banks + 1,
+ (*entry)->digests = kcalloc(ima_tpm_chip->nr_allocated_banks + 2,
sizeof(*(*entry)->digests), GFP_NOFS);
if (!(*entry)->digests) {
result = -ENOMEM;
diff --git a/security/integrity/ima/ima_crypto.c b/security/integrity/ima/ima_crypto.c
index 786340feebbb..f84dfd8fc5ca 100644
--- a/security/integrity/ima/ima_crypto.c
+++ b/security/integrity/ima/ima_crypto.c
@@ -93,7 +93,7 @@ static struct crypto_shash *ima_alloc_tfm(enum hash_algo algo)
if (algo == ima_hash_algo)
return tfm;
- for (i = 0; i < ima_tpm_chip->nr_allocated_banks + 1; i++)
+ for (i = 0; i < ima_tpm_chip->nr_allocated_banks + 2; i++)
if (ima_algo_array[i].algo == algo && ima_algo_array[i].tfm)
return ima_algo_array[i].tfm;
@@ -116,19 +116,20 @@ int __init ima_init_crypto(void)
if (rc)
return rc;
- ima_algo_array = kmalloc_array(ima_tpm_chip->nr_allocated_banks + 1,
+ ima_algo_array = kmalloc_array(ima_tpm_chip->nr_allocated_banks + 2,
sizeof(*ima_algo_array), GFP_KERNEL);
if (!ima_algo_array) {
rc = -ENOMEM;
goto out;
}
- for (i = 0; i < ima_tpm_chip->nr_allocated_banks + 1; i++) {
+ for (i = 0; i < ima_tpm_chip->nr_allocated_banks + 2; i++) {
ima_algo_array[i].tfm = NULL;
ima_algo_array[i].algo = HASH_ALGO__LAST;
}
ima_sha1_idx = -1;
+ ima_hash_algo_idx = -1;
for (i = 0; i < ima_tpm_chip->nr_allocated_banks; i++) {
algo = ima_tpm_chip->allocated_banks[i].crypto_id;
@@ -143,6 +144,7 @@ int __init ima_init_crypto(void)
if (algo == ima_hash_algo) {
ima_algo_array[i].tfm = ima_shash_tfm;
+ ima_hash_algo_idx = i;
continue;
}
@@ -166,12 +168,21 @@ int __init ima_init_crypto(void)
}
ima_sha1_idx = i;
- ima_algo_array[i].algo = HASH_ALGO_SHA1;
+ if (ima_hash_algo == HASH_ALGO_SHA1)
+ ima_hash_algo_idx = i;
+
+ ima_algo_array[i++].algo = HASH_ALGO_SHA1;
+ }
+
+ if (ima_hash_algo_idx < 0) {
+ ima_algo_array[i].tfm = ima_shash_tfm;
+ ima_algo_array[i].algo = ima_hash_algo;
+ ima_hash_algo_idx = i;
}
return 0;
out_array:
- for (i = 0; i < ima_tpm_chip->nr_allocated_banks + 1; i++) {
+ for (i = 0; i < ima_tpm_chip->nr_allocated_banks + 2; i++) {
if (!ima_algo_array[i].tfm ||
ima_algo_array[i].tfm == ima_shash_tfm)
continue;
@@ -190,7 +201,7 @@ static void ima_free_tfm(struct crypto_shash *tfm)
if (tfm == ima_shash_tfm)
return;
- for (i = 0; i < ima_tpm_chip->nr_allocated_banks + 1; i++)
+ for (i = 0; i < ima_tpm_chip->nr_allocated_banks + 2; i++)
if (ima_algo_array[i].tfm == tfm)
return;
@@ -619,7 +630,7 @@ int ima_calc_field_array_hash(struct ima_field_data *field_data,
entry->digests[ima_sha1_idx].alg_id = TPM_ALG_SHA1;
- for (i = 0; i < ima_tpm_chip->nr_allocated_banks + 1; i++) {
+ for (i = 0; i < ima_tpm_chip->nr_allocated_banks + 2; i++) {
if (i == ima_sha1_idx)
continue;
diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
index c068067a0c47..6963d6c31bf1 100644
--- a/security/integrity/ima/ima_main.c
+++ b/security/integrity/ima/ima_main.c
@@ -31,6 +31,7 @@
#include "ima.h"
int ima_sha1_idx;
+int ima_hash_algo_idx;
#ifdef CONFIG_IMA_APPRAISE
int ima_appraise = IMA_APPRAISE_ENFORCE;
diff --git a/security/integrity/ima/ima_queue.c b/security/integrity/ima/ima_queue.c
index 7f7509774b85..58983d0f0214 100644
--- a/security/integrity/ima/ima_queue.c
+++ b/security/integrity/ima/ima_queue.c
@@ -57,8 +57,8 @@ static struct ima_queue_entry *ima_lookup_digest_entry(u8 *digest_value,
key = ima_hash_key(digest_value);
rcu_read_lock();
hlist_for_each_entry_rcu(qe, &ima_htable.queue[key], hnext) {
- rc = memcmp(qe->entry->digests[ima_sha1_idx].digest,
- digest_value, TPM_DIGEST_SIZE);
+ rc = memcmp(qe->entry->digests[ima_hash_algo_idx].digest,
+ digest_value, hash_digest_size[ima_hash_algo]);
if ((rc == 0) && (qe->entry->pcr == pcr)) {
ret = qe;
break;
@@ -110,7 +110,7 @@ static int ima_add_digest_entry(struct ima_template_entry *entry,
atomic_long_inc(&ima_htable.len);
if (update_htable) {
- key = ima_hash_key(entry->digests[ima_sha1_idx].digest);
+ key = ima_hash_key(entry->digests[ima_hash_algo_idx].digest);
hlist_add_head_rcu(&qe->hnext, &ima_htable.queue[key]);
}
@@ -162,7 +162,7 @@ int ima_add_template_entry(struct ima_template_entry *entry, int violation,
const char *op, struct inode *inode,
const unsigned char *filename)
{
- u8 *digest = entry->digests[ima_sha1_idx].digest;
+ u8 *digest = entry->digests[ima_hash_algo_idx].digest;
struct tpm_digest *digests_arg = entry->digests;
const char *audit_cause = "hash_added";
char tpm_audit_cause[AUDIT_CAUSE_LEN_MAX];
diff --git a/security/integrity/ima/ima_template.c b/security/integrity/ima/ima_template.c
index a9e1390c8e12..f71b5ee44179 100644
--- a/security/integrity/ima/ima_template.c
+++ b/security/integrity/ima/ima_template.c
@@ -311,7 +311,7 @@ static int ima_restore_template_data(struct ima_template_desc *template_desc,
if (!*entry)
return -ENOMEM;
- (*entry)->digests = kcalloc(ima_tpm_chip->nr_allocated_banks + 1,
+ (*entry)->digests = kcalloc(ima_tpm_chip->nr_allocated_banks + 2,
sizeof(*(*entry)->digests), GFP_NOFS);
if (!(*entry)->digests) {
kfree(*entry);
--
2.17.1
^ permalink raw reply related
* [PATCH 8/8] ima: switch to ima_hash_algo for boot aggregate
From: Roberto Sassu @ 2020-01-27 17:04 UTC (permalink / raw)
To: zohar, jarkko.sakkinen, james.bottomley, linux-integrity
Cc: linux-security-module, linux-kernel, silviu.vlasceanu,
Roberto Sassu
In-Reply-To: <20200127170443.21538-1-roberto.sassu@huawei.com>
boot_aggregate is the first entry of IMA measurement list. Its purpose is
to link pre-boot measurements to IMA measurements. As IMA was designed to
work with a TPM 1.2, the SHA1 PCR bank was always selected.
Currently, even if a TPM 2.0 is used, the SHA1 PCR bank is selected.
However, the assumption that the SHA1 PCR bank is always available is not
correct, as PCR banks can be selected with the PCR_Allocate() TPM command.
This patch tries to use ima_hash_algo as hash algorithm for boot_aggregate.
If no PCR bank uses that algorithm, the patch scans the allocated PCR banks
and selects the first for which the mapping between TPM algorithm ID and
crypto algorithm ID is known. If no suitable algorithm is found, an error
is returned.
Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
---
security/integrity/ima/ima_crypto.c | 38 +++++++++++++++++------------
security/integrity/ima/ima_init.c | 6 ++---
2 files changed, 25 insertions(+), 19 deletions(-)
diff --git a/security/integrity/ima/ima_crypto.c b/security/integrity/ima/ima_crypto.c
index f84dfd8fc5ca..9bf5e69945b7 100644
--- a/security/integrity/ima/ima_crypto.c
+++ b/security/integrity/ima/ima_crypto.c
@@ -780,25 +780,27 @@ static void __init ima_pcrread(u32 idx, struct tpm_digest *d)
/*
* Calculate the boot aggregate hash
*/
-static int __init ima_calc_boot_aggregate_tfm(char *digest,
- struct crypto_shash *tfm)
+static int __init ima_calc_boot_aggregate_tfm(char *digest, int bank_idx)
{
- struct tpm_digest d = { .alg_id = TPM_ALG_SHA1, .digest = {0} };
+ struct tpm_digest d = { .digest = {0} };
int rc;
u32 i;
- SHASH_DESC_ON_STACK(shash, tfm);
+ SHASH_DESC_ON_STACK(shash, ima_algo_array[bank_idx].tfm);
- shash->tfm = tfm;
+ shash->tfm = ima_algo_array[bank_idx].tfm;
rc = crypto_shash_init(shash);
if (rc != 0)
return rc;
+ d.alg_id = ima_tpm_chip->allocated_banks[bank_idx].alg_id;
+
/* cumulative sha1 over tpm registers 0-7 */
for (i = TPM_PCR0; i < TPM_PCR8; i++) {
ima_pcrread(i, &d);
/* now accumulate with current aggregate */
- rc = crypto_shash_update(shash, d.digest, TPM_DIGEST_SIZE);
+ rc = crypto_shash_update(shash, d.digest,
+ ima_tpm_chip->allocated_banks[bank_idx].digest_size);
}
if (!rc)
crypto_shash_final(shash, digest);
@@ -807,17 +809,21 @@ static int __init ima_calc_boot_aggregate_tfm(char *digest,
int __init ima_calc_boot_aggregate(struct ima_digest_data *hash)
{
- struct crypto_shash *tfm;
- int rc;
+ int bank_idx = ima_hash_algo_idx;
- tfm = ima_alloc_tfm(hash->algo);
- if (IS_ERR(tfm))
- return PTR_ERR(tfm);
-
- hash->length = crypto_shash_digestsize(tfm);
- rc = ima_calc_boot_aggregate_tfm(hash->digest, tfm);
+ if (bank_idx >= ima_tpm_chip->nr_allocated_banks) {
+ for (bank_idx = 0; bank_idx < ima_tpm_chip->nr_allocated_banks;
+ bank_idx++)
+ if (ima_algo_array[bank_idx].tfm)
+ break;
- ima_free_tfm(tfm);
+ if (bank_idx == ima_tpm_chip->nr_allocated_banks) {
+ pr_err("No suitable algo found for boot aggregate\n");
+ return -ENOENT;
+ }
+ }
- return rc;
+ hash->algo = ima_algo_array[bank_idx].algo;
+ hash->length = crypto_shash_digestsize(ima_algo_array[bank_idx].tfm);
+ return ima_calc_boot_aggregate_tfm(hash->digest, bank_idx);
}
diff --git a/security/integrity/ima/ima_init.c b/security/integrity/ima/ima_init.c
index 195cb4079b2b..b4da190a33ba 100644
--- a/security/integrity/ima/ima_init.c
+++ b/security/integrity/ima/ima_init.c
@@ -51,14 +51,14 @@ static int __init ima_add_boot_aggregate(void)
int violation = 0;
struct {
struct ima_digest_data hdr;
- char digest[TPM_DIGEST_SIZE];
+ char digest[SHA512_DIGEST_SIZE];
} hash;
memset(iint, 0, sizeof(*iint));
memset(&hash, 0, sizeof(hash));
iint->ima_hash = &hash.hdr;
- iint->ima_hash->algo = HASH_ALGO_SHA1;
- iint->ima_hash->length = SHA1_DIGEST_SIZE;
+ iint->ima_hash->algo = ima_hash_algo;
+ iint->ima_hash->length = hash_digest_size[ima_hash_algo];
if (ima_tpm_chip) {
result = ima_calc_boot_aggregate(&hash.hdr);
--
2.17.1
^ permalink raw reply related
* Re: [PATCH v14 00/23] LSM: Module stacking for AppArmor
From: Casey Schaufler @ 2020-01-27 17:16 UTC (permalink / raw)
To: Stephen Smalley, casey.schaufler, jmorris, linux-security-module,
selinux
Cc: keescook, john.johansen, penguin-kernel, paul
In-Reply-To: <de97dc66-7f5b-21f0-cf3d-a1485acbc1c9@tycho.nsa.gov>
On 1/27/2020 8:14 AM, Stephen Smalley wrote:
> On 1/24/20 4:49 PM, Casey Schaufler wrote:
>> On 1/24/2020 1:04 PM, Stephen Smalley wrote:
>>> On 1/23/20 7:22 PM, Casey Schaufler wrote:
>>>> This patchset provides the changes required for
>>>> the AppArmor security module to stack safely with any other.
>>>>
>>>> v14: Rebase to 5.5-rc5
>>>> Incorporate feedback from v13
>>>> - Use an array of audit rules (patch 0002)
>>>> - Significant change, removed Acks (patch 0002)
>>>> - Remove unneeded include (patch 0013)
>>>> - Use context.len correctly (patch 0015)
>>>> - Reorder code to be more sensible (patch 0016)
>>>> - Drop SO_PEERCONTEXT as it's not needed yet (patch 0023)
>>>
>>> I don't know for sure if this is your bug, but it happens every time I boot with your patches applied and not at all on stock v5.5-rc5 so here it is. Will try to bisect as time permits but not until next week. Trigger seems to be loading the tun driver.
>>
>> Thanks. I will have a look as well.
>
> Bisection led to the first patch in the series, "LSM: Infrastructure management of the sock security". Still not sure if the bug is in the patch itself or just being surfaced by it.
It looks like the tun code is making a private socket in tun_chr_open()
without initializing the sk_security member. It's possible that this used
to work implicitly, but I don't see how the change should have broken that.
Investigation continues.
^ permalink raw reply
* RE: [PATCH 6/8] ima: calculate and extend PCR with digests in ima_template_entry
From: Roberto Sassu @ 2020-01-27 17:29 UTC (permalink / raw)
To: zohar@linux.ibm.com, jarkko.sakkinen@linux.intel.com,
james.bottomley@hansenpartnership.com,
linux-integrity@vger.kernel.org
Cc: linux-security-module@vger.kernel.org,
linux-kernel@vger.kernel.org, Silviu Vlasceanu
In-Reply-To: <20200127170443.21538-7-roberto.sassu@huawei.com>
> -----Original Message-----
> From: Roberto Sassu
> Sent: Monday, January 27, 2020 6:05 PM
> To: zohar@linux.ibm.com; jarkko.sakkinen@linux.intel.com;
> james.bottomley@hansenpartnership.com; linux-integrity@vger.kernel.org
> Cc: linux-security-module@vger.kernel.org; linux-kernel@vger.kernel.org;
> Silviu Vlasceanu <Silviu.Vlasceanu@huawei.com>; Roberto Sassu
> <roberto.sassu@huawei.com>
> Subject: [PATCH 6/8] ima: calculate and extend PCR with digests in
> ima_template_entry
>
> This patch modifies ima_calc_field_array_hash() to calculate a template
> digest for each allocated PCR bank and SHA1. It also passes the tpm_digest
> array of the template entry to ima_pcr_extend() or in case of a violation,
> the pre-initialized digests array filled with 0xff.
>
> Padding with zeros is still done if the mapping between TPM algorithm ID
> and crypto ID is unknown.
>
> This patch calculates again the template digest when a measurement list is
> restored. Copying only the SHA1 digest (due to the limitation of the
> current measurement list format) is not sufficient, as hash collision
> detection will be done on the digest calculated with the default IMA hash
> algorithm.
>
> Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
> ---
> security/integrity/ima/ima_crypto.c | 26 ++++++++++++++++++++++-
> security/integrity/ima/ima_queue.c | 30 ++++++++++++++++-----------
> security/integrity/ima/ima_template.c | 14 +++++++++++--
> 3 files changed, 55 insertions(+), 15 deletions(-)
>
> diff --git a/security/integrity/ima/ima_crypto.c
> b/security/integrity/ima/ima_crypto.c
> index 63fb4bdf80b0..786340feebbb 100644
> --- a/security/integrity/ima/ima_crypto.c
> +++ b/security/integrity/ima/ima_crypto.c
> @@ -610,9 +610,33 @@ static int ima_calc_field_array_hash_tfm(struct
> ima_field_data *field_data,
> int ima_calc_field_array_hash(struct ima_field_data *field_data,
> struct ima_template_entry *entry)
> {
> - int rc;
> + u16 alg_id;
> + int rc, i;
>
> rc = ima_calc_field_array_hash_tfm(field_data, entry,
> ima_sha1_idx);
> + if (rc)
> + return rc;
> +
> + entry->digests[ima_sha1_idx].alg_id = TPM_ALG_SHA1;
> +
> + for (i = 0; i < ima_tpm_chip->nr_allocated_banks + 1; i++) {
> + if (i == ima_sha1_idx)
> + continue;
> +
> + alg_id = ima_tpm_chip->allocated_banks[i].alg_id;
The line above should be executed for i < ima_tpm_chip->nr_allocated_banks.
I will fix in the next version of the patch set.
Roberto
HUAWEI TECHNOLOGIES Duesseldorf GmbH, HRB 56063
Managing Director: Li Peng, Li Jian, Shi Yanli
^ permalink raw reply
* Re: KASAN slab-out-of-bounds in tun_chr_open/sock_init_data (Was: Re: [PATCH v14 00/23] LSM: Module stacking for AppArmor)
From: Casey Schaufler @ 2020-01-27 17:34 UTC (permalink / raw)
To: Stephen Smalley, casey.schaufler, jmorris, linux-security-module,
selinux
Cc: keescook, john.johansen, penguin-kernel, paul, lorenzo,
David S. Miller, amade, Linux Kernel Network Developers, maxk,
Casey Schaufler
In-Reply-To: <628f018e-5a88-295b-9e4d-b4c6a49645b5@tycho.nsa.gov>
On 1/27/2020 8:56 AM, Stephen Smalley wrote:
> On 1/27/20 11:14 AM, Stephen Smalley wrote:
>> On 1/24/20 4:49 PM, Casey Schaufler wrote:
>>> On 1/24/2020 1:04 PM, Stephen Smalley wrote:
>>>> On 1/23/20 7:22 PM, Casey Schaufler wrote:
>>>>> This patchset provides the changes required for
>>>>> the AppArmor security module to stack safely with any other.
>>>>>
>>>>> v14: Rebase to 5.5-rc5
>>>>> Incorporate feedback from v13
>>>>> - Use an array of audit rules (patch 0002)
>>>>> - Significant change, removed Acks (patch 0002)
>>>>> - Remove unneeded include (patch 0013)
>>>>> - Use context.len correctly (patch 0015)
>>>>> - Reorder code to be more sensible (patch 0016)
>>>>> - Drop SO_PEERCONTEXT as it's not needed yet (patch 0023)
>>>>
>>>> I don't know for sure if this is your bug, but it happens every time I boot with your patches applied and not at all on stock v5.5-rc5 so here it is. Will try to bisect as time permits but not until next week. Trigger seems to be loading the tun driver.
>>>
>>> Thanks. I will have a look as well.
>>
>> Bisection led to the first patch in the series, "LSM: Infrastructure management of the sock security". Still not sure if the bug is in the patch itself or just being surfaced by it.
>
> Looks like the bug is pre-existing to me and just exposed by your patch.
OK, thanks. I don't see how moving the allocation ought to have
perturbed that, but it's good to know what happened.
^ permalink raw reply
* Re: [PATCH 1/1] proc_keys_next should increase position index
From: Vasily Averin @ 2020-01-27 19:27 UTC (permalink / raw)
To: David Howells
Cc: keyrings, linux-security-module, Jarkko Sakkinen, James Morris,
Serge E. Hallyn
In-Reply-To: <1451508.1580125174@warthog.procyon.org.uk>
On 1/27/20 2:39 PM, David Howells wrote:
> I don't see the effect you're talking about with /proc/keys. I see the
> following:
>
> [root@andromeda ~]# dd if=/proc/keys bs=40 skip=1
> dd: /proc/keys: cannot skip to specified offset
>
> and then it follows up with the normal content with no obvious duplicates (the
> lines are numbered ascendingly in the first column).
>
> I think I may be being confused by what you mean by "the last line".
on unpatched kernel
$ uname -a
Linux vvsx1 5.3.0-26-generic #28~18.04.1-Ubuntu SMP Wed Dec 18 16:40:14 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux
$ dd if=/proc/keys bs=1 # VvS: full usual output
0f6bfdf5 I--Q--- 2 perm 3f010000 1000 1000 user 4af2f79ab8848d0a: 740
1fb91b32 I--Q--- 3 perm 1f3f0000 1000 65534 keyring _uid.1000: 2
27589480 I--Q--- 1 perm 0b0b0000 0 0 user invocation_id: 16
2f33ab67 I--Q--- 152 perm 3f030000 0 0 keyring _ses: 2
33f1d8fa I--Q--- 4 perm 3f030000 1000 1000 keyring _ses: 1
3d427fda I--Q--- 2 perm 3f010000 1000 1000 user 69ec44aec7678e5a: 740
3ead4096 I--Q--- 1 perm 1f3f0000 1000 65534 keyring _uid_ses.1000: 1
521+0 records in
521+0 records out
521 bytes copied, 0,00123769 s, 421 kB/s
$ dd if=/proc/keys bs=500 skip=1 # read after lseek in middle of last line
dd: /proc/keys: cannot skip to specified offset
g _uid_ses.1000: 1 <<<< end of last line
3ead4096 I--Q--- 1 perm 1f3f0000 1000 65534 keyring _uid_ses.1000: 1 <<<< and whole last lien again
0+1 records in
0+1 records out
97 bytes copied, 0,000135035 s, 718 kB/s
$ dd if=/proc/keys bs=1000 skip=1 # read after lseek beyond end of file
dd: /proc/keys: cannot skip to specified offset
3ead4096 I--Q--- 1 perm 1f3f0000 1000 65534 keyring _uid_ses.1000: 1 <<<< generates last line
0+1 records in
0+1 records out
76 bytes copied, 0,000119981 s, 633 kB/s
On patched kernel:
[test@localhost ~]$ uname -a
Linux localhost.localdomain 5.5.0-rc6-00151-gd8d014f #8 SMP Fri Jan 24 13:25:06 MSK 2020 x86_64 x86_64 x86_64 GNU/Linux
[test@localhost ~]$ dd if=/proc/keys bs=1
06e8bec5 I--Q--- 4 perm 1f3f0000 1000 65534 keyring _uid.1000: empty
1b7ee8ed I--Q--- 11 perm 3f030000 1000 1000 keyring _ses: 1
2c1a365d I--Q--- 1 perm 1f3f0000 1000 65534 keyring _uid_ses.1000: 1
3f5823b4 I--Q--- 6 perm 3f030000 1000 1000 keyring _ses: 1
286+0 records in
286+0 records out
286 bytes copied, 0,000414581 s, 690 kB/s
[test@localhost ~]$ dd if=/proc/keys bs=270 skip=1 # VvS: read after lseek in middle of last line
dd: /proc/keys: cannot skip to specified offset
yring _ses: 1 <<<< only end of last line was generated, as expected
0+1 records in
0+1 records out
16 bytes copied, 7,7199e-05 s, 207 kB/s
[test@localhost ~]$ dd if=/proc/keys bs=1000 skip=1 # VvS: read after lseek beond end of file
dd: /proc/keys: cannot skip to specified offset
0+0 records in <<<< nothing was generated, as expected
0+0 records out
0 bytes copied, 8,8036e-05 s, 0,0 kB/s
^ permalink raw reply
* Re: [PATCH v14 22/23] LSM: Add /proc attr entry for full LSM context
From: Simon McVittie @ 2020-01-27 20:05 UTC (permalink / raw)
To: Stephen Smalley
Cc: Casey Schaufler, casey.schaufler, jmorris, linux-security-module,
selinux, keescook, john.johansen, penguin-kernel, paul
In-Reply-To: <6bd3e393-e1df-7117-d15a-81cb1946807b@tycho.nsa.gov>
On Fri, 24 Jan 2020 at 15:16:36 -0500, Stephen Smalley wrote:
> Aside from the trailing newline and \0 issues, AppArmor also has a
> whitespace-separated (mode) field that may or may not be present in the
> contexts it presently returns, ala "/usr/sbin/cupsd (enforce)".
My understanding from last time I worked with AppArmor is that this
is genuinely part of the context, and whether it is present or absent
does not vary according to the kernel API used to access contexts.
AppArmor-specific higher-level APIs parse it into a label and an optional
mode, but LSM-agnostic user-space APIs (like the one in dbus) pass the
whole string through as-is.
(In practice it seems to be present if and only if the context is
something other than "unconfined", although I don't know offhand whether
that's an API guarantee.)
smcv
^ permalink raw reply
* Denying access from sb_umount issue
From: Alexander Ivanov @ 2020-01-27 20:34 UTC (permalink / raw)
To: linux-security-module
The lsm module implementes sb_umount() hook to disable lazy umounts. When access is denied, all consecutive umounts fail. It looks like mount reference count is messed up when sb_umount() return -EPERM. Is it possible that umount syscall assumes do_umount() touches those references and it calls
dput(path.dentry);
mntput_no_expire(mnt);
regardless of what do_mount() returns?
Thanks,
--Alex
^ permalink raw reply
* Re: Denying access from sb_umount issue
From: James Morris @ 2020-01-27 20:58 UTC (permalink / raw)
To: Alexander Ivanov; +Cc: linux-security-module
In-Reply-To: <9e7ccbf8-e640-49b1-8e9c-ffa330b0feef@www.fastmail.com>
On Mon, 27 Jan 2020, Alexander Ivanov wrote:
> The lsm module implementes sb_umount() hook to disable lazy umounts. When access is denied, all consecutive umounts fail. It looks like mount reference count is messed up when sb_umount() return -EPERM. Is it possible that umount syscall assumes do_umount() touches those references and it calls
>
> dput(path.dentry);
> mntput_no_expire(mnt);
>
> regardless of what do_mount() returns?
You mean do_umount()?
There are several other do_umount() error cases which would have the same
behavior, right after security_sb_umount().
What error codes are you seeing for the consecutive failures?
--
James Morris
<jmorris@namei.org>
^ permalink raw reply
* Re: Denying access from sb_umount issue
From: Alexander Ivanov @ 2020-01-27 21:10 UTC (permalink / raw)
To: James Morris; +Cc: linux-security-module
In-Reply-To: <alpine.LRH.2.21.2001280754180.22715@namei.org>
On Mon, 27 Jan 2020 12:58 -08:00, James Morris <jmorris@namei.org> wrote:
> On Mon, 27 Jan 2020, Alexander Ivanov wrote:
>
> > The lsm module implementes sb_umount() hook to disable lazy umounts. When access is denied, all consecutive umounts fail. It looks like mount reference count is messed up when sb_umount() return -EPERM. Is it possible that umount syscall assumes do_umount() touches those references and it calls
> >
> > dput(path.dentry);
> > mntput_no_expire(mnt);
> >
> > regardless of what do_mount() returns?
>
> You mean do_umount()?
>
yes, typo, sorry.
> There are several other do_umount() error cases which would have the same
> behavior, right after security_sb_umount().
>
We do umount with MNT_DETACH, thus those early returns seem not applicable?
> What error codes are you seeing for the consecutive failures?
>
We got EBUSY.
Thanks,
--Alex
^ permalink raw reply
* [GIT PULL] SELinux patches for v5.6
From: Paul Moore @ 2020-01-27 22:26 UTC (permalink / raw)
To: Linus Torvalds; +Cc: selinux, linux-security-module, linux-kernel
Hi Linus,
This is one of the bigger SELinux pull requests in recent years with
28 patches. Everything is passing our test suite and the highlights
are listed below, please merge them for v5.6.
- Mark CONFIG_SECURITY_SELINUX_DISABLE as deprecated. We're some time
away from actually attempting to remove this in the kernel, but the
only distro we know that still uses it (Fedora) is working on moving
away from this so we want to at least let people know we are planning
to remove it.
- Reorder the SELinux hooks to help prevent bad things when SELinux is
disabled at runtime. The proper fix is to remove the
CONFIG_SECURITY_SELINUX_DISABLE functionality (see above) and just
take care of it at boot time (e.g. "selinux=0").
- Add SELinux controls for the kernel lockdown functionality,
introducing a new SELinux class/permissions: "lockdown { integrity
confidentiality }".
- Add a SELinux control for move_mount(2) that reuses the "file {
mounton }" permission.
- Improvements to the SELinux security label data store lookup
functions to speed up translations between our internal label
representations and the visible string labels (both directions).
- Revisit a previous fix related to SELinux inode auditing and
permission caching and do it correctly this time.
- Fix the SELinux access decision cache to cleanup properly on error.
In some extreme cases this could limit the cache size and result in a
decrease in performance.
- Enable SELinux per-file labeling for binderfs.
- The SELinux initialized and disabled flags were wrapped with
accessors to ensure they are accessed correctly.
- Mark several key SELinux structures with __randomize_layout.
- Changes to the LSM build configuration to only build
security/lsm_audit.c when needed.
- Changes to the SELinux build configuration to only build the IB
object cache when CONFIG_SECURITY_INFINIBAND is enabled.
- Move a number of single-caller functions into their callers.
- Documentation fixes (/selinux -> /sys/fs/selinux).
- A handful of cleanup patches that aren't worth mentioning on their
own, the individual descriptions have plenty of detail.
Thanks,
-Paul
--
The following changes since commit e42617b825f8073569da76dc4510bfa019b1c35a:
Linux 5.5-rc1 (2019-12-08 14:57:55 -0800)
are available in the Git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/pcmoore/selinux.git
tags/selinux-pr-20200127
for you to fetch changes up to 98aa00345de54b8340dc2ddcd87f446d33387b5e:
selinux: fix regression introduced by move_mount(2) syscall
(2020-01-20 07:42:37 -0500)
----------------------------------------------------------------
selinux/stable-5.6 PR 20200127
----------------------------------------------------------------
Hridya Valsaraju (1):
selinux: allow per-file labelling for binderfs
Huaisheng Ye (2):
selinux: remove redundant msg_msg_alloc_security
selinux: remove redundant selinux_nlmsg_perm
Jaihind Yadav (1):
selinux: ensure we cleanup the internal AVC counters on error in
avc_update()
Jeff Vander Stoep (1):
selinux: sidtab reverse lookup hash table
Ondrej Mosnacek (5):
selinux: cache the SID -> context string translation
selinux: treat atomic flags more carefully
selinux: reorder hooks to make runtime disable less broken
selinux: fix wrong buffer types in policydb.c
selinux: do not allocate ancillary buffer on first load
Paul Moore (4):
selinux: ensure we cleanup the internal AVC counters on error in
avc_insert()
selinux: ensure the policy has been loaded before reading the sidtab stats
selinux: deprecate disabling SELinux and runtime
selinux: remove redundant allocation and helper functions
Ravi Kumar Siddojigari (1):
selinux: move ibpkeys code under CONFIG_SECURITY_INFINIBAND.
Stephen Smalley (10):
security,lockdown,selinux: implement SELinux lockdown
selinux: revert "stop passing MAY_NOT_BLOCK to the AVC upon follow_link"
selinux: fall back to ref-walk if audit is required
selinux: clean up selinux_inode_permission MAY_NOT_BLOCK tests
security: only build lsm_audit if CONFIG_SECURITY=y
selinux: clean up selinux_enabled/disabled/enforcing_boot
selinux: randomize layout of key structures
Documentation,selinux: fix references to old selinuxfs mount point
selinux: make default_noexec read-only after init
selinux: fix regression introduced by move_mount(2) syscall
Yang Guo (1):
selinux: remove unnecessary selinux cred request
YueHaibing (1):
selinux: remove set but not used variable 'sidtab'
liuyang34 (1):
selinuxfs: use scnprintf to get real length for inode
Documentation/ABI/obsolete/sysfs-selinux-disable | 26 ++
Documentation/admin-guide/kernel-parameters.txt | 9 +-
MAINTAINERS | 1 +
include/linux/lsm_audit.h | 2 +
include/linux/security.h | 2 +
security/Makefile | 2 +-
security/lockdown/lockdown.c | 27 --
security/lsm_audit.c | 5 +
security/security.c | 33 ++
security/selinux/Kconfig | 33 +-
security/selinux/Makefile | 4 +-
security/selinux/avc.c | 95 +++---
security/selinux/hooks.c | 388 ++++++++++--------
security/selinux/ibpkey.c | 2 +-
security/selinux/include/avc.h | 13 +-
security/selinux/include/classmap.h | 2 +
security/selinux/include/ibpkey.h | 13 +-
security/selinux/include/objsec.h | 2 +-
security/selinux/include/security.h | 40 ++-
security/selinux/netif.c | 2 +-
security/selinux/netnode.c | 2 +-
security/selinux/netport.c | 2 +-
security/selinux/selinuxfs.c | 87 ++++-
security/selinux/ss/context.h | 11 +-
security/selinux/ss/policydb.c | 9 +-
security/selinux/ss/policydb.h | 2 +-
security/selinux/ss/services.c | 312 +++++++++++-------
security/selinux/ss/services.h | 6 +-
security/selinux/ss/sidtab.c | 402 ++++++++++++-------
security/selinux/ss/sidtab.h | 70 +++-
30 files changed, 1045 insertions(+), 559 deletions(-)
create mode 100644 Documentation/ABI/obsolete/sysfs-selinux-disable
--
paul moore
www.paul-moore.com
^ permalink raw reply
* Re: [PATCH v14 22/23] LSM: Add /proc attr entry for full LSM context
From: Casey Schaufler @ 2020-01-27 22:49 UTC (permalink / raw)
To: Stephen Smalley, casey.schaufler, jmorris, linux-security-module,
selinux
Cc: keescook, john.johansen, penguin-kernel, paul
In-Reply-To: <6bd3e393-e1df-7117-d15a-81cb1946807b@tycho.nsa.gov>
On 1/24/2020 12:16 PM, Stephen Smalley wrote:
> On 1/24/20 2:28 PM, Casey Schaufler wrote:
>> On 1/24/2020 8:20 AM, Stephen Smalley wrote:
>>> On 1/24/20 9:42 AM, Stephen Smalley wrote:
>>>> On 1/23/20 7:23 PM, Casey Schaufler 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.
>>>>>
>>>>> Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
>>>>> Cc: linux-api@vger.kernel.org
>>>>
>>>> As previously discussed, there are issues with AppArmor's implementation of getprocattr() particularly around the trailing newline that dbus-daemon and perhaps others would like to see go away in any new interface. Hence, I don't think we should implement this new API using the existing getprocattr() hook lest it also be locked into the current behavior forever.
>>>
>>> Also, it would be good if whatever hook is introduced to support /proc/pid/attr/context could also be leveraged by the SO_PEERCONTEXT implementation in the future so that we are guaranteed a consistent result between the two interfaces, unlike the current situation for /proc/self/attr/current versus SO_PEERSEC.
>>
>> I don't believe that a new hook is necessary, and that introducing one
>> just to deal with a '\n' would be pedantic. We really have two rational
>> options. AppArmor could drop the '\n' from their "context". Or, we can
>> simply document that the /proc/pid/attr/context interface will trim any
>> trailing whitespace. I understand that this would be a break from the
>> notion that the LSM infrastructure does not constrain what a module uses
>> for its own data. On the other hand, we have been saying that "context"s
>> are strings, and ignoring trailing whitespace is usual behavior for
>> strings.
>
> Well, you can either introduce a new common underlying hook for use by /proc/pid/attr/context and SO_PEERCONTEXT to produce the string that is to be returned to userspace (in order to guarantee consistency in format and allowing them to be directly compared, which I think is what the dbus maintainers wanted),
The getprocattr() hooks are already required to provide a nul terminated string.
The behavior of /proc/self/attr/current with regard to trailing whitespace or
the terminating nul is left to the security module.
The behavior of /proc/self/attr/context is new, and as such it can be defined
to be reasonable and consistent.
> or you can modify every security module to provide that guarantee in its existing getprocattr and getpeersec* hook functions (SELinux already provides this guarantee; Smack and AppArmor produce slightly different results with respect to \0 and/or \n), or you can have the framework trim the values it gets from the security modules before composing them.
Changing "every security module" turns out to be trivial, really.
The security_getprocattr() interface doesn't change, the getprocattr
hooks get a bool argument indicating if newlines are allowed, the
AppArmor code changes to check the bool and security_getprocattr()
sets to bool specially for the "context" case.
The getpeersec() interface use for SO_PEERCONTEXT isn't any more difficult.
The security modules are free to include a terminating nul or not in
the existing use case, but a bool can tell them what the behavior must
be when we get to that interface.
It's not like there are all that many security modules to bring in line
at this point. Establishing sane rules of behavior is overdue. With the
current set of new modules sniffing at the door it really is time to
tighten these up.
> But you need to do one of those things before this interface gets merged upstream.
>
> Aside from the trailing newline and \0 issues, AppArmor also has a whitespace-separated (mode) field that may or may not be present in the contexts it presently returns, ala "/usr/sbin/cupsd (enforce)". Not sure what they want for the new interfaces.
Using '\0' as a field terminator addresses that issue. That's a
primary motivator.
^ permalink raw reply
* Re: [PATCH v7 07/11] proc: flush task dcache entries from all procfs instances
From: Alexey Gladkov @ 2020-01-27 23:23 UTC (permalink / raw)
To: Linus Torvalds
Cc: LKML, Kernel Hardening, Linux API, Linux FS Devel,
Linux Security Module, Akinobu Mita, Alexander Viro,
Alexey Dobriyan, Andrew Morton, Andy Lutomirski, Daniel Micay,
Djalal Harouni, Dmitry V . Levin, Eric W . Biederman,
Greg Kroah-Hartman, Ingo Molnar, J . Bruce Fields, Jeff Layton,
Jonathan Corbet, Kees Cook, Oleg Nesterov, Solar Designer,
Stephen Rothwell
In-Reply-To: <CAHk-=wiGNSQCA8TYa1Akp0_GRpe=ELKDPkDX5nzM5R=oDy1U+Q@mail.gmail.com>
On Sat, Jan 25, 2020 at 10:45:25AM -0800, Linus Torvalds wrote:
> On Sat, Jan 25, 2020 at 5:06 AM Alexey Gladkov <gladkov.alexey@gmail.com> wrote:
> >
> > This allows to flush dcache entries of a task on multiple procfs mounts
> > per pid namespace.
>
> From a quick read-through, this is the only one I really react negatively to.
>
> The locking looks odd. It only seems to protect the new proc_mounts
> list, but then it's a whole big rwsem, and it's taken over all of
> proc_flush_task_mnt(), and the locking is exported to all over as a
> result of that - including the dummy functions for "there is no proc"
> case.
>
> And proc_flush_task_mnt() itself should need no locking over any of
> it, so it's all just for the silly looping over the list.
Thank you, I will rework this part.
> So
>
> (a) this looks fishy and feels wrong - I get a very strong feeling
> that the locking is wrong to begin with, and could/should have been
> done differently
>
> (b) all the locking should have been internal to /proc, and those
> wrappers shouldn't exist in a common header file (and certainly not
> for the non-proc case).
>
> Yes, (a) is just a feeling, and I don't have any great suggestions.
> Maybe make it an RCU list and use a spinlock for updating it?
I’m thinking, is it possible to get rid of proc_flush_task at all ?
Maybe we can try to flush dcache during readdir for example.
> But (b) is pretty much a non-starter in this form. Those wrappers
> shouldn't be in a globally exported core header file. No way.
>
> Linus
>
--
Rgrds, legion
^ permalink raw reply
* Re: [GIT PULL] SELinux patches for v5.6
From: pr-tracker-bot @ 2020-01-27 23:55 UTC (permalink / raw)
To: Paul Moore; +Cc: Linus Torvalds, selinux, linux-security-module, linux-kernel
In-Reply-To: <CAHC9VhR_5564up7u1V-PRXOz_RRfTLj16m508qgNvdWC8gcRTA@mail.gmail.com>
The pull request you sent on Mon, 27 Jan 2020 17:26:18 -0500:
> git://git.kernel.org/pub/scm/linux/kernel/git/pcmoore/selinux.git tags/selinux-pr-20200127
has been merged into torvalds/linux.git:
https://git.kernel.org/torvalds/c/b1dba2473114588be3df916bf629a61bdcc83737
Thank you!
--
Deet-doot-dot, I am a bot.
https://korg.wiki.kernel.org/userdoc/prtracker
^ permalink raw reply
* [GIT PULL] TOMOYO patches for v5.6
From: Tetsuo Handa @ 2020-01-28 4:17 UTC (permalink / raw)
To: Linus Torvalds; +Cc: linux-security-module
The following changes since commit a8772fad0172aeae339144598b809fd8d4823331:
tomoyo: Use atomic_t for statistics counter (2020-01-02 12:53:49 +0900)
are available in the git repository at:
git://git.osdn.net/gitroot/tomoyo/tomoyo-test1.git tags/tomoyo-pr-20200128
for you to fetch changes up to a8772fad0172aeae339144598b809fd8d4823331:
tomoyo: Use atomic_t for statistics counter (2020-01-02 12:53:49 +0900)
----------------------------------------------------------------
Hello, Linus.
One "int -> atomic_t" conversion patch for suppressing KCSAN's warning.
----------------------------------------------------------------
^ permalink raw reply
* [PATCH v6 00/10] Introduce CAP_PERFMON to secure system performance monitoring and observability
From: Alexey Budankov @ 2020-01-28 5:52 UTC (permalink / raw)
To: Peter Zijlstra, Arnaldo Carvalho de Melo, Ingo Molnar,
benh@kernel.crashing.org, Paul Mackerras, Michael Ellerman,
james.bottomley@hansenpartnership.com, Serge Hallyn, Will Deacon,
Robert Richter, Alexei Starovoitov
Cc: intel-gfx@lists.freedesktop.org, Jiri Olsa, Andi Kleen,
Stephane Eranian, Igor Lubashev, Alexander Shishkin, Namhyung Kim,
Song Liu, Lionel Landwerlin, linux-kernel,
linux-security-module@vger.kernel.org, selinux@vger.kernel.org,
linux-arm-kernel, linuxppc-dev@lists.ozlabs.org,
linux-parisc@vger.kernel.org, linux-perf-users@vger.kernel.org,
oprofile-list
Currently access to perf_events, i915_perf and other performance monitoring and
observability subsystems of the kernel is open only for a privileged process [1]
with CAP_SYS_ADMIN capability enabled in the process effective set [2].
This patch set introduces CAP_PERFMON capability designed to secure system
performance monitoring and observability operations so that CAP_PERFMON would
assist CAP_SYS_ADMIN capability in its governing role for performance monitoring
and observability subsystems of the kernel.
CAP_PERFMON intends to harden system security and integrity during system
performance monitoring and observability operations by decreasing attack surface
that is available to a CAP_SYS_ADMIN privileged process [2]. Providing the access
to system performance monitoring and observability operations under CAP_PERFMON
capability singly, without the rest of CAP_SYS_ADMIN credentials, excludes chances
to misuse the credentials and makes the operation more secure. Thus, CAP_PERFMON
implements the principal of least privilege for performance monitoring and
observability operations (POSIX IEEE 1003.1e: 2.2.2.39 principle of least
privilege: A security design principle that states that a process or program be
granted only those privileges (e.g., capabilities) necessary to accomplish its
legitimate function, and only for the time that such privileges are actually
required)
CAP_PERFMON intends to meet the demand to secure system performance monitoring
and observability operations for adoption in security sensitive, restricted,
multiuser production environments (e.g. HPC clusters, cloud and virtual compute
environments), where root or CAP_SYS_ADMIN credentials are not available to mass
users of a system, and securely unblock applicability and scalability of system
performance monitoring and observability operations beyond root or CAP_SYS_ADMIN
process use cases.
CAP_PERFMON intends to take over CAP_SYS_ADMIN credentials related to system
performance monitoring and observability operations and balance amount of
CAP_SYS_ADMIN credentials following the recommendations in the capabilities man
page [2] for CAP_SYS_ADMIN: "Note: this capability is overloaded; see Notes to
kernel developers, below." For backward compatibility reasons access to system
performance monitoring and observability subsystems of the kernel remains open
for CAP_SYS_ADMIN privileged processes but CAP_SYS_ADMIN capability usage for
secure system performance monitoring and observability operations is discouraged
with respect to the designed CAP_PERFMON capability.
Possible alternative solution to this system security hardening, capabilities
balancing task of making performance monitoring and observability operations
more accessible could be to use the existing CAP_SYS_PTRACE capability to govern
system performance monitoring and observability subsystems. However CAP_SYS_PTRACE
capability still provides users with more credentials than are required for secure
performance monitoring and observability operations and this excess is avoided by
the designed CAP_PERFMON capability.
Although software running under CAP_PERFMON can not ensure avoidance of related
hardware issues, the software can still mitigate those issues following the official
embargoed hardware issues mitigation procedure [3]. The bugs in the software itself
can be fixed following the standard kernel development process [4] to maintain and
harden security of system performance monitoring and observability operations.
Finally, the patch set is shaped in the way that simplifies backtracking procedure
of possible induced issues [5] as much as possible.
The patch set is for tip perf/core repository:
git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip perf/core
sha1: 56ee04aa63285d6bc8a995a26e2441ae3d419bcd
---
Changes in v6:
- avoided noaudit checks in perfmon_capable() to explicitly advertise CAP_PERFMON
usage thru audit logs to secure system performance monitoring and observability
Changes in v5:
- renamed CAP_SYS_PERFMON to CAP_PERFMON
- extended perfmon_capable() with noaudit checks
Changes in v4:
- converted perfmon_capable() into an inline function
- made perf_events kprobes, uprobes, hw breakpoints and namespaces data available
to CAP_SYS_PERFMON privileged processes
- applied perfmon_capable() to drivers/perf and drivers/oprofile
- extended __cmd_ftrace() with support of CAP_SYS_PERFMON
Changes in v3:
- implemented perfmon_capable() macros aggregating required capabilities checks
Changes in v2:
- made perf_events trace points available to CAP_SYS_PERFMON privileged processes
- made perf_event_paranoid_check() treat CAP_SYS_PERFMON equally to CAP_SYS_ADMIN
- applied CAP_SYS_PERFMON to i915_perf, bpf_trace, powerpc and parisc system
performance monitoring and observability related subsystems
---
Alexey Budankov (10):
capabilities: introduce CAP_PERFMON to kernel and user space
perf/core: open access to the core for CAP_PERFMON privileged process
perf/core: open access to probes for CAP_PERFMON privileged process
perf tool: extend Perf tool with CAP_PERFMON capability support
drm/i915/perf: open access for CAP_PERFMON privileged process
trace/bpf_trace: open access for CAP_PERFMON privileged process
powerpc/perf: open access for CAP_PERFMON privileged process
parisc/perf: open access for CAP_PERFMON privileged process
drivers/perf: open access for CAP_PERFMON privileged process
drivers/oprofile: open access for CAP_PERFMON privileged process
arch/parisc/kernel/perf.c | 2 +-
arch/powerpc/perf/imc-pmu.c | 4 ++--
drivers/gpu/drm/i915/i915_perf.c | 13 ++++++-------
drivers/oprofile/event_buffer.c | 2 +-
drivers/perf/arm_spe_pmu.c | 4 ++--
include/linux/capability.h | 4 ++++
include/linux/perf_event.h | 6 +++---
include/uapi/linux/capability.h | 8 +++++++-
kernel/events/core.c | 6 +++---
kernel/trace/bpf_trace.c | 2 +-
security/selinux/include/classmap.h | 4 ++--
tools/perf/builtin-ftrace.c | 5 +++--
tools/perf/design.txt | 3 ++-
tools/perf/util/cap.h | 4 ++++
tools/perf/util/evsel.c | 10 +++++-----
tools/perf/util/util.c | 1 +
16 files changed, 47 insertions(+), 31 deletions(-)
---
Testing and validation (Intel Skylake, 8 cores, Fedora 29, 5.5.0-rc3+, x86_64):
libcap library [6], [7], [8] and Perf tool can be used to apply CAP_PERFMON
capability for secure system performance monitoring and observability beyond the
scope permitted by the system wide perf_event_paranoid kernel setting [9] and
below are the steps for evaluation:
- patch, build and boot the kernel
- patch, build Perf tool e.g. to /home/user/perf
...
# git clone git://git.kernel.org/pub/scm/libs/libcap/libcap.git libcap
# pushd libcap
# patch libcap/include/uapi/linux/capabilities.h with [PATCH 1]
# make
# pushd progs
# ./setcap "cap_perfmon,cap_sys_ptrace,cap_syslog=ep" /home/user/perf
# ./setcap -v "cap_perfmon,cap_sys_ptrace,cap_syslog=ep" /home/user/perf
/home/user/perf: OK
# ./getcap /home/user/perf
/home/user/perf = cap_sys_ptrace,cap_syslog,cap_perfmon+ep
# echo 2 > /proc/sys/kernel/perf_event_paranoid
# cat /proc/sys/kernel/perf_event_paranoid
2
...
$ /home/user/perf top
... works as expected ...
$ cat /proc/`pidof perf`/status
Name: perf
Umask: 0002
State: S (sleeping)
Tgid: 2958
Ngid: 0
Pid: 2958
PPid: 9847
TracerPid: 0
Uid: 500 500 500 500
Gid: 500 500 500 500
FDSize: 256
...
CapInh: 0000000000000000
CapPrm: 0000004400080000
CapEff: 0000004400080000 => 01000100 00000000 00001000 00000000 00000000
cap_perfmon,cap_sys_ptrace,cap_syslog
CapBnd: 0000007fffffffff
CapAmb: 0000000000000000
NoNewPrivs: 0
Seccomp: 0
Speculation_Store_Bypass: thread vulnerable
Cpus_allowed: ff
Cpus_allowed_list: 0-7
...
Usage of cap_perfmon effectively avoids unused credentials excess:
- with cap_sys_admin:
CapEff: 0000007fffffffff => 01111111 11111111 11111111 11111111 11111111
- with cap_perfmon:
CapEff: 0000004400080000 => 01000100 00000000 00001000 00000000 00000000
38 34 19
perfmon syslog sys_ptrace
---
[1] https://www.kernel.org/doc/html/latest/admin-guide/perf-security.html
[2] http://man7.org/linux/man-pages/man7/capabilities.7.html
[3] https://www.kernel.org/doc/html/latest/process/embargoed-hardware-issues.html
[4] https://www.kernel.org/doc/html/latest/admin-guide/security-bugs.html
[5] https://www.kernel.org/doc/html/latest/process/management-style.html#decisions
[6] http://man7.org/linux/man-pages/man8/setcap.8.html
[7] https://git.kernel.org/pub/scm/libs/libcap/libcap.git
[8] https://sites.google.com/site/fullycapable/, posix_1003.1e-990310.pdf
[9] http://man7.org/linux/man-pages/man2/perf_event_open.2.html
--
2.20.1
^ permalink raw reply
* [PATCH v6 01/10] capabilities: introduce CAP_PERFMON to kernel and user space
From: Alexey Budankov @ 2020-01-28 6:07 UTC (permalink / raw)
To: Peter Zijlstra, Arnaldo Carvalho de Melo, Ingo Molnar,
benh@kernel.crashing.org, Paul Mackerras, Michael Ellerman,
james.bottomley@hansenpartnership.com, Serge Hallyn, Will Deacon,
Robert Richter, Alexei Starovoitov
Cc: intel-gfx@lists.freedesktop.org, Jiri Olsa, Andi Kleen,
Stephane Eranian, Igor Lubashev, Alexander Shishkin, Namhyung Kim,
Song Liu, Lionel Landwerlin, linux-kernel,
linux-security-module@vger.kernel.org, selinux@vger.kernel.org,
linux-arm-kernel, linuxppc-dev@lists.ozlabs.org,
linux-parisc@vger.kernel.org, linux-perf-users@vger.kernel.org,
oprofile-list
In-Reply-To: <74d524ab-ac11-a7b8-1052-eba10f117e09@linux.intel.com>
Introduce CAP_PERFMON capability designed to secure system performance
monitoring and observability operations so that CAP_PERFMON would assist
CAP_SYS_ADMIN capability in its governing role for performance monitoring
and observability subsystems.
CAP_PERFMON hardens system security and integrity during system performance
monitoring and observability operations by decreasing attack surface that
is available to a CAP_SYS_ADMIN privileged process [2]. Providing the access
to system performance monitoring and observability operations under CAP_PERFMON
capability singly, without the rest of CAP_SYS_ADMIN credentials, excludes
chances to misuse the credentials and makes the operation more secure.
Thus, CAP_PERFMON implements the principal of least privilege for performance
monitoring and observability operations (POSIX IEEE 1003.1e: 2.2.2.39 principle
of least privilege: A security design principle that states that a process
or program be granted only those privileges (e.g., capabilities) necessary
to accomplish its legitimate function, and only for the time that such
privileges are actually required)
CAP_PERFMON meets the demand to secure system performance monitoring and
observability operations for adoption in security sensitive, restricted,
multiuser production environments (e.g. HPC clusters, cloud and virtual compute
environments), where root or CAP_SYS_ADMIN credentials are not available to
mass users of a system, and securely unblocks applicability and scalability
of system performance monitoring and observability operations beyond root
and CAP_SYS_ADMIN process use cases.
CAP_PERFMON takes over CAP_SYS_ADMIN credentials related to system performance
monitoring and observability operations and balances amount of CAP_SYS_ADMIN
credentials following the recommendations in the capabilities man page [1]
for CAP_SYS_ADMIN: "Note: this capability is overloaded; see Notes to kernel
developers, below." For backward compatibility reasons access to system
performance monitoring and observability subsystems of the kernel remains
open for CAP_SYS_ADMIN privileged processes but CAP_SYS_ADMIN capability
usage for secure system performance monitoring and observability operations
is discouraged with respect to the designed CAP_PERFMON capability.
Although the software running under CAP_PERFMON can not ensure avoidance
of related hardware issues, the software can still mitigate these issues
following the official embargoed hardware issues mitigation procedure [2].
The bugs in the software itself can be fixed following the standard kernel
development process [3] to maintain and harden security of system performance
monitoring and observability operations.
[1] http://man7.org/linux/man-pages/man7/capabilities.7.html
[2] https://www.kernel.org/doc/html/latest/process/embargoed-hardware-issues.html
[3] https://www.kernel.org/doc/html/latest/admin-guide/security-bugs.html
Signed-off-by: Alexey Budankov <alexey.budankov@linux.intel.com>
---
include/linux/capability.h | 4 ++++
include/uapi/linux/capability.h | 8 +++++++-
security/selinux/include/classmap.h | 4 ++--
3 files changed, 13 insertions(+), 3 deletions(-)
diff --git a/include/linux/capability.h b/include/linux/capability.h
index ecce0f43c73a..027d7e4a853b 100644
--- a/include/linux/capability.h
+++ b/include/linux/capability.h
@@ -251,6 +251,10 @@ extern bool privileged_wrt_inode_uidgid(struct user_namespace *ns, const struct
extern bool capable_wrt_inode_uidgid(const struct inode *inode, int cap);
extern bool file_ns_capable(const struct file *file, struct user_namespace *ns, int cap);
extern bool ptracer_capable(struct task_struct *tsk, struct user_namespace *ns);
+static inline bool perfmon_capable(void)
+{
+ return capable(CAP_PERFMON) || capable(CAP_SYS_ADMIN);
+}
/* audit system wants to get cap info from files as well */
extern int get_vfs_caps_from_disk(const struct dentry *dentry, struct cpu_vfs_cap_data *cpu_caps);
diff --git a/include/uapi/linux/capability.h b/include/uapi/linux/capability.h
index 240fdb9a60f6..8b416e5f3afa 100644
--- a/include/uapi/linux/capability.h
+++ b/include/uapi/linux/capability.h
@@ -366,8 +366,14 @@ struct vfs_ns_cap_data {
#define CAP_AUDIT_READ 37
+/*
+ * Allow system performance and observability privileged operations
+ * using perf_events, i915_perf and other kernel subsystems
+ */
+
+#define CAP_PERFMON 38
-#define CAP_LAST_CAP CAP_AUDIT_READ
+#define CAP_LAST_CAP CAP_PERFMON
#define cap_valid(x) ((x) >= 0 && (x) <= CAP_LAST_CAP)
diff --git a/security/selinux/include/classmap.h b/security/selinux/include/classmap.h
index 7db24855e12d..c599b0c2b0e7 100644
--- a/security/selinux/include/classmap.h
+++ b/security/selinux/include/classmap.h
@@ -27,9 +27,9 @@
"audit_control", "setfcap"
#define COMMON_CAP2_PERMS "mac_override", "mac_admin", "syslog", \
- "wake_alarm", "block_suspend", "audit_read"
+ "wake_alarm", "block_suspend", "audit_read", "perfmon"
-#if CAP_LAST_CAP > CAP_AUDIT_READ
+#if CAP_LAST_CAP > CAP_PERFMON
#error New capability defined, please update COMMON_CAP2_PERMS.
#endif
--
2.20.1
^ permalink raw reply related
* [PATCH v6 02/10] perf/core: open access to the core for CAP_PERFMON privileged process
From: Alexey Budankov @ 2020-01-28 6:08 UTC (permalink / raw)
To: Peter Zijlstra, Arnaldo Carvalho de Melo, Ingo Molnar,
benh@kernel.crashing.org, Paul Mackerras, Michael Ellerman,
james.bottomley@hansenpartnership.com, Serge Hallyn, Will Deacon,
Robert Richter, Alexei Starovoitov
Cc: intel-gfx@lists.freedesktop.org, Jiri Olsa, Andi Kleen,
Stephane Eranian, Igor Lubashev, Alexander Shishkin, Namhyung Kim,
Song Liu, Lionel Landwerlin, linux-kernel,
linux-security-module@vger.kernel.org, selinux@vger.kernel.org,
linux-arm-kernel, linuxppc-dev@lists.ozlabs.org,
linux-parisc@vger.kernel.org, linux-perf-users@vger.kernel.org,
oprofile-list
In-Reply-To: <74d524ab-ac11-a7b8-1052-eba10f117e09@linux.intel.com>
Open access to monitoring of kernel code, cpus, tracepoints and namespaces
data for a CAP_PERFMON privileged process. Providing the access under
CAP_PERFMON capability singly, without the rest of CAP_SYS_ADMIN credentials,
excludes chances to misuse the credentials and makes operation more secure.
CAP_PERFMON implements the principal of least privilege for performance
monitoring and observability operations (POSIX IEEE 1003.1e 2.2.2.39 principle
of least privilege: A security design principle that states that a process or
program be granted only those privileges (e.g., capabilities) necessary to
accomplish its legitimate function, and only for the time that such privileges
are actually required)
For backward compatibility reasons access to perf_events subsystem remains
open for CAP_SYS_ADMIN privileged processes but CAP_SYS_ADMIN usage for secure
perf_events monitoring is discouraged with respect to CAP_PERFMON capability.
Signed-off-by: Alexey Budankov <alexey.budankov@linux.intel.com>
---
include/linux/perf_event.h | 6 +++---
kernel/events/core.c | 2 +-
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/include/linux/perf_event.h b/include/linux/perf_event.h
index 6d4c22aee384..730469babcc2 100644
--- a/include/linux/perf_event.h
+++ b/include/linux/perf_event.h
@@ -1285,7 +1285,7 @@ static inline int perf_is_paranoid(void)
static inline int perf_allow_kernel(struct perf_event_attr *attr)
{
- if (sysctl_perf_event_paranoid > 1 && !capable(CAP_SYS_ADMIN))
+ if (sysctl_perf_event_paranoid > 1 && !perfmon_capable())
return -EACCES;
return security_perf_event_open(attr, PERF_SECURITY_KERNEL);
@@ -1293,7 +1293,7 @@ static inline int perf_allow_kernel(struct perf_event_attr *attr)
static inline int perf_allow_cpu(struct perf_event_attr *attr)
{
- if (sysctl_perf_event_paranoid > 0 && !capable(CAP_SYS_ADMIN))
+ if (sysctl_perf_event_paranoid > 0 && !perfmon_capable())
return -EACCES;
return security_perf_event_open(attr, PERF_SECURITY_CPU);
@@ -1301,7 +1301,7 @@ static inline int perf_allow_cpu(struct perf_event_attr *attr)
static inline int perf_allow_tracepoint(struct perf_event_attr *attr)
{
- if (sysctl_perf_event_paranoid > -1 && !capable(CAP_SYS_ADMIN))
+ if (sysctl_perf_event_paranoid > -1 && !perfmon_capable())
return -EPERM;
return security_perf_event_open(attr, PERF_SECURITY_TRACEPOINT);
diff --git a/kernel/events/core.c b/kernel/events/core.c
index 2173c23c25b4..d956c81bd310 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -11186,7 +11186,7 @@ SYSCALL_DEFINE5(perf_event_open,
}
if (attr.namespaces) {
- if (!capable(CAP_SYS_ADMIN))
+ if (!perfmon_capable())
return -EACCES;
}
--
2.20.1
^ permalink raw reply related
* [PATCH v6 03/10] perf/core: open access to probes for CAP_PERFMON privileged process
From: Alexey Budankov @ 2020-01-28 6:09 UTC (permalink / raw)
To: Peter Zijlstra, Arnaldo Carvalho de Melo, Ingo Molnar,
benh@kernel.crashing.org, Paul Mackerras, Michael Ellerman,
james.bottomley@hansenpartnership.com, Serge Hallyn, Will Deacon,
Robert Richter, Alexei Starovoitov
Cc: intel-gfx@lists.freedesktop.org, Jiri Olsa, Andi Kleen,
Stephane Eranian, Igor Lubashev, Alexander Shishkin, Namhyung Kim,
Song Liu, Lionel Landwerlin, linux-kernel,
linux-security-module@vger.kernel.org, selinux@vger.kernel.org,
linux-arm-kernel, linuxppc-dev@lists.ozlabs.org,
linux-parisc@vger.kernel.org, linux-perf-users@vger.kernel.org,
oprofile-list
In-Reply-To: <74d524ab-ac11-a7b8-1052-eba10f117e09@linux.intel.com>
Open access to monitoring via kprobes and uprobes and eBPF tracing for
CAP_PERFMON privileged process. Providing the access under CAP_PERFMON
capability singly, without the rest of CAP_SYS_ADMIN credentials, excludes
chances to misuse the credentials and makes operation more secure.
perf kprobes and uprobes are used by ftrace and eBPF. perf probe uses
ftrace to define new kprobe events, and those events are treated as
tracepoint events. eBPF defines new probes via perf_event_open interface
and then the probes are used in eBPF tracing.
CAP_PERFMON implements the principal of least privilege for performance
monitoring and observability operations (POSIX IEEE 1003.1e 2.2.2.39 principle
of least privilege: A security design principle that states that a process or
program be granted only those privileges (e.g., capabilities) necessary to
accomplish its legitimate function, and only for the time that such privileges
are actually required)
For backward compatibility reasons access to perf_events subsystem remains
open for CAP_SYS_ADMIN privileged processes but CAP_SYS_ADMIN usage for
secure perf_events monitoring is discouraged with respect to CAP_PERFMON
capability.
Signed-off-by: Alexey Budankov <alexey.budankov@linux.intel.com>
---
kernel/events/core.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/kernel/events/core.c b/kernel/events/core.c
index d956c81bd310..c6453320ffea 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -9088,7 +9088,7 @@ static int perf_kprobe_event_init(struct perf_event *event)
if (event->attr.type != perf_kprobe.type)
return -ENOENT;
- if (!capable(CAP_SYS_ADMIN))
+ if (!perfmon_capable())
return -EACCES;
/*
@@ -9148,7 +9148,7 @@ static int perf_uprobe_event_init(struct perf_event *event)
if (event->attr.type != perf_uprobe.type)
return -ENOENT;
- if (!capable(CAP_SYS_ADMIN))
+ if (!perfmon_capable())
return -EACCES;
/*
--
2.20.1
^ permalink raw reply related
* [PATCH v6 04/10] perf tool: extend Perf tool with CAP_PERFMON capability support
From: Alexey Budankov @ 2020-01-28 6:09 UTC (permalink / raw)
To: Peter Zijlstra, Arnaldo Carvalho de Melo, Ingo Molnar,
benh@kernel.crashing.org, Paul Mackerras, Michael Ellerman,
james.bottomley@hansenpartnership.com, Serge Hallyn, Will Deacon,
Robert Richter, Alexei Starovoitov
Cc: intel-gfx@lists.freedesktop.org, Jiri Olsa, Andi Kleen,
Stephane Eranian, Igor Lubashev, Alexander Shishkin, Namhyung Kim,
Song Liu, Lionel Landwerlin, linux-kernel,
linux-security-module@vger.kernel.org, selinux@vger.kernel.org,
linux-arm-kernel, linuxppc-dev@lists.ozlabs.org,
linux-parisc@vger.kernel.org, linux-perf-users@vger.kernel.org,
oprofile-list
In-Reply-To: <74d524ab-ac11-a7b8-1052-eba10f117e09@linux.intel.com>
Extend error messages to mention CAP_PERFMON capability as an option
to substitute CAP_SYS_ADMIN capability for secure system performance
monitoring and observability operations. Make perf_event_paranoid_check()
and __cmd_ftrace() to be aware of CAP_PERFMON capability.
CAP_PERFMON implements the principal of least privilege for performance
monitoring and observability operations (POSIX IEEE 1003.1e 2.2.2.39 principle
of least privilege: A security design principle that states that a process
or program be granted only those privileges (e.g., capabilities) necessary
to accomplish its legitimate function, and only for the time that such
privileges are actually required)
For backward compatibility reasons access to perf_events subsystem remains
open for CAP_SYS_ADMIN privileged processes but CAP_SYS_ADMIN usage for
secure perf_events monitoring is discouraged with respect to CAP_PERFMON
capability.
Signed-off-by: Alexey Budankov <alexey.budankov@linux.intel.com>
---
tools/perf/builtin-ftrace.c | 5 +++--
tools/perf/design.txt | 3 ++-
tools/perf/util/cap.h | 4 ++++
tools/perf/util/evsel.c | 10 +++++-----
tools/perf/util/util.c | 1 +
5 files changed, 15 insertions(+), 8 deletions(-)
diff --git a/tools/perf/builtin-ftrace.c b/tools/perf/builtin-ftrace.c
index d5adc417a4ca..55eda54240fb 100644
--- a/tools/perf/builtin-ftrace.c
+++ b/tools/perf/builtin-ftrace.c
@@ -284,10 +284,11 @@ static int __cmd_ftrace(struct perf_ftrace *ftrace, int argc, const char **argv)
.events = POLLIN,
};
- if (!perf_cap__capable(CAP_SYS_ADMIN)) {
+ if (!(perf_cap__capable(CAP_PERFMON) ||
+ perf_cap__capable(CAP_SYS_ADMIN))) {
pr_err("ftrace only works for %s!\n",
#ifdef HAVE_LIBCAP_SUPPORT
- "users with the SYS_ADMIN capability"
+ "users with the CAP_PERFMON or CAP_SYS_ADMIN capability"
#else
"root"
#endif
diff --git a/tools/perf/design.txt b/tools/perf/design.txt
index 0453ba26cdbd..a42fab308ff6 100644
--- a/tools/perf/design.txt
+++ b/tools/perf/design.txt
@@ -258,7 +258,8 @@ gets schedule to. Per task counters can be created by any user, for
their own tasks.
A 'pid == -1' and 'cpu == x' counter is a per CPU counter that counts
-all events on CPU-x. Per CPU counters need CAP_SYS_ADMIN privilege.
+all events on CPU-x. Per CPU counters need CAP_PERFMON or CAP_SYS_ADMIN
+privilege.
The 'flags' parameter is currently unused and must be zero.
diff --git a/tools/perf/util/cap.h b/tools/perf/util/cap.h
index 051dc590ceee..ae52878c0b2e 100644
--- a/tools/perf/util/cap.h
+++ b/tools/perf/util/cap.h
@@ -29,4 +29,8 @@ static inline bool perf_cap__capable(int cap __maybe_unused)
#define CAP_SYSLOG 34
#endif
+#ifndef CAP_PERFMON
+#define CAP_PERFMON 38
+#endif
+
#endif /* __PERF_CAP_H */
diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
index a69e64236120..a35f17723dd3 100644
--- a/tools/perf/util/evsel.c
+++ b/tools/perf/util/evsel.c
@@ -2491,14 +2491,14 @@ int perf_evsel__open_strerror(struct evsel *evsel, struct target *target,
"You may not have permission to collect %sstats.\n\n"
"Consider tweaking /proc/sys/kernel/perf_event_paranoid,\n"
"which controls use of the performance events system by\n"
- "unprivileged users (without CAP_SYS_ADMIN).\n\n"
+ "unprivileged users (without CAP_PERFMON or CAP_SYS_ADMIN).\n\n"
"The current value is %d:\n\n"
" -1: Allow use of (almost) all events by all users\n"
" Ignore mlock limit after perf_event_mlock_kb without CAP_IPC_LOCK\n"
- ">= 0: Disallow ftrace function tracepoint by users without CAP_SYS_ADMIN\n"
- " Disallow raw tracepoint access by users without CAP_SYS_ADMIN\n"
- ">= 1: Disallow CPU event access by users without CAP_SYS_ADMIN\n"
- ">= 2: Disallow kernel profiling by users without CAP_SYS_ADMIN\n\n"
+ ">= 0: Disallow ftrace function tracepoint by users without CAP_PERFMON or CAP_SYS_ADMIN\n"
+ " Disallow raw tracepoint access by users without CAP_SYS_PERFMON or CAP_SYS_ADMIN\n"
+ ">= 1: Disallow CPU event access by users without CAP_PERFMON or CAP_SYS_ADMIN\n"
+ ">= 2: Disallow kernel profiling by users without CAP_PERFMON or CAP_SYS_ADMIN\n\n"
"To make this setting permanent, edit /etc/sysctl.conf too, e.g.:\n\n"
" kernel.perf_event_paranoid = -1\n" ,
target->system_wide ? "system-wide " : "",
diff --git a/tools/perf/util/util.c b/tools/perf/util/util.c
index 969ae560dad9..51cf3071db74 100644
--- a/tools/perf/util/util.c
+++ b/tools/perf/util/util.c
@@ -272,6 +272,7 @@ int perf_event_paranoid(void)
bool perf_event_paranoid_check(int max_level)
{
return perf_cap__capable(CAP_SYS_ADMIN) ||
+ perf_cap__capable(CAP_PERFMON) ||
perf_event_paranoid() <= max_level;
}
--
2.20.1
^ permalink raw reply related
* [PATCH v6 05/10] drm/i915/perf: open access for CAP_PERFMON privileged process
From: Alexey Budankov @ 2020-01-28 6:10 UTC (permalink / raw)
To: Peter Zijlstra, Arnaldo Carvalho de Melo, Ingo Molnar,
benh@kernel.crashing.org, Paul Mackerras, Michael Ellerman,
james.bottomley@hansenpartnership.com, Serge Hallyn, Will Deacon,
Robert Richter, Alexei Starovoitov
Cc: intel-gfx@lists.freedesktop.org, Jiri Olsa, Andi Kleen,
Stephane Eranian, Igor Lubashev, Alexander Shishkin, Namhyung Kim,
Song Liu, Lionel Landwerlin, linux-kernel,
linux-security-module@vger.kernel.org, selinux@vger.kernel.org,
linux-arm-kernel, linuxppc-dev@lists.ozlabs.org,
linux-parisc@vger.kernel.org, linux-perf-users@vger.kernel.org,
oprofile-list
In-Reply-To: <74d524ab-ac11-a7b8-1052-eba10f117e09@linux.intel.com>
Open access to i915_perf monitoring for CAP_PERFMON privileged process.
Providing the access under CAP_PERFMON capability singly, without the
rest of CAP_SYS_ADMIN credentials, excludes chances to misuse the
credentials and makes operation more secure.
CAP_PERFMON implements the principal of least privilege for performance
monitoring and observability operations (POSIX IEEE 1003.1e 2.2.2.39 principle
of least privilege: A security design principle that states that a process
or program be granted only those privileges (e.g., capabilities) necessary
to accomplish its legitimate function, and only for the time that such
privileges are actually required)
For backward compatibility reasons access to i915_events subsystem remains
open for CAP_SYS_ADMIN privileged processes but CAP_SYS_ADMIN usage for
secure i915_events monitoring is discouraged with respect to CAP_PERFMON
capability.
Signed-off-by: Alexey Budankov <alexey.budankov@linux.intel.com>
---
drivers/gpu/drm/i915/i915_perf.c | 13 ++++++-------
1 file changed, 6 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c
index 2ae14bc14931..d89347861b7d 100644
--- a/drivers/gpu/drm/i915/i915_perf.c
+++ b/drivers/gpu/drm/i915/i915_perf.c
@@ -3375,10 +3375,10 @@ i915_perf_open_ioctl_locked(struct i915_perf *perf,
/* Similar to perf's kernel.perf_paranoid_cpu sysctl option
* we check a dev.i915.perf_stream_paranoid sysctl option
* to determine if it's ok to access system wide OA counters
- * without CAP_SYS_ADMIN privileges.
+ * without CAP_PERFMON or CAP_SYS_ADMIN privileges.
*/
if (privileged_op &&
- i915_perf_stream_paranoid && !capable(CAP_SYS_ADMIN)) {
+ i915_perf_stream_paranoid && !perfmon_capable()) {
DRM_DEBUG("Insufficient privileges to open i915 perf stream\n");
ret = -EACCES;
goto err_ctx;
@@ -3571,9 +3571,8 @@ static int read_properties_unlocked(struct i915_perf *perf,
} else
oa_freq_hz = 0;
- if (oa_freq_hz > i915_oa_max_sample_rate &&
- !capable(CAP_SYS_ADMIN)) {
- DRM_DEBUG("OA exponent would exceed the max sampling frequency (sysctl dev.i915.oa_max_sample_rate) %uHz without root privileges\n",
+ if (oa_freq_hz > i915_oa_max_sample_rate && !perfmon_capable()) {
+ DRM_DEBUG("OA exponent would exceed the max sampling frequency (sysctl dev.i915.oa_max_sample_rate) %uHz without CAP_PERFMON or CAP_SYS_ADMIN privileges\n",
i915_oa_max_sample_rate);
return -EACCES;
}
@@ -3994,7 +3993,7 @@ int i915_perf_add_config_ioctl(struct drm_device *dev, void *data,
return -EINVAL;
}
- if (i915_perf_stream_paranoid && !capable(CAP_SYS_ADMIN)) {
+ if (i915_perf_stream_paranoid && !perfmon_capable()) {
DRM_DEBUG("Insufficient privileges to add i915 OA config\n");
return -EACCES;
}
@@ -4141,7 +4140,7 @@ int i915_perf_remove_config_ioctl(struct drm_device *dev, void *data,
return -ENOTSUPP;
}
- if (i915_perf_stream_paranoid && !capable(CAP_SYS_ADMIN)) {
+ if (i915_perf_stream_paranoid && !perfmon_capable()) {
DRM_DEBUG("Insufficient privileges to remove i915 OA config\n");
return -EACCES;
}
--
2.20.1
^ permalink raw reply related
* [PATCH v6 06/10] trace/bpf_trace: open access for CAP_PERFMON privileged process
From: Alexey Budankov @ 2020-01-28 6:11 UTC (permalink / raw)
To: Peter Zijlstra, Arnaldo Carvalho de Melo, Ingo Molnar,
benh@kernel.crashing.org, Paul Mackerras, Michael Ellerman,
james.bottomley@hansenpartnership.com, Serge Hallyn, Will Deacon,
Robert Richter, Alexei Starovoitov
Cc: intel-gfx@lists.freedesktop.org, Jiri Olsa, Andi Kleen,
Stephane Eranian, Igor Lubashev, Alexander Shishkin, Namhyung Kim,
Song Liu, Lionel Landwerlin, linux-kernel,
linux-security-module@vger.kernel.org, selinux@vger.kernel.org,
linux-arm-kernel, linuxppc-dev@lists.ozlabs.org,
linux-parisc@vger.kernel.org, linux-perf-users@vger.kernel.org,
oprofile-list
In-Reply-To: <74d524ab-ac11-a7b8-1052-eba10f117e09@linux.intel.com>
Open access to bpf_trace monitoring for CAP_PERFMON privileged process.
Providing the access under CAP_PERFMON capability singly, without the
rest of CAP_SYS_ADMIN credentials, excludes chances to misuse the
credentials and makes operation more secure.
CAP_PERFMON implements the principal of least privilege for performance
monitoring and observability operations (POSIX IEEE 1003.1e 2.2.2.39 principle
of least privilege: A security design principle that states that a process
or program be granted only those privileges (e.g., capabilities) necessary
to accomplish its legitimate function, and only for the time that such
privileges are actually required)
For backward compatibility reasons access to bpf_trace monitoring remains
open for CAP_SYS_ADMIN privileged processes but CAP_SYS_ADMIN usage for
secure bpf_trace monitoring is discouraged with respect to CAP_PERFMON
capability.
Signed-off-by: Alexey Budankov <alexey.budankov@linux.intel.com>
---
kernel/trace/bpf_trace.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
index e5ef4ae9edb5..334f1d71ebb1 100644
--- a/kernel/trace/bpf_trace.c
+++ b/kernel/trace/bpf_trace.c
@@ -1395,7 +1395,7 @@ int perf_event_query_prog_array(struct perf_event *event, void __user *info)
u32 *ids, prog_cnt, ids_len;
int ret;
- if (!capable(CAP_SYS_ADMIN))
+ if (!perfmon_capable())
return -EPERM;
if (event->attr.type != PERF_TYPE_TRACEPOINT)
return -EINVAL;
--
2.20.1
^ permalink raw reply related
* [PATCH v6 07/10] powerpc/perf: open access for CAP_PERFMON privileged process
From: Alexey Budankov @ 2020-01-28 6:12 UTC (permalink / raw)
To: Peter Zijlstra, Arnaldo Carvalho de Melo, Ingo Molnar,
benh@kernel.crashing.org, Paul Mackerras, Michael Ellerman,
james.bottomley@hansenpartnership.com, Serge Hallyn, Will Deacon,
Robert Richter, Alexei Starovoitov
Cc: intel-gfx@lists.freedesktop.org, Jiri Olsa, Andi Kleen,
Stephane Eranian, Igor Lubashev, Alexander Shishkin, Namhyung Kim,
Song Liu, Lionel Landwerlin, linux-kernel,
linux-security-module@vger.kernel.org, selinux@vger.kernel.org,
linux-arm-kernel, linuxppc-dev@lists.ozlabs.org,
linux-parisc@vger.kernel.org, linux-perf-users@vger.kernel.org,
oprofile-list
In-Reply-To: <74d524ab-ac11-a7b8-1052-eba10f117e09@linux.intel.com>
Open access to monitoring for CAP_PERFMON privileged process.
Providing the access under CAP_PERFMON capability singly, without the
rest of CAP_SYS_ADMIN credentials, excludes chances to misuse the
credentials and makes operation more secure.
CAP_PERFMON implements the principal of least privilege for performance
monitoring and observability operations (POSIX IEEE 1003.1e 2.2.2.39 principle
of least privilege: A security design principle that states that a process
or program be granted only those privileges (e.g., capabilities) necessary
to accomplish its legitimate function, and only for the time that such
privileges are actually required)
For backward compatibility reasons access to the monitoring remains open
for CAP_SYS_ADMIN privileged processes but CAP_SYS_ADMIN usage for secure
monitoring is discouraged with respect to CAP_PERFMON capability.
Signed-off-by: Alexey Budankov <alexey.budankov@linux.intel.com>
---
arch/powerpc/perf/imc-pmu.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/powerpc/perf/imc-pmu.c b/arch/powerpc/perf/imc-pmu.c
index cb50a9e1fd2d..e837717492e4 100644
--- a/arch/powerpc/perf/imc-pmu.c
+++ b/arch/powerpc/perf/imc-pmu.c
@@ -898,7 +898,7 @@ static int thread_imc_event_init(struct perf_event *event)
if (event->attr.type != event->pmu->type)
return -ENOENT;
- if (!capable(CAP_SYS_ADMIN))
+ if (!perfmon_capable())
return -EACCES;
/* Sampling not supported */
@@ -1307,7 +1307,7 @@ static int trace_imc_event_init(struct perf_event *event)
if (event->attr.type != event->pmu->type)
return -ENOENT;
- if (!capable(CAP_SYS_ADMIN))
+ if (!perfmon_capable())
return -EACCES;
/* Return if this is a couting event */
--
2.20.1
^ permalink raw reply related
* [PATCH v6 08/10] parisc/perf: open access for CAP_PERFMON privileged process
From: Alexey Budankov @ 2020-01-28 6:12 UTC (permalink / raw)
To: Peter Zijlstra, Arnaldo Carvalho de Melo, Ingo Molnar,
benh@kernel.crashing.org, Paul Mackerras, Michael Ellerman,
james.bottomley@hansenpartnership.com, Serge Hallyn, Will Deacon,
Robert Richter, Alexei Starovoitov
Cc: intel-gfx@lists.freedesktop.org, Jiri Olsa, Andi Kleen,
Stephane Eranian, Igor Lubashev, Alexander Shishkin, Namhyung Kim,
Song Liu, Lionel Landwerlin, linux-kernel,
linux-security-module@vger.kernel.org, selinux@vger.kernel.org,
linux-arm-kernel, linuxppc-dev@lists.ozlabs.org,
linux-parisc@vger.kernel.org, linux-perf-users@vger.kernel.org,
oprofile-list
In-Reply-To: <74d524ab-ac11-a7b8-1052-eba10f117e09@linux.intel.com>
Open access to monitoring for CAP_PERFMON privileged process.
Providing the access under CAP_PERFMON capability singly, without the
rest of CAP_SYS_ADMIN credentials, excludes chances to misuse the
credentials and makes operation more secure.
CAP_PERFMON implements the principal of least privilege for performance
monitoring and observability operations (POSIX IEEE 1003.1e 2.2.2.39 principle
of least privilege: A security design principle that states that a process
or program be granted only those privileges (e.g., capabilities) necessary
to accomplish its legitimate function, and only for the time that such
privileges are actually required)
For backward compatibility reasons access to the monitoring remains open
for CAP_SYS_ADMIN privileged processes but CAP_SYS_ADMIN usage for secure
monitoring is discouraged with respect to CAP_PERFMON capability.
Signed-off-by: Alexey Budankov <alexey.budankov@linux.intel.com>
---
arch/parisc/kernel/perf.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/parisc/kernel/perf.c b/arch/parisc/kernel/perf.c
index 676683641d00..c4208d027794 100644
--- a/arch/parisc/kernel/perf.c
+++ b/arch/parisc/kernel/perf.c
@@ -300,7 +300,7 @@ static ssize_t perf_write(struct file *file, const char __user *buf,
else
return -EFAULT;
- if (!capable(CAP_SYS_ADMIN))
+ if (!perfmon_capable())
return -EACCES;
if (count != sizeof(uint32_t))
--
2.20.1
^ permalink raw reply related
* [PATCH v6 09/10] drivers/perf: open access for CAP_PERFMON privileged process
From: Alexey Budankov @ 2020-01-28 6:13 UTC (permalink / raw)
To: Peter Zijlstra, Arnaldo Carvalho de Melo, Ingo Molnar,
benh@kernel.crashing.org, Paul Mackerras, Michael Ellerman,
james.bottomley@hansenpartnership.com, Serge Hallyn, Will Deacon,
Robert Richter, Alexei Starovoitov
Cc: intel-gfx@lists.freedesktop.org, Jiri Olsa, Andi Kleen,
Stephane Eranian, Igor Lubashev, Alexander Shishkin, Namhyung Kim,
Song Liu, Lionel Landwerlin, linux-kernel,
linux-security-module@vger.kernel.org, selinux@vger.kernel.org,
linux-arm-kernel, linuxppc-dev@lists.ozlabs.org,
linux-parisc@vger.kernel.org, linux-perf-users@vger.kernel.org,
oprofile-list
In-Reply-To: <74d524ab-ac11-a7b8-1052-eba10f117e09@linux.intel.com>
Open access to monitoring for CAP_PERFMON privileged process.
Providing the access under CAP_PERFMON capability singly, without the
rest of CAP_SYS_ADMIN credentials, excludes chances to misuse the
credentials and makes operation more secure.
CAP_PERFMON implements the principal of least privilege for performance
monitoring and observability operations (POSIX IEEE 1003.1e 2.2.2.39 principle
of least privilege: A security design principle that states that a process
or program be granted only those privileges (e.g., capabilities) necessary
to accomplish its legitimate function, and only for the time that such
privileges are actually required)
For backward compatibility reasons access to the monitoring remains open
for CAP_SYS_ADMIN privileged processes but CAP_SYS_ADMIN usage for secure
monitoring is discouraged with respect to CAP_PERFMON capability.
Signed-off-by: Alexey Budankov <alexey.budankov@linux.intel.com>
---
drivers/perf/arm_spe_pmu.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/perf/arm_spe_pmu.c b/drivers/perf/arm_spe_pmu.c
index 4e4984a55cd1..5dff81bc3324 100644
--- a/drivers/perf/arm_spe_pmu.c
+++ b/drivers/perf/arm_spe_pmu.c
@@ -274,7 +274,7 @@ static u64 arm_spe_event_to_pmscr(struct perf_event *event)
if (!attr->exclude_kernel)
reg |= BIT(SYS_PMSCR_EL1_E1SPE_SHIFT);
- if (IS_ENABLED(CONFIG_PID_IN_CONTEXTIDR) && capable(CAP_SYS_ADMIN))
+ if (IS_ENABLED(CONFIG_PID_IN_CONTEXTIDR) && perfmon_capable())
reg |= BIT(SYS_PMSCR_EL1_CX_SHIFT);
return reg;
@@ -700,7 +700,7 @@ static int arm_spe_pmu_event_init(struct perf_event *event)
return -EOPNOTSUPP;
reg = arm_spe_event_to_pmscr(event);
- if (!capable(CAP_SYS_ADMIN) &&
+ if (!perfmon_capable() &&
(reg & (BIT(SYS_PMSCR_EL1_PA_SHIFT) |
BIT(SYS_PMSCR_EL1_CX_SHIFT) |
BIT(SYS_PMSCR_EL1_PCT_SHIFT))))
--
2.20.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