[PATCH] landlock: Initialize kernel stack variables properly

In case kernel stack variables are not initialized properly, there might 
be a little chance of kernel information disclosure. So it is better for 
kernel stack variables to be initialized with null characters.

Signed-off-by: Austin Kim <redacted>
---
 security/landlock/syscalls.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/security/landlock/syscalls.c b/security/landlock/syscalls.c
index 32396962f04d..50a6f7091428 100644
--- a/security/landlock/syscalls.c
+++ b/security/landlock/syscalls.c

@@ -320,6 +320,8 @@ SYSCALL_DEFINE4(landlock_add_rule,
 	if (rule_type != LANDLOCK_RULE_PATH_BENEATH)
 		return -EINVAL;
 
+	memset(&path_beneath_attr, 0, sizeof(path_beneath_attr));
+
 	/* Copies raw user space buffer, only one type for now. */
 	res = copy_from_user(&path_beneath_attr, rule_attr,
 			sizeof(path_beneath_attr));

-- 
2.20.1

Hi Austin,

On 03/11/2021 08:14, Austin Kim wrote:

This memset is already done with the copy_from_user() call just below.

 	/* Copies raw user space buffer, only one type for now. */
 	res = copy_from_user(&path_beneath_attr, rule_attr,
 			sizeof(path_beneath_attr));

2021년 11월 3일 (수) 오후 9:14, Mickaël Salaün [off-list ref]님이 작성:

Hi Austin,

On 03/11/2021 08:14, Austin Kim wrote:

quoted

In case kernel stack variables are not initialized properly, there might
be a little chance of kernel information disclosure. So it is better for
kernel stack variables to be initialized with null characters.

Signed-off-by: Austin Kim <redacted>
---
 security/landlock/syscalls.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/security/landlock/syscalls.c b/security/landlock/syscalls.c
index 32396962f04d..50a6f7091428 100644
--- a/security/landlock/syscalls.c
+++ b/security/landlock/syscalls.c

@@ -320,6 +320,8 @@ SYSCALL_DEFINE4(landlock_add_rule,
      if (rule_type != LANDLOCK_RULE_PATH_BENEATH)
              return -EINVAL;

+     memset(&path_beneath_attr, 0, sizeof(path_beneath_attr));
+

This memset is already done with the copy_from_user() call just below.

It seems that memset() is done inside copy_from_user().
Thanks for feedback.

BR,
Austin Kim

quoted

      /* Copies raw user space buffer, only one type for now. */
      res = copy_from_user(&path_beneath_attr, rule_attr,
                      sizeof(path_beneath_attr));

On 11/4/21 4:41 AM, Austin Kim wrote:

2021년 11월 3일 (수) 오후 9:14, Mickaël Salaün [off-list ref]님이 작성:

quoted

Hi Austin,

On 03/11/2021 08:14, Austin Kim wrote:

quoted

In case kernel stack variables are not initialized properly, there might
be a little chance of kernel information disclosure. So it is better for
kernel stack variables to be initialized with null characters.

Signed-off-by: Austin Kim <redacted>
---
  security/landlock/syscalls.c | 2 ++
  1 file changed, 2 insertions(+)

diff --git a/security/landlock/syscalls.c b/security/landlock/syscalls.c
index 32396962f04d..50a6f7091428 100644
--- a/security/landlock/syscalls.c
+++ b/security/landlock/syscalls.c

@@ -320,6 +320,8 @@ SYSCALL_DEFINE4(landlock_add_rule,
       if (rule_type != LANDLOCK_RULE_PATH_BENEATH)
               return -EINVAL;

+     memset(&path_beneath_attr, 0, sizeof(path_beneath_attr));
+

This memset is already done with the copy_from_user() call just below.

It seems that memset() is done inside copy_from_user().
Thanks for feedback.

If you are really sensitive of what information may be disclosed in this 
case you can consider memzero_explicit() to use instead:

https://www.kernel.org/doc/htmldocs/kernel-api/API-memzero-explicit.html

Jay

BR,
Austin Kim

quoted

quoted

       /* Copies raw user space buffer, only one type for now. */
       res = copy_from_user(&path_beneath_attr, rule_attr,
                       sizeof(path_beneath_attr));