* [PATCH 1/6] ima/ima_boot_aggregate: Fix the definition of event log
2019-01-15 2:14 [PATCH v2 0/6] LTP IMA fix bundle Jia Zhang
@ 2019-01-15 2:14 ` Jia Zhang
2019-01-15 2:14 ` [PATCH 2/6] ima/ima_boot_aggregate: Don't hard code the length of sha1 hash Jia Zhang
` (4 subsequent siblings)
5 siblings, 0 replies; 13+ messages in thread
From: Jia Zhang @ 2019-01-15 2:14 UTC (permalink / raw)
To: zohar, pvorel; +Cc: linux-integrity, ltp, zhang.jia
According to [1], the structure of event log should be packed,
and certain fields should be 32-bit unsigned integer. Fortunately,
keeping natural alignment seems to make everything working as
expected all the time.
[1] page 17,18 @https://trustedcomputinggroup.org/wp-content/uploads/TCG_EFI_Protocol_1_22_Final-v05.pdf
Signed-off-by: Jia Zhang <zhang.jia@linux.alibaba.com>
---
testcases/kernel/security/integrity/ima/src/ima_boot_aggregate.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/testcases/kernel/security/integrity/ima/src/ima_boot_aggregate.c b/testcases/kernel/security/integrity/ima/src/ima_boot_aggregate.c
index f6e7be0..d85d222 100644
--- a/testcases/kernel/security/integrity/ima/src/ima_boot_aggregate.c
+++ b/testcases/kernel/security/integrity/ima/src/ima_boot_aggregate.c
@@ -53,10 +53,10 @@ int main(int argc, char *argv[])
struct {
struct {
u_int32_t pcr;
- int type;
- unsigned char digest[SHA_DIGEST_LENGTH];
- u_int16_t len;
- } header;
+ u_int32_t type;
+ u_int8_t digest[SHA_DIGEST_LENGTH];
+ u_int32_t len;
+ } header __attribute__ ((packed));
char *data;
} event;
struct {
--
1.8.3.1
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH 2/6] ima/ima_boot_aggregate: Don't hard code the length of sha1 hash
2019-01-15 2:14 [PATCH v2 0/6] LTP IMA fix bundle Jia Zhang
2019-01-15 2:14 ` [PATCH 1/6] ima/ima_boot_aggregate: Fix the definition of event log Jia Zhang
@ 2019-01-15 2:14 ` Jia Zhang
2019-01-15 2:14 ` [PATCH 3/6] ima/ima_boot_aggregate: Fix extending PCRs beyond PCR 0-7 Jia Zhang
` (3 subsequent siblings)
5 siblings, 0 replies; 13+ messages in thread
From: Jia Zhang @ 2019-01-15 2:14 UTC (permalink / raw)
To: zohar, pvorel; +Cc: linux-integrity, ltp, zhang.jia
Instead, use SHA_DIGEST_LENGTH.
Signed-off-by: Jia Zhang <zhang.jia@linux.alibaba.com>
Reviewed-by: Mimi Zohar <zohar@linux.ibm.com>
---
testcases/kernel/security/integrity/ima/src/ima_boot_aggregate.c | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/testcases/kernel/security/integrity/ima/src/ima_boot_aggregate.c b/testcases/kernel/security/integrity/ima/src/ima_boot_aggregate.c
index d85d222..67be6a7 100644
--- a/testcases/kernel/security/integrity/ima/src/ima_boot_aggregate.c
+++ b/testcases/kernel/security/integrity/ima/src/ima_boot_aggregate.c
@@ -42,7 +42,7 @@ static void display_sha1_digest(unsigned char *pcr)
{
int i;
- for (i = 0; i < 20; i++)
+ for (i = 0; i < SHA_DIGEST_LENGTH; i++)
printf("%02x", *(pcr + i) & 0xff);
printf("\n");
}
@@ -94,8 +94,9 @@ int main(int argc, char *argv[])
display_sha1_digest(event.header.digest);
}
SHA1_Init(&c);
- SHA1_Update(&c, pcr[event.header.pcr].digest, 20);
- SHA1_Update(&c, event.header.digest, 20);
+ SHA1_Update(&c, pcr[event.header.pcr].digest,
+ SHA_DIGEST_LENGTH);
+ SHA1_Update(&c, event.header.digest, SHA_DIGEST_LENGTH);
SHA1_Final(pcr[event.header.pcr].digest, &c);
#if MAX_EVENT_DATA_SIZE < USHRT_MAX
if (event.header.len > MAX_EVENT_DATA_SIZE) {
@@ -116,7 +117,7 @@ int main(int argc, char *argv[])
printf("PCR-%2.2x: ", i);
display_sha1_digest(pcr[i].digest);
}
- SHA1_Update(&c, pcr[i].digest, 20);
+ SHA1_Update(&c, pcr[i].digest, SHA_DIGEST_LENGTH);
}
SHA1_Final(boot_aggregate, &c);
--
1.8.3.1
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH 3/6] ima/ima_boot_aggregate: Fix extending PCRs beyond PCR 0-7
2019-01-15 2:14 [PATCH v2 0/6] LTP IMA fix bundle Jia Zhang
2019-01-15 2:14 ` [PATCH 1/6] ima/ima_boot_aggregate: Fix the definition of event log Jia Zhang
2019-01-15 2:14 ` [PATCH 2/6] ima/ima_boot_aggregate: Don't hard code the length of sha1 hash Jia Zhang
@ 2019-01-15 2:14 ` Jia Zhang
2019-01-15 2:14 ` [PATCH 4/6] ima: Code cleanup Jia Zhang
` (2 subsequent siblings)
5 siblings, 0 replies; 13+ messages in thread
From: Jia Zhang @ 2019-01-15 2:14 UTC (permalink / raw)
To: zohar, pvorel; +Cc: linux-integrity, ltp, zhang.jia
The boot aggragate calculation should never touch PCRs beyond PCR 0-7,
even a PCR extension really manipulates out-of-domain PCRs.
Signed-off-by: Jia Zhang <zhang.jia@linux.alibaba.com>
Reviewed-by: Mimi Zohar <zohar@linux.ibm.com>
---
.../security/integrity/ima/src/ima_boot_aggregate.c | 15 ++++++++++-----
1 file changed, 10 insertions(+), 5 deletions(-)
diff --git a/testcases/kernel/security/integrity/ima/src/ima_boot_aggregate.c b/testcases/kernel/security/integrity/ima/src/ima_boot_aggregate.c
index 67be6a7..98893b9 100644
--- a/testcases/kernel/security/integrity/ima/src/ima_boot_aggregate.c
+++ b/testcases/kernel/security/integrity/ima/src/ima_boot_aggregate.c
@@ -93,11 +93,16 @@ int main(int argc, char *argv[])
printf("%03u ", event.header.pcr);
display_sha1_digest(event.header.digest);
}
- SHA1_Init(&c);
- SHA1_Update(&c, pcr[event.header.pcr].digest,
- SHA_DIGEST_LENGTH);
- SHA1_Update(&c, event.header.digest, SHA_DIGEST_LENGTH);
- SHA1_Final(pcr[event.header.pcr].digest, &c);
+
+ if (event.header.pcr < NUM_PCRS) {
+ SHA1_Init(&c);
+ SHA1_Update(&c, pcr[event.header.pcr].digest,
+ SHA_DIGEST_LENGTH);
+ SHA1_Update(&c, event.header.digest,
+ SHA_DIGEST_LENGTH);
+ SHA1_Final(pcr[event.header.pcr].digest, &c);
+ }
+
#if MAX_EVENT_DATA_SIZE < USHRT_MAX
if (event.header.len > MAX_EVENT_DATA_SIZE) {
printf("Error event too long\n");
--
1.8.3.1
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH 4/6] ima: Code cleanup
2019-01-15 2:14 [PATCH v2 0/6] LTP IMA fix bundle Jia Zhang
` (2 preceding siblings ...)
2019-01-15 2:14 ` [PATCH 3/6] ima/ima_boot_aggregate: Fix extending PCRs beyond PCR 0-7 Jia Zhang
@ 2019-01-15 2:14 ` Jia Zhang
2019-01-15 2:14 ` [PATCH 5/6] ima: Rename the folder name for policy files to datafiles Jia Zhang
2019-01-15 2:14 ` [PATCH 6/6] ima/ima_violations: Temporarily remove the printk rate limit Jia Zhang
5 siblings, 0 replies; 13+ messages in thread
From: Jia Zhang @ 2019-01-15 2:14 UTC (permalink / raw)
To: zohar, pvorel; +Cc: linux-integrity, ltp, zhang.jia
- Change the legacy function name from PATH_CHECK to FILE_CHECK.
- Use the variable IMA_POLICY instead of hard code path.
Signed-off-by: Jia Zhang <zhang.jia@linux.alibaba.com>
Reviewed-by: Mimi Zohar <zohar@linux.ibm.com>
---
testcases/kernel/security/integrity/ima/policy/measure.policy | 2 +-
testcases/kernel/security/integrity/ima/policy/measure.policy-invalid | 2 +-
testcases/kernel/security/integrity/ima/tests/ima_policy.sh | 2 +-
3 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/testcases/kernel/security/integrity/ima/policy/measure.policy b/testcases/kernel/security/integrity/ima/policy/measure.policy
index c68e722..9976ddf 100644
--- a/testcases/kernel/security/integrity/ima/policy/measure.policy
+++ b/testcases/kernel/security/integrity/ima/policy/measure.policy
@@ -13,4 +13,4 @@ dont_measure fsmagic=0x01021994
dont_measure fsmagic=0x73636673
measure func=FILE_MMAP mask=MAY_EXEC
measure func=BPRM_CHECK mask=MAY_EXEC
-measure func=PATH_CHECK mask=MAY_READ uid=0
+measure func=FILE_CHECK mask=MAY_READ uid=0
diff --git a/testcases/kernel/security/integrity/ima/policy/measure.policy-invalid b/testcases/kernel/security/integrity/ima/policy/measure.policy-invalid
index e406757..04dff89 100644
--- a/testcases/kernel/security/integrity/ima/policy/measure.policy-invalid
+++ b/testcases/kernel/security/integrity/ima/policy/measure.policy-invalid
@@ -13,4 +13,4 @@ dont_measure fsmagic=0x01021994
dnt_measure fsmagic=0x73636673
measure func=FILE_MMAP mask=MAY_EXEC
measure func=BPRM_CHECK mask=MAY_EXEC
-measure func=PATH_CHECK mask=MAY_READ uid=0
+measure func=FILE_CHECK mask=MAY_READ uid=0
diff --git a/testcases/kernel/security/integrity/ima/tests/ima_policy.sh b/testcases/kernel/security/integrity/ima/tests/ima_policy.sh
index 64aa8cb..a0c7869 100755
--- a/testcases/kernel/security/integrity/ima/tests/ima_policy.sh
+++ b/testcases/kernel/security/integrity/ima/tests/ima_policy.sh
@@ -28,7 +28,7 @@ check_policy_writable()
{
local err="IMA policy already loaded and kernel not configured to enable multiple writes to it (need CONFIG_IMA_WRITE_POLICY=y)"
- [ -f /sys/kernel/security/ima/policy ] || tst_brk TCONF "$err"
+ [ -f $IMA_POLICY ] || tst_brk TCONF "$err"
# CONFIG_IMA_READ_POLICY
echo "" 2> log > $IMA_POLICY
grep -q "Device or resource busy" log && tst_brk TCONF "$err"
--
1.8.3.1
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH 5/6] ima: Rename the folder name for policy files to datafiles
2019-01-15 2:14 [PATCH v2 0/6] LTP IMA fix bundle Jia Zhang
` (3 preceding siblings ...)
2019-01-15 2:14 ` [PATCH 4/6] ima: Code cleanup Jia Zhang
@ 2019-01-15 2:14 ` Jia Zhang
2019-01-15 2:14 ` [PATCH 6/6] ima/ima_violations: Temporarily remove the printk rate limit Jia Zhang
5 siblings, 0 replies; 13+ messages in thread
From: Jia Zhang @ 2019-01-15 2:14 UTC (permalink / raw)
To: zohar, pvorel; +Cc: linux-integrity, ltp, zhang.jia
If we choose to run ima_policy.sh locally without installation,
a failure message is reported as following:
ima_policy 1 TCONF: missing <path>/ltp/testcases/kernel/security/integrity/ima/datafiles/measure.policy
TST_DATAROOT would be extended to datafiles but the policy files
are actually placed under policy.
In order to make it easier, just rename the folder name to datafiles.
Signed-off-by: Jia Zhang <zhang.jia@linux.alibaba.com>
---
testcases/kernel/security/integrity/ima/Makefile | 2 +-
.../security/integrity/ima/datafiles/Makefile | 31 ++++++++++++++++++++++
.../integrity/ima/datafiles/measure.policy | 16 +++++++++++
.../integrity/ima/datafiles/measure.policy-invalid | 16 +++++++++++
.../kernel/security/integrity/ima/policy/Makefile | 31 ----------------------
.../security/integrity/ima/policy/measure.policy | 16 -----------
.../integrity/ima/policy/measure.policy-invalid | 16 -----------
7 files changed, 64 insertions(+), 64 deletions(-)
create mode 100644 testcases/kernel/security/integrity/ima/datafiles/Makefile
create mode 100644 testcases/kernel/security/integrity/ima/datafiles/measure.policy
create mode 100644 testcases/kernel/security/integrity/ima/datafiles/measure.policy-invalid
delete mode 100644 testcases/kernel/security/integrity/ima/policy/Makefile
delete mode 100644 testcases/kernel/security/integrity/ima/policy/measure.policy
delete mode 100644 testcases/kernel/security/integrity/ima/policy/measure.policy-invalid
diff --git a/testcases/kernel/security/integrity/ima/Makefile b/testcases/kernel/security/integrity/ima/Makefile
index 1290e6f..19b10ff 100644
--- a/testcases/kernel/security/integrity/ima/Makefile
+++ b/testcases/kernel/security/integrity/ima/Makefile
@@ -24,6 +24,6 @@ top_srcdir ?= ../../../../..
include $(top_srcdir)/include/mk/env_pre.mk
-SUBDIRS := policy src tests
+SUBDIRS := datafiles src tests
include $(top_srcdir)/include/mk/generic_trunk_target.mk
diff --git a/testcases/kernel/security/integrity/ima/datafiles/Makefile b/testcases/kernel/security/integrity/ima/datafiles/Makefile
new file mode 100644
index 0000000..a960f9d
--- /dev/null
+++ b/testcases/kernel/security/integrity/ima/datafiles/Makefile
@@ -0,0 +1,31 @@
+#
+# testcases/kernel/security/integrity/ima/policy testcases Makefile.
+#
+# Copyright (C) 2009, Cisco Systems Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License along
+# with this program; if not, write to the Free Software Foundation, Inc.,
+# 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+#
+# Ngie Cooper, July 2009
+#
+
+top_srcdir ?= ../../../../../..
+
+include $(top_srcdir)/include/mk/env_pre.mk
+
+INSTALL_DIR := testcases/data/ima_policy
+
+INSTALL_TARGETS := measure*
+
+include $(top_srcdir)/include/mk/generic_leaf_target.mk
diff --git a/testcases/kernel/security/integrity/ima/datafiles/measure.policy b/testcases/kernel/security/integrity/ima/datafiles/measure.policy
new file mode 100644
index 0000000..9976ddf
--- /dev/null
+++ b/testcases/kernel/security/integrity/ima/datafiles/measure.policy
@@ -0,0 +1,16 @@
+#
+# Integrity measure policy
+#
+# PROC_SUPER_MAGIC
+dont_measure fsmagic=0x9fa0
+# SYSFS_MAGIC
+dont_measure fsmagic=0x62656572
+# DEBUGFS_MAGIC
+dont_measure fsmagic=0x64626720
+# TMPFS_MAGIC
+dont_measure fsmagic=0x01021994
+# SECURITYFS_MAGIC
+dont_measure fsmagic=0x73636673
+measure func=FILE_MMAP mask=MAY_EXEC
+measure func=BPRM_CHECK mask=MAY_EXEC
+measure func=FILE_CHECK mask=MAY_READ uid=0
diff --git a/testcases/kernel/security/integrity/ima/datafiles/measure.policy-invalid b/testcases/kernel/security/integrity/ima/datafiles/measure.policy-invalid
new file mode 100644
index 0000000..04dff89
--- /dev/null
+++ b/testcases/kernel/security/integrity/ima/datafiles/measure.policy-invalid
@@ -0,0 +1,16 @@
+#
+# Integrity measure policy
+#
+# PROC_SUPER_MAGIC
+dont_measure fsmagic=0x9fa0
+# SYSFS_MAGIC
+dont_measure fsmagic=0x62656572
+# DEBUGFS_MAGIC
+dont_measure fsmagic=0x64626720
+# TMPFS_MAGIC
+dont_measure fsmagic=0x01021994
+# SECURITYFS_MAGIC
+dnt_measure fsmagic=0x73636673
+measure func=FILE_MMAP mask=MAY_EXEC
+measure func=BPRM_CHECK mask=MAY_EXEC
+measure func=FILE_CHECK mask=MAY_READ uid=0
diff --git a/testcases/kernel/security/integrity/ima/policy/Makefile b/testcases/kernel/security/integrity/ima/policy/Makefile
deleted file mode 100644
index a960f9d..0000000
--- a/testcases/kernel/security/integrity/ima/policy/Makefile
+++ /dev/null
@@ -1,31 +0,0 @@
-#
-# testcases/kernel/security/integrity/ima/policy testcases Makefile.
-#
-# Copyright (C) 2009, Cisco Systems Inc.
-#
-# This program is free software; you can redistribute it and/or modify
-# it under the terms of the GNU General Public License as published by
-# the Free Software Foundation; either version 2 of the License, or
-# (at your option) any later version.
-#
-# This program is distributed in the hope that it will be useful,
-# but WITHOUT ANY WARRANTY; without even the implied warranty of
-# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
-# GNU General Public License for more details.
-#
-# You should have received a copy of the GNU General Public License along
-# with this program; if not, write to the Free Software Foundation, Inc.,
-# 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
-#
-# Ngie Cooper, July 2009
-#
-
-top_srcdir ?= ../../../../../..
-
-include $(top_srcdir)/include/mk/env_pre.mk
-
-INSTALL_DIR := testcases/data/ima_policy
-
-INSTALL_TARGETS := measure*
-
-include $(top_srcdir)/include/mk/generic_leaf_target.mk
diff --git a/testcases/kernel/security/integrity/ima/policy/measure.policy b/testcases/kernel/security/integrity/ima/policy/measure.policy
deleted file mode 100644
index 9976ddf..0000000
--- a/testcases/kernel/security/integrity/ima/policy/measure.policy
+++ /dev/null
@@ -1,16 +0,0 @@
-#
-# Integrity measure policy
-#
-# PROC_SUPER_MAGIC
-dont_measure fsmagic=0x9fa0
-# SYSFS_MAGIC
-dont_measure fsmagic=0x62656572
-# DEBUGFS_MAGIC
-dont_measure fsmagic=0x64626720
-# TMPFS_MAGIC
-dont_measure fsmagic=0x01021994
-# SECURITYFS_MAGIC
-dont_measure fsmagic=0x73636673
-measure func=FILE_MMAP mask=MAY_EXEC
-measure func=BPRM_CHECK mask=MAY_EXEC
-measure func=FILE_CHECK mask=MAY_READ uid=0
diff --git a/testcases/kernel/security/integrity/ima/policy/measure.policy-invalid b/testcases/kernel/security/integrity/ima/policy/measure.policy-invalid
deleted file mode 100644
index 04dff89..0000000
--- a/testcases/kernel/security/integrity/ima/policy/measure.policy-invalid
+++ /dev/null
@@ -1,16 +0,0 @@
-#
-# Integrity measure policy
-#
-# PROC_SUPER_MAGIC
-dont_measure fsmagic=0x9fa0
-# SYSFS_MAGIC
-dont_measure fsmagic=0x62656572
-# DEBUGFS_MAGIC
-dont_measure fsmagic=0x64626720
-# TMPFS_MAGIC
-dont_measure fsmagic=0x01021994
-# SECURITYFS_MAGIC
-dnt_measure fsmagic=0x73636673
-measure func=FILE_MMAP mask=MAY_EXEC
-measure func=BPRM_CHECK mask=MAY_EXEC
-measure func=FILE_CHECK mask=MAY_READ uid=0
--
1.8.3.1
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH 6/6] ima/ima_violations: Temporarily remove the printk rate limit
2019-01-15 2:14 [PATCH v2 0/6] LTP IMA fix bundle Jia Zhang
` (4 preceding siblings ...)
2019-01-15 2:14 ` [PATCH 5/6] ima: Rename the folder name for policy files to datafiles Jia Zhang
@ 2019-01-15 2:14 ` Jia Zhang
2019-01-15 15:07 ` Petr Vorel
5 siblings, 1 reply; 13+ messages in thread
From: Jia Zhang @ 2019-01-15 2:14 UTC (permalink / raw)
To: zohar, pvorel; +Cc: linux-integrity, ltp, zhang.jia
The output frequency of audit log is limited by printk_ratelimit()
in kernel if auditd not used. Thus, the test heavily depending on
searching keywords in log file may fail if the matching patterns
are exactly suppressed by printk_ratelimit().
In order to fix such a sort of failure, just temporarily remove
the limit, and recover its setting at the end of test.
Signed-off-by: Jia Zhang <zhang.jia@linux.alibaba.com>
---
.../kernel/security/integrity/ima/tests/ima_violations.sh | 14 ++++++++++++++
1 file changed, 14 insertions(+)
diff --git a/testcases/kernel/security/integrity/ima/tests/ima_violations.sh b/testcases/kernel/security/integrity/ima/tests/ima_violations.sh
index f3f40d4..a1360b8 100755
--- a/testcases/kernel/security/integrity/ima/tests/ima_violations.sh
+++ b/testcases/kernel/security/integrity/ima/tests/ima_violations.sh
@@ -31,15 +31,27 @@ setup()
FILE="test.txt"
IMA_VIOLATIONS="$SECURITYFS/ima/violations"
LOG="/var/log/messages"
+ PRINTK_RATE_LIMIT="0"
if status_daemon auditd; then
LOG="/var/log/audit/audit.log"
+ else
+ tst_check_cmds sysctl
+
+ PRINTK_RATE_LIMIT=`sysctl -n kernel.printk_ratelimit`
+ sysctl -wq kernel.printk_ratelimit=0
fi
[ -f "$LOG" ] || \
tst_brk TBROK "log $LOG does not exist (bug in detection?)"
tst_res TINFO "using log $LOG"
}
+reset_printk_ratelimit()
+{
+ [ "$PRINTK_RATE_LIMIT" != "0" ] && \
+ sysctl -wq kernel.printk_ratelimit=$PRINTK_RATE_LIMIT
+}
+
open_file_read()
{
exec 3< $FILE || exit 1
@@ -151,6 +163,8 @@ test3()
validate $num_violations $count $search
+ reset_printk_ratelimit
+
# wait for ima_mmap to exit, so we can umount
tst_sleep 2s
}
--
1.8.3.1
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH 6/6] ima/ima_violations: Temporarily remove the printk rate limit
2019-01-15 2:14 ` [PATCH 6/6] ima/ima_violations: Temporarily remove the printk rate limit Jia Zhang
@ 2019-01-15 15:07 ` Petr Vorel
2019-01-15 16:42 ` Jia Zhang
0 siblings, 1 reply; 13+ messages in thread
From: Petr Vorel @ 2019-01-15 15:07 UTC (permalink / raw)
To: Jia Zhang, zohar; +Cc: linux-integrity, ltp
Hi Jia, Mimi,
...
> +reset_printk_ratelimit()
> +{
> + [ "$PRINTK_RATE_LIMIT" != "0" ] && \
> + sysctl -wq kernel.printk_ratelimit=$PRINTK_RATE_LIMIT
> +}
...
> @@ -151,6 +163,8 @@ test3()
> validate $num_violations $count $search
> + reset_printk_ratelimit
This should be called in cleanup function.
Actually as it's called only once I'd put it into cleanup function.
No need to resent the patchset, if it's the only change, I change it before
merge.
@Mimi: do you agree with this change?
I've seen some issues on some distros when not using auditd daemon,
but now cannot reproduce them any more.
Kind regards,
Petr
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH 6/6] ima/ima_violations: Temporarily remove the printk rate limit
2019-01-15 15:07 ` Petr Vorel
@ 2019-01-15 16:42 ` Jia Zhang
2019-01-15 17:24 ` Petr Vorel
0 siblings, 1 reply; 13+ messages in thread
From: Jia Zhang @ 2019-01-15 16:42 UTC (permalink / raw)
To: Petr Vorel, zohar; +Cc: linux-integrity, ltp
On 2019/1/15 下午11:07, Petr Vorel wrote:
> Hi Jia, Mimi,
>
> ...
>> +reset_printk_ratelimit()
>> +{
>> + [ "$PRINTK_RATE_LIMIT" != "0" ] && \
>> + sysctl -wq kernel.printk_ratelimit=$PRINTK_RATE_LIMIT
>> +}
> ...
>> @@ -151,6 +163,8 @@ test3()
>
>> validate $num_violations $count $search
>
>> + reset_printk_ratelimit
> This should be called in cleanup function.
> Actually as it's called only once I'd put it into cleanup function.
I recognized that ima_setup.sh already defines a cleanup() so I cannot
define another cleanup function in ima_violations.sh. Do you mean moving
reset_printk_ratelimit into cleanup() defined in ima_setup.sh?
Thanks,
Jia
> No need to resent the patchset, if it's the only change, I change it before
> merge.
>
> @Mimi: do you agree with this change?
> I've seen some issues on some distros when not using auditd daemon,
> but now cannot reproduce them any more.
>
> Kind regards,
> Petr
>
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH 6/6] ima/ima_violations: Temporarily remove the printk rate limit
2019-01-15 16:42 ` Jia Zhang
@ 2019-01-15 17:24 ` Petr Vorel
2019-01-16 2:53 ` Jia Zhang
0 siblings, 1 reply; 13+ messages in thread
From: Petr Vorel @ 2019-01-15 17:24 UTC (permalink / raw)
To: Jia Zhang; +Cc: zohar, linux-integrity, ltp
Hi Jia,
> >> + reset_printk_ratelimit
> > This should be called in cleanup function.
> > Actually as it's called only once I'd put it into cleanup function.
> I recognized that ima_setup.sh already defines a cleanup() so I cannot
> define another cleanup function in ima_violations.sh. Do you mean moving
> reset_printk_ratelimit into cleanup() defined in ima_setup.sh?
No. You can define redefine local cleanup function in ima_violations.sh after
loading ima_setup.sh and call ima_cleanup() in it.
+ you can change setup in ima_setup.sh:
TST_CLEANUP="${TST_CLEANUP:-ima_cleanup}"
(see testcases/network/stress/ipsec/ipsec_lib.sh and testcases/network/stress/ipsec/ipsec_lib.sh and it's usage).
Cleanup of something defined in setup should be always in cleanup function and
not in any tests as it's called always after tst_brk() calls (some tests might
get skipped, so cleanup cannot be in them).
Kind regards,
Petr
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH 6/6] ima/ima_violations: Temporarily remove the printk rate limit
2019-01-15 17:24 ` Petr Vorel
@ 2019-01-16 2:53 ` Jia Zhang
0 siblings, 0 replies; 13+ messages in thread
From: Jia Zhang @ 2019-01-16 2:53 UTC (permalink / raw)
To: Petr Vorel; +Cc: zohar, linux-integrity, ltp
On 2019/1/16 上午1:24, Petr Vorel wrote:
> Hi Jia,
>
>>>> + reset_printk_ratelimit
>>> This should be called in cleanup function.
>>> Actually as it's called only once I'd put it into cleanup function.
>
>> I recognized that ima_setup.sh already defines a cleanup() so I cannot
>> define another cleanup function in ima_violations.sh. Do you mean moving
>> reset_printk_ratelimit into cleanup() defined in ima_setup.sh?
> No. You can define redefine local cleanup function in ima_violations.sh after
> loading ima_setup.sh and call ima_cleanup() in it.
>
> + you can change setup in ima_setup.sh:
> TST_CLEANUP="${TST_CLEANUP:-ima_cleanup}"
> (see testcases/network/stress/ipsec/ipsec_lib.sh and testcases/network/stress/ipsec/ipsec_lib.sh and it's usage).
>
> Cleanup of something defined in setup should be always in cleanup function and
> not in any tests as it's called always after tst_brk() calls (some tests might
> get skipped, so cleanup cannot be in them).
Thanks for the comments. I will send V3 to address this.
Jia
>
> Kind regards,
> Petr
>
^ permalink raw reply [flat|nested] 13+ messages in thread