| Message ID | 20211103071427.GA13854@raspberrypi (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | landlock: Initialize kernel stack variables properly | expand |
Hi Austin, On 03/11/2021 08:14, Austin Kim wrote: > 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 <austindh.kim@gmail.com> > --- > 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. > /* 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 <mic@digikod.net>님이 작성: > > Hi Austin, > > On 03/11/2021 08:14, Austin Kim wrote: > > 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 <austindh.kim@gmail.com> > > --- > > 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 > > /* 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 <mic@digikod.net>님이 작성: >> Hi Austin, >> >> On 03/11/2021 08:14, Austin Kim wrote: >>> 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 <austindh.kim@gmail.com> >>> --- >>> 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 > >>> /* Copies raw user space buffer, only one type for now. */ >>> res = copy_from_user(&path_beneath_attr, rule_attr, >>> sizeof(path_beneath_attr)); >>>
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));
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 <austindh.kim@gmail.com> --- security/landlock/syscalls.c | 2 ++ 1 file changed, 2 insertions(+)