Re: [PATCH v2] integrity: Add declarations to init_once void arguments.

[Jiele Zhao]

Okay, thank you. I'll keep an eye on that !

On 2021/4/10 2:55, Mimi Zohar wrote:

Hi Jiele,

On Wed, 2021-04-07 at 01:44 +, Jiele Zhao wrote:

init_once is a callback to kmem_cache_create. The parameter
type of this function is void *, so it's better to give a
explicit cast here.

Signed-off-by: Jiele Zhao 
---
  security/integrity/iint.c | 2 +-
  security/integrity/ima/ima_main.c | 2 +-
  2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/security/integrity/iint.c b/security/integrity/iint.c
index 0ba01847e836..fca8a9409e4a 100644
--- a/security/integrity/iint.c
+++ b/security/integrity/iint.c
@@ -160,7 +160,7 @@ void integrity_inode_free(struct inode *inode)
  
  static void init_once(void *foo)

  {
-   struct integrity_iint_cache *iint = foo;
+   struct integrity_iint_cache *iint = (struct integrity_iint_cache *) foo;
  
  	memset(iint, 0, sizeof(*iint));

iint->ima_file_status = INTEGRITY_UNKNOWN;
diff --git a/security/integrity/ima/ima_main.c 
b/security/integrity/ima/ima_main.c
index 9ef748ea829f..03bef720ab44 100644
--- a/security/integrity/ima/ima_main.c
+++ b/security/integrity/ima/ima_main.c
@@ -482,7 +482,7 @@ int ima_bprm_check(struct linux_binprm *bprm)
  }
  
  /**

- * ima_path_check - based on policy, collect/store measurement.
+ * ima_file_check - based on policy, collect/store measurement.
   * @file: pointer to the file to be measured
   * @mask: contains MAY_READ, MAY_WRITE, MAY_EXEC or MAY_APPEND
   *

This change was already posted as "ima: Fix function name error in
comment".  I've dropped this hunk.  In the future, please review your
patch line by line before posting.

Applied to
git://git.kernel.org/pub/scm/linux/kernel/git/zohar/linux-integrity.git
next-integrity

thanks,

Mimi

Re: [PATCH v2] integrity: Add declarations to init_once void arguments.

[Jiele Zhao]

Hi Mimi,

And this is another patch that has been modified.

On 2021/4/7 9:44, Jiele Zhao wrote:

init_once is a callback to kmem_cache_create. The parameter
type of this function is void *, so it's better to give a
explicit cast here.

Signed-off-by: Jiele Zhao 
---
  security/integrity/iint.c | 2 +-
  security/integrity/ima/ima_main.c | 2 +-
  2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/security/integrity/iint.c b/security/integrity/iint.c
index 0ba01847e836..fca8a9409e4a 100644
--- a/security/integrity/iint.c
+++ b/security/integrity/iint.c
@@ -160,7 +160,7 @@ void integrity_inode_free(struct inode *inode)
  
  static void init_once(void *foo)

  {
-   struct integrity_iint_cache *iint = foo;
+   struct integrity_iint_cache *iint = (struct integrity_iint_cache *) foo;
  
  	memset(iint, 0, sizeof(*iint));

iint->ima_file_status = INTEGRITY_UNKNOWN;
diff --git a/security/integrity/ima/ima_main.c 
b/security/integrity/ima/ima_main.c
index 9ef748ea829f..03bef720ab44 100644
--- a/security/integrity/ima/ima_main.c
+++ b/security/integrity/ima/ima_main.c
@@ -482,7 +482,7 @@ int ima_bprm_check(struct linux_binprm *bprm)
  }
  
  /**

- * ima_path_check - based on policy, collect/store measurement.
+ * ima_file_check - based on policy, collect/store measurement.
   * @file: pointer to the file to be measured
   * @mask: contains MAY_READ, MAY_WRITE, MAY_EXEC or MAY_APPEND
   *

Re: [PATCH v2] ima: Fix function name error in comment.

[Jiele Zhao]

Hi Mimi,

On 2021/4/6 10:12, Jiele Zhao wrote:

The original function name was ima_path_check().  The policy parsing
still supports PATH_CHECK.   Commit 9bbb6cad0173 ("ima: rename
ima_path_check to ima_file_check") renamed the function to
ima_file_check(), but missed modifying the function name in the
comment.

Fixes: 9bbb6cad0173 ("ima: rename ima_path_check to ima_file_check").

Signed-off-by: Jiele Zhao 
---
  security/integrity/ima/ima_main.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/security/integrity/ima/ima_main.c 
b/security/integrity/ima/ima_main.c
index 9ef748ea829f..03bef720ab44 100644
--- a/security/integrity/ima/ima_main.c
+++ b/security/integrity/ima/ima_main.c
@@ -482,7 +482,7 @@ int ima_bprm_check(struct linux_binprm *bprm)
  }
  
  /**

- * ima_path_check - based on policy, collect/store measurement.
+ * ima_file_check - based on policy, collect/store measurement.
   * @file: pointer to the file to be measured
   * @mask: contains MAY_READ, MAY_WRITE, MAY_EXEC or MAY_APPEND
   *


This is the [patch v2] based on your suggestion. Does this meet the 
requirements ?

[PATCH v2] integrity: Add declarations to init_once void arguments.

[Jiele Zhao]

init_once is a callback to kmem_cache_create. The parameter
type of this function is void *, so it's better to give a
explicit cast here.

Signed-off-by: Jiele Zhao 
---
 security/integrity/iint.c | 2 +-
 security/integrity/ima/ima_main.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/security/integrity/iint.c b/security/integrity/iint.c
index 0ba01847e836..fca8a9409e4a 100644
--- a/security/integrity/iint.c
+++ b/security/integrity/iint.c
@@ -160,7 +160,7 @@ void integrity_inode_free(struct inode *inode)
 
 static void init_once(void *foo)
 {
-   struct integrity_iint_cache *iint = foo;
+   struct integrity_iint_cache *iint = (struct integrity_iint_cache *) foo;
 
memset(iint, 0, sizeof(*iint));
iint->ima_file_status = INTEGRITY_UNKNOWN;
diff --git a/security/integrity/ima/ima_main.c 
b/security/integrity/ima/ima_main.c
index 9ef748ea829f..03bef720ab44 100644
--- a/security/integrity/ima/ima_main.c
+++ b/security/integrity/ima/ima_main.c
@@ -482,7 +482,7 @@ int ima_bprm_check(struct linux_binprm *bprm)
 }
 
 /**
- * ima_path_check - based on policy, collect/store measurement.
+ * ima_file_check - based on policy, collect/store measurement.
  * @file: pointer to the file to be measured
  * @mask: contains MAY_READ, MAY_WRITE, MAY_EXEC or MAY_APPEND
  *
-- 
2.25.1

Re: [PATCH] integrity/ima: Add declarations to init_once void arguments.

[Jiele Zhao]

Hi Mimi,

Please see if this is a useful suggestion.

On 2021/4/6 10:38, James Morris wrote:

On Tue, 6 Apr 2021, Jiele Zhao wrote:


Ping.

Mimi Zohar is the maintainer for this code.


On 2021/3/23 9:33, Jiele Zhao wrote:

init_once is a callback to kmem_cache_create. The parameter
type of this function is void *, so it's better to give a
explicit cast here.

Signed-off-by: Jiele Zhao 
---
   security/integrity/iint.c | 2 +-
   1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/security/integrity/iint.c b/security/integrity/iint.c
index 1d20003243c3..5f3f2de997e1 100644
--- a/security/integrity/iint.c
+++ b/security/integrity/iint.c
@@ -152,7 +152,7 @@ void integrity_inode_free(struct inode *inode)
   
   static void init_once(void *foo)

   {
-   struct integrity_iint_cache *iint = foo;
+   struct integrity_iint_cache *iint = (struct integrity_iint_cache
*)foo;
   
memset(iint, 0, sizeof(*iint));

iint->ima_file_status = INTEGRITY_UNKNOWN;

Re: [PATCH] integrity/ima: Add declarations to init_once void arguments.

[Jiele Zhao]

Ping.

On 2021/3/23 9:33, Jiele Zhao wrote:

init_once is a callback to kmem_cache_create. The parameter
type of this function is void *, so it's better to give a
explicit cast here.

Signed-off-by: Jiele Zhao 
---
  security/integrity/iint.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/security/integrity/iint.c b/security/integrity/iint.c
index 1d20003243c3..5f3f2de997e1 100644
--- a/security/integrity/iint.c
+++ b/security/integrity/iint.c
@@ -152,7 +152,7 @@ void integrity_inode_free(struct inode *inode)
  
  static void init_once(void *foo)

  {
-   struct integrity_iint_cache *iint = foo;
+   struct integrity_iint_cache *iint = (struct integrity_iint_cache *)foo;
  
  	memset(iint, 0, sizeof(*iint));

iint->ima_file_status = INTEGRITY_UNKNOWN;

[PATCH v2] ima: Fix function name error in comment.

[Jiele Zhao]

The original function name was ima_path_check().  The policy parsing
still supports PATH_CHECK.   Commit 9bbb6cad0173 ("ima: rename
ima_path_check to ima_file_check") renamed the function to
ima_file_check(), but missed modifying the function name in the
comment.

Fixes: 9bbb6cad0173 ("ima: rename ima_path_check to ima_file_check").

Signed-off-by: Jiele Zhao 
---
 security/integrity/ima/ima_main.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/security/integrity/ima/ima_main.c 
b/security/integrity/ima/ima_main.c
index 9ef748ea829f..03bef720ab44 100644
--- a/security/integrity/ima/ima_main.c
+++ b/security/integrity/ima/ima_main.c
@@ -482,7 +482,7 @@ int ima_bprm_check(struct linux_binprm *bprm)
 }
 
 /**
- * ima_path_check - based on policy, collect/store measurement.
+ * ima_file_check - based on policy, collect/store measurement.
  * @file: pointer to the file to be measured
  * @mask: contains MAY_READ, MAY_WRITE, MAY_EXEC or MAY_APPEND
  *
-- 
2.25.1

Re: [PATCH] integrity/ima: Add declarations to init_once void arguments.

[Jiele Zhao]

Ping.

On 2021/3/23 9:33, Jiele Zhao wrote:

init_once is a callback to kmem_cache_create. The parameter
type of this function is void *, so it's better to give a
explicit cast here.

Signed-off-by: Jiele Zhao 
---
  security/integrity/iint.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/security/integrity/iint.c b/security/integrity/iint.c
index 1d20003243c3..5f3f2de997e1 100644
--- a/security/integrity/iint.c
+++ b/security/integrity/iint.c
@@ -152,7 +152,7 @@ void integrity_inode_free(struct inode *inode)
  
  static void init_once(void *foo)

  {
-   struct integrity_iint_cache *iint = foo;
+   struct integrity_iint_cache *iint = (struct integrity_iint_cache *)foo;
  
  	memset(iint, 0, sizeof(*iint));

iint->ima_file_status = INTEGRITY_UNKNOWN;

[PATCH] integrity/ima: Add declarations to init_once void arguments.

[Jiele Zhao]

init_once is a callback to kmem_cache_create. The parameter
type of this function is void *, so it's better to give a
explicit cast here.

Signed-off-by: Jiele Zhao 
---
 security/integrity/iint.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/security/integrity/iint.c b/security/integrity/iint.c
index 1d20003243c3..5f3f2de997e1 100644
--- a/security/integrity/iint.c
+++ b/security/integrity/iint.c
@@ -152,7 +152,7 @@ void integrity_inode_free(struct inode *inode)
 
 static void init_once(void *foo)
 {
-   struct integrity_iint_cache *iint = foo;
+   struct integrity_iint_cache *iint = (struct integrity_iint_cache *)foo;
 
memset(iint, 0, sizeof(*iint));
iint->ima_file_status = INTEGRITY_UNKNOWN;
-- 
2.25.1

Re: [PATCH] security/loadpin: Update the changing interface in the source code.

[Jiele Zhao]

On 2021/3/8 10:03, Jiele zhao wrote:

Loadpin cmdline interface "enabled" has been renamed to "enforce"
for a long time, but the User Description Document was not updated.
(Meaning unchanged)

And kernel_read_file* were moved from linux/fs.h to its own
linux/kernel_read_file.h include file. So update that change here.

Signed-off-by: Jiele zhao 
---
  Documentation/admin-guide/LSM/LoadPin.rst | 6 +++---
  1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/Documentation/admin-guide/LSM/LoadPin.rst 
b/Documentation/admin-guide/LSM/LoadPin.rst
index 716ad9b23c9a..dd3ca68b5df1 100644
--- a/Documentation/admin-guide/LSM/LoadPin.rst
+++ b/Documentation/admin-guide/LSM/LoadPin.rst
@@ -11,8 +11,8 @@ restrictions without needing to sign the files individually.
  
  The LSM is selectable at build-time with ``CONFIG_SECURITY_LOADPIN``, and

  can be controlled at boot-time with the kernel command line option
-"``loadpin.enabled``". By default, it is enabled, but can be disabled at
-boot ("``loadpin.enabled=0``").
+"``loadpin.enforce``". By default, it is enabled, but can be disabled at
+boot ("``loadpin.enforce=0``").
  
  LoadPin starts pinning when it sees the first file loaded. If the

  block device backing the filesystem is not read-only, a sysctl is
@@ -28,4 +28,4 @@ different mechanisms such as ``CONFIG_MODULE_SIG`` and
  ``CONFIG_KEXEC_VERIFY_SIG`` to verify kernel module and kernel image while
  still use LoadPin to protect the integrity of other files kernel loads. The
  full list of valid file types can be found in ``kernel_read_file_str``
-defined in ``include/linux/fs.h``.
+defined in ``include/linux/kernel_read_file.h``.


Ping. It's been almost one week now, can someone respond this patch?
Kees? Jonathan?

[PATCH v3] security/loadpin: Replace "kernel_read_file_str[j]" with function "kernel_read_file_id_str(j)".

[Jiele zhao]

Actually Linux kernel already provide function "kernel_read_file_id_str()"
for secure access in "kernel_read_file.h".
And, in "parse_exclude()" function, it's better to use

BUILD_BUG_ON(ARRAY_SIZE(kernel_read_file_str) - 1 !=
ARRAY_SIZE(ignore_read_file_id));

to make sure the arrays stay within expected sizes.

By the way, sorry for that mistake PATCH v2 file, I sent wrong path... ...

Signed-off-by: Jiele zhao 
---
 security/loadpin/loadpin.c | 7 ---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/security/loadpin/loadpin.c b/security/loadpin/loadpin.c
index b12f7d986b1e..1c35164673b4 100644
--- a/security/loadpin/loadpin.c
+++ b/security/loadpin/loadpin.c
@@ -210,9 +210,10 @@ static void __init parse_exclude(void)
 */
BUILD_BUG_ON(ARRAY_SIZE(exclude_read_files) !=
 ARRAY_SIZE(ignore_read_file_id));
-   BUILD_BUG_ON(ARRAY_SIZE(kernel_read_file_str) <
+   BUILD_BUG_ON(ARRAY_SIZE(kernel_read_file_str) - 1 !=
 ARRAY_SIZE(ignore_read_file_id));
 
+
for (i = 0; i < ARRAY_SIZE(exclude_read_files); i++) {
cur = exclude_read_files[i];
if (!cur)
@@ -221,9 +222,9 @@ static void __init parse_exclude(void)
continue;
 
for (j = 0; j < ARRAY_SIZE(ignore_read_file_id); j++) {
-   if (strcmp(cur, kernel_read_file_str[j]) == 0) {
+   if (strcmp(cur, kernel_read_file_id_str(j)) == 0) {
pr_info("excluding: %s\n",
-   kernel_read_file_str[j]);
+   kernel_read_file_id_str(j));
ignore_read_file_id[j] = 1;
/*
 * Can not break, because one read_file_str
-- 
2.25.1

[PATCH v2] security/loadpin: Replace "kernel_read_file_str[j]" with function "kernel_read_file_id_str(j)".

[Jiele zhao]

Actually Linux kernel already provide function "kernel_read_file_id_str()"
for secure access in "kernel_read_file.h".
And, in "parse_exclude()" function, it's better to use

BUILD_BUG_ON(ARRAY_SIZE(kernel_read_file_str) - 1 ==
ARRAY_SIZE(ignore_read_file_id));

to make sure the arrays stay within expected sizes.

Signed-off-by: Jiele zhao 
---
 security/loadpin/loadpin.c | 7 ---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/security/loadpin/loadpin.c b/security/loadpin/loadpin.c
index b12f7d986b1e..3d59ff363087 100644
--- a/security/loadpin/loadpin.c
+++ b/security/loadpin/loadpin.c
@@ -210,9 +210,10 @@ static void __init parse_exclude(void)
 */
BUILD_BUG_ON(ARRAY_SIZE(exclude_read_files) !=
 ARRAY_SIZE(ignore_read_file_id));
-   BUILD_BUG_ON(ARRAY_SIZE(kernel_read_file_str) <
+   BUILD_BUG_ON(ARRAY_SIZE(kernel_read_file_str) - 1 ==
 ARRAY_SIZE(ignore_read_file_id));
 
+
for (i = 0; i < ARRAY_SIZE(exclude_read_files); i++) {
cur = exclude_read_files[i];
if (!cur)
@@ -221,9 +222,9 @@ static void __init parse_exclude(void)
continue;
 
for (j = 0; j < ARRAY_SIZE(ignore_read_file_id); j++) {
-   if (strcmp(cur, kernel_read_file_str[j]) == 0) {
+   if (strcmp(cur, kernel_read_file_id_str(j)) == 0) {
pr_info("excluding: %s\n",
-   kernel_read_file_str[j]);
+   kernel_read_file_id_str(j));
ignore_read_file_id[j] = 1;
/*
 * Can not break, because one read_file_str
-- 
2.25.1