Skip to contentSkip to conversation

LSM: add NULL check for kcalloc()

PATCH Superseded Austin Kim 3 participants 4 messages Jul 12, 2021 – Aug 6, 2021 Security (LSM)

v1 of 2 Superseded Latest is v2 →

lore
Share & tools
lore

Austin Kim

author
#0 lore raw

PATCH 1/1 LSM: add NULL check for kcalloc()

From: Austin Kim <[email protected]>

kcalloc() may return NULL when memory allocation fails.
So it is necessary to add NULL check after the call to kcalloc() is made.

Signed-off-by Austin Kim <[email protected]>
---
 security/security.c | 2 ++
 1 file changed, 2 insertions(+)
security/security.c +2 -0
index 09533cbb7221..f885c9e9bc35 100644
@@ -321,6 +321,8 @@ static void __init ordered_lsm_init(void)
 
 	ordered_lsms = kcalloc(LSM_COUNT + 1, sizeof(*ordered_lsms),
 				GFP_KERNEL);
+	if (ordered_lsms)
+		return;
 
 	if (chosen_lsm_order) {
 		if (chosen_major_lsm) {
Signature · 1 line
2.20.1
maintainer
#1 lore raw
Quoted 15 lines · Austin Kim

From: Austin Kim <[email protected]>

kcalloc() may return NULL when memory allocation fails.
So it is necessary to add NULL check after the call to kcalloc() is made.

Signed-off-by Austin Kim <[email protected]>
---
 security/security.c | 2 ++
 1 file changed, 2 insertions(+)
security/security.c +0 -0
index 09533cbb7221..f885c9e9bc35 100644

security/security.c static void __init ordered_lsm_init(void) · line 325

Quoted 5 lines
security/security.c +2 -0
@@ -321,6 +321,8 @@ static void __init ordered_lsm_init(void)
 
 	ordered_lsms = kcalloc(LSM_COUNT + 1, sizeof(*ordered_lsms),
 				GFP_KERNEL);
+	if (ordered_lsms)
+		return;

Your logic is reversed here.

Should this also be a kernel panic?

Quoted 6 lines
security/security.c +0 -0
 	if (chosen_lsm_order) {
 		if (chosen_major_lsm) {
Signature · 1 line
2.20.1
Signature · 2 lines
James Morris
<[email protected]>
author
#2 lore raw

2021년 7월 15일 (목) 오전 4:12, James Morris <[email protected]>님이 작성:

Quoted 19 lines · Austin Kim
Quoted 17 lines

From: Austin Kim <[email protected]>

kcalloc() may return NULL when memory allocation fails.
So it is necessary to add NULL check after the call to kcalloc() is made.

Signed-off-by Austin Kim <[email protected]>
---
 security/security.c | 2 ++
 1 file changed, 2 insertions(+)
security/security.c +0 -0
index 09533cbb7221..f885c9e9bc35 100644
@@ -321,6 +321,8 @@ static void __init ordered_lsm_init(void)
Quoted 6 lines

security/security.c static void __init ordered_lsm_init(void) · line 325

Quoted 4 lines
security/security.c +2 -0
      ordered_lsms = kcalloc(LSM_COUNT + 1, sizeof(*ordered_lsms),
                              GFP_KERNEL);
+     if (ordered_lsms)
+             return;

Your logic is reversed here.

I feel very sorry for my terrible mistake.
'if (ordered_lsms)' should have been 'if (!ordered_lsms)'.

Let me resend patch(v2) soon.

Thanks,
Austin Kim

Quoted 14 lines

Should this also be a kernel panic?

Quoted 6 lines
security/security.c +0 -1
      if (chosen_lsm_order) {
              if (chosen_major_lsm) {
--

2.20.1

--
James Morris
<[email protected]>

#3 lore raw
Quoted 27 lines · Austin Kim

2021년 7월 15일 (목) 오전 4:12, James Morris <[email protected]>님이 작성:

Quoted 19 lines · Austin Kim
Quoted 17 lines

From: Austin Kim <[email protected]>

kcalloc() may return NULL when memory allocation fails.
So it is necessary to add NULL check after the call to kcalloc() is made.

Signed-off-by Austin Kim <[email protected]>
---
  security/security.c | 2 ++
  1 file changed, 2 insertions(+)
security/security.c +0 -0
index 09533cbb7221..f885c9e9bc35 100644
@@ -321,6 +321,8 @@ static void __init ordered_lsm_init(void)
Quoted 6 lines

security/security.c static void __init ordered_lsm_init(void) · line 325

Quoted 4 lines
security/security.c +2 -0
       ordered_lsms = kcalloc(LSM_COUNT + 1, sizeof(*ordered_lsms),
                               GFP_KERNEL);
+     if (ordered_lsms)
+             return;

Your logic is reversed here.

Quoted 3 lines

I feel very sorry for my terrible mistake.
'if (ordered_lsms)' should have been 'if (!ordered_lsms)'.

I know it's a bit more typing, but

	if (ordered_lsms == NULL)

compiles down to the same binary and avoids there errors that try to treat a pointer as a boolean.