Skip to contentSkip to conversation

landlock: Initialize kernel stack variables properly

PATCH In review Austin Kim 3 participants 4 messages Nov 3, 2021 – Nov 5, 2021

Subsystems (2)
Security (LSM) Landlock security module
lore
Share & tools
lore

Austin Kim

author
#0 lore raw

PATCH 1/1 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 <[email protected]>
---
 security/landlock/syscalls.c | 2 ++
 1 file changed, 2 insertions(+)
security/landlock/syscalls.c +2 -0
index 32396962f04d..50a6f7091428 100644
@@ -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));
Signature · 1 line
2.20.1
maintainer
#1 lore raw

Hi Austin,

Quoted 14 lines · Austin Kim

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 <[email protected]>
---
 security/landlock/syscalls.c | 2 ++
 1 file changed, 2 insertions(+)
security/landlock/syscalls.c +0 -0
index 32396962f04d..50a6f7091428 100644

security/landlock/syscalls.c SYSCALL_DEFINE4(landlock_add_rule, · line 324

Quoted 5 lines
security/landlock/syscalls.c +2 -0
@@ -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.

Quoted 4 lines
security/landlock/syscalls.c +0 -0
 	/* Copies raw user space buffer, only one type for now. */
 	res = copy_from_user(&path_beneath_attr, rule_attr,
 			sizeof(path_beneath_attr));
author
#2 lore raw

2021년 11월 3일 (수) 오후 9:14, Mickaël Salaün <[email protected]>님이 작성:

Quoted 21 lines

Hi Austin,

Quoted 17 lines · Austin Kim

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 <[email protected]>
---
 security/landlock/syscalls.c | 2 ++
 1 file changed, 2 insertions(+)
security/landlock/syscalls.c +0 -0
index 32396962f04d..50a6f7091428 100644
@@ -320,6 +320,8 @@ SYSCALL_DEFINE4(landlock_add_rule,
      if (rule_type != LANDLOCK_RULE_PATH_BENEATH)
              return -EINVAL;
Quoted 5 lines

security/landlock/syscalls.c SYSCALL_DEFINE4(landlock_add_rule, · line 324

Quoted 2 lines
security/landlock/syscalls.c +2 -0
+     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 4 lines
Quoted 4 lines
security/landlock/syscalls.c +0 -0
      /* Copies raw user space buffer, only one type for now. */
      res = copy_from_user(&path_beneath_attr, rule_attr,
                      sizeof(path_beneath_attr));
#3 lore raw
Quoted 21 lines · Austin Kim

2021년 11월 3일 (수) 오후 9:14, Mickaël Salaün <[email protected]>님이 작성:

Quoted 20 lines

Hi Austin,

Quoted 17 lines · Austin Kim

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 <[email protected]>
---
  security/landlock/syscalls.c | 2 ++
  1 file changed, 2 insertions(+)
security/landlock/syscalls.c +0 -0
index 32396962f04d..50a6f7091428 100644
@@ -320,6 +320,8 @@ SYSCALL_DEFINE4(landlock_add_rule,
       if (rule_type != LANDLOCK_RULE_PATH_BENEATH)
               return -EINVAL;
Quoted 6 lines
Quoted 4 lines

security/landlock/syscalls.c SYSCALL_DEFINE4(landlock_add_rule, · line 324

Quoted 2 lines
security/landlock/syscalls.c +2 -0
+     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

Quoted 8 lines

BR,
Austin Kim

Quoted 4 lines
Quoted 4 lines
security/landlock/syscalls.c +0 -0
       /* Copies raw user space buffer, only one type for now. */
       res = copy_from_user(&path_beneath_attr, rule_attr,
                       sizeof(path_beneath_attr));