[PATCH] LSM: add NULL check for kcalloc()

Subsystems: security subsystem, the rest

STALE1899d

4 messages, 3 authors, 2021-08-06 · open the first message on its own page

[PATCH] LSM: add NULL check for kcalloc()

From: Austin Kim <hidden>
Date: 2021-07-12 23:44:41

From: Austin Kim <redacted>

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 <redacted>
---
 security/security.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/security/security.c b/security/security.c
index 09533cbb7221..f885c9e9bc35 100644
--- a/security/security.c
+++ b/security/security.c

@@ -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) {

-- 
2.20.1

Re: [PATCH] LSM: add NULL check for kcalloc()

From: James Morris <jmorris@namei.org>
Date: 2021-07-14 19:12:34

On Tue, 13 Jul 2021, Austin Kim wrote:

quoted hunk

From: Austin Kim <redacted>

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 <redacted>
---
 security/security.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/security/security.c b/security/security.c
index 09533cbb7221..f885c9e9bc35 100644
--- a/security/security.c
+++ b/security/security.c

@@ -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?

 
 	if (chosen_lsm_order) {
 		if (chosen_major_lsm) {
-- 
2.20.1

-- 
James Morris
[off-list ref]

Re: [PATCH] LSM: add NULL check for kcalloc()

From: Austin Kim <hidden>
Date: 2021-07-14 21:45:07

2021년 7월 15일 (목) 오전 4:12, James Morris [off-list ref]님이 작성:

On Tue, 13 Jul 2021, Austin Kim wrote:

quoted

From: Austin Kim <redacted>

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 <redacted>
---
 security/security.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/security/security.c b/security/security.c
index 09533cbb7221..f885c9e9bc35 100644
--- a/security/security.c
+++ b/security/security.c

@@ -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.

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

Should this also be a kernel panic?

quoted

      if (chosen_lsm_order) {
              if (chosen_major_lsm) {
--
2.20.1

--
James Morris
[off-list ref]

Re: [PATCH] LSM: add NULL check for kcalloc()

From: Ken Goldman <hidden>
Date: 2021-08-06 18:33:32

On 7/14/2021 5:44 PM, Austin Kim wrote:

2021년 7월 15일 (목) 오전 4:12, James Morris [off-list ref]님이 작성:

quoted

On Tue, 13 Jul 2021, Austin Kim wrote:

quoted

From: Austin Kim <redacted>

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 <redacted>
---
  security/security.c | 2 ++
  1 file changed, 2 insertions(+)

diff --git a/security/security.c b/security/security.c
index 09533cbb7221..f885c9e9bc35 100644
--- a/security/security.c
+++ b/security/security.c

@@ -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.

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.

`h`	back out one level
`j`	next message in thread
`k`	previous message in thread
`l`	drill in
`Esc`	close help / fold thread tree
`?`	toggle this help