* [PATCH -next] selinux: Fix memleak in security_read_state_kernel
@ 2022-06-13 13:59 Xiu Jianfeng
2022-06-13 20:34 ` Paul Moore
2022-06-14 12:57 ` Ondrej Mosnacek
0 siblings, 2 replies; 5+ messages in thread
From: Xiu Jianfeng @ 2022-06-13 13:59 UTC (permalink / raw)
To: paul, stephen.smalley.work, eparis, cgzones, omosnace,
michalorzel.eng, austin.kim
Cc: selinux, linux-kernel
In this function, it directly returns the result of __security_read_policy
without freeing the allocated memory in *data, cause memory leak issue,
so free the memory if __security_read_policy failed.
Signed-off-by: Xiu Jianfeng <xiujianfeng@huawei.com>
---
security/selinux/ss/services.c | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c
index 69b2734311a6..fe5fcf571c56 100644
--- a/security/selinux/ss/services.c
+++ b/security/selinux/ss/services.c
@@ -4048,6 +4048,7 @@ int security_read_policy(struct selinux_state *state,
int security_read_state_kernel(struct selinux_state *state,
void **data, size_t *len)
{
+ int err;
struct selinux_policy *policy;
policy = rcu_dereference_protected(
@@ -4060,5 +4061,11 @@ int security_read_state_kernel(struct selinux_state *state,
if (!*data)
return -ENOMEM;
- return __security_read_policy(policy, *data, len);
+ err = __security_read_policy(policy, *data, len);
+ if (err) {
+ vfree(*data);
+ *data = NULL;
+ *len = 0;
+ }
+ return err;
}
--
2.17.1
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH -next] selinux: Fix memleak in security_read_state_kernel
2022-06-13 13:59 [PATCH -next] selinux: Fix memleak in security_read_state_kernel Xiu Jianfeng
@ 2022-06-13 20:34 ` Paul Moore
2022-06-14 12:57 ` Ondrej Mosnacek
1 sibling, 0 replies; 5+ messages in thread
From: Paul Moore @ 2022-06-13 20:34 UTC (permalink / raw)
To: Xiu Jianfeng
Cc: stephen.smalley.work, eparis, cgzones, omosnace, michalorzel.eng,
austin.kim, selinux, linux-kernel
On Mon, Jun 13, 2022 at 10:01 AM Xiu Jianfeng <xiujianfeng@huawei.com> wrote:
>
> In this function, it directly returns the result of __security_read_policy
> without freeing the allocated memory in *data, cause memory leak issue,
> so free the memory if __security_read_policy failed.
>
> Signed-off-by: Xiu Jianfeng <xiujianfeng@huawei.com>
> ---
> security/selinux/ss/services.c | 9 ++++++++-
> 1 file changed, 8 insertions(+), 1 deletion(-)
Merged into selinux/next, thanks.
--
paul-moore.com
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH -next] selinux: Fix memleak in security_read_state_kernel
2022-06-13 13:59 [PATCH -next] selinux: Fix memleak in security_read_state_kernel Xiu Jianfeng
2022-06-13 20:34 ` Paul Moore
@ 2022-06-14 12:57 ` Ondrej Mosnacek
2022-06-14 13:34 ` xiujianfeng
1 sibling, 1 reply; 5+ messages in thread
From: Ondrej Mosnacek @ 2022-06-14 12:57 UTC (permalink / raw)
To: Xiu Jianfeng
Cc: Paul Moore, Stephen Smalley, Eric Paris, Christian Göttsche,
michalorzel.eng, Austin Kim, SElinux list,
Linux kernel mailing list
On Mon, Jun 13, 2022 at 4:02 PM Xiu Jianfeng <xiujianfeng@huawei.com> wrote:
> In this function, it directly returns the result of __security_read_policy
> without freeing the allocated memory in *data, cause memory leak issue,
> so free the memory if __security_read_policy failed.
>
> Signed-off-by: Xiu Jianfeng <xiujianfeng@huawei.com>
> ---
> security/selinux/ss/services.c | 9 ++++++++-
> 1 file changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c
> index 69b2734311a6..fe5fcf571c56 100644
> --- a/security/selinux/ss/services.c
> +++ b/security/selinux/ss/services.c
> @@ -4048,6 +4048,7 @@ int security_read_policy(struct selinux_state *state,
> int security_read_state_kernel(struct selinux_state *state,
> void **data, size_t *len)
> {
> + int err;
> struct selinux_policy *policy;
>
> policy = rcu_dereference_protected(
> @@ -4060,5 +4061,11 @@ int security_read_state_kernel(struct selinux_state *state,
> if (!*data)
> return -ENOMEM;
>
> - return __security_read_policy(policy, *data, len);
> + err = __security_read_policy(policy, *data, len);
> + if (err) {
> + vfree(*data);
> + *data = NULL;
> + *len = 0;
> + }
> + return err;
> }
> --
> 2.17.1
>
security_read_policy() defined a few lines above has the same pattern
(just with vmalloc_user() in place of vmalloc()). Would you like to
send another patch to fix that function as well?
--
Ondrej Mosnacek
Software Engineer, Linux Security - SELinux kernel
Red Hat, Inc.
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH -next] selinux: Fix memleak in security_read_state_kernel
2022-06-14 12:57 ` Ondrej Mosnacek
@ 2022-06-14 13:34 ` xiujianfeng
2022-06-14 14:57 ` Ondrej Mosnacek
0 siblings, 1 reply; 5+ messages in thread
From: xiujianfeng @ 2022-06-14 13:34 UTC (permalink / raw)
To: Ondrej Mosnacek
Cc: Paul Moore, Stephen Smalley, Eric Paris, Christian Göttsche,
michalorzel.eng, Austin Kim, SElinux list,
Linux kernel mailing list
在 2022/6/14 20:57, Ondrej Mosnacek 写道:
> On Mon, Jun 13, 2022 at 4:02 PM Xiu Jianfeng <xiujianfeng@huawei.com> wrote:
>> In this function, it directly returns the result of __security_read_policy
>> without freeing the allocated memory in *data, cause memory leak issue,
>> so free the memory if __security_read_policy failed.
>>
>> Signed-off-by: Xiu Jianfeng <xiujianfeng@huawei.com>
>> ---
>> security/selinux/ss/services.c | 9 ++++++++-
>> 1 file changed, 8 insertions(+), 1 deletion(-)
>>
>> diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c
>> index 69b2734311a6..fe5fcf571c56 100644
>> --- a/security/selinux/ss/services.c
>> +++ b/security/selinux/ss/services.c
>> @@ -4048,6 +4048,7 @@ int security_read_policy(struct selinux_state *state,
>> int security_read_state_kernel(struct selinux_state *state,
>> void **data, size_t *len)
>> {
>> + int err;
>> struct selinux_policy *policy;
>>
>> policy = rcu_dereference_protected(
>> @@ -4060,5 +4061,11 @@ int security_read_state_kernel(struct selinux_state *state,
>> if (!*data)
>> return -ENOMEM;
>>
>> - return __security_read_policy(policy, *data, len);
>> + err = __security_read_policy(policy, *data, len);
>> + if (err) {
>> + vfree(*data);
>> + *data = NULL;
>> + *len = 0;
>> + }
>> + return err;
>> }
>> --
>> 2.17.1
>>
> security_read_policy() defined a few lines above has the same pattern
> (just with vmalloc_user() in place of vmalloc()). Would you like to
> send another patch to fix that function as well?
No problem, patch already sent.
>
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH -next] selinux: Fix memleak in security_read_state_kernel
2022-06-14 13:34 ` xiujianfeng
@ 2022-06-14 14:57 ` Ondrej Mosnacek
0 siblings, 0 replies; 5+ messages in thread
From: Ondrej Mosnacek @ 2022-06-14 14:57 UTC (permalink / raw)
To: xiujianfeng
Cc: Paul Moore, Stephen Smalley, Eric Paris, Christian Göttsche,
michalorzel.eng, Austin Kim, SElinux list,
Linux kernel mailing list
On Tue, Jun 14, 2022 at 3:35 PM xiujianfeng <xiujianfeng@huawei.com> wrote:
>
>
> 在 2022/6/14 20:57, Ondrej Mosnacek 写道:
> > On Mon, Jun 13, 2022 at 4:02 PM Xiu Jianfeng <xiujianfeng@huawei.com> wrote:
> >> In this function, it directly returns the result of __security_read_policy
> >> without freeing the allocated memory in *data, cause memory leak issue,
> >> so free the memory if __security_read_policy failed.
> >>
> >> Signed-off-by: Xiu Jianfeng <xiujianfeng@huawei.com>
> >> ---
> >> security/selinux/ss/services.c | 9 ++++++++-
> >> 1 file changed, 8 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c
> >> index 69b2734311a6..fe5fcf571c56 100644
> >> --- a/security/selinux/ss/services.c
> >> +++ b/security/selinux/ss/services.c
> >> @@ -4048,6 +4048,7 @@ int security_read_policy(struct selinux_state *state,
> >> int security_read_state_kernel(struct selinux_state *state,
> >> void **data, size_t *len)
> >> {
> >> + int err;
> >> struct selinux_policy *policy;
> >>
> >> policy = rcu_dereference_protected(
> >> @@ -4060,5 +4061,11 @@ int security_read_state_kernel(struct selinux_state *state,
> >> if (!*data)
> >> return -ENOMEM;
> >>
> >> - return __security_read_policy(policy, *data, len);
> >> + err = __security_read_policy(policy, *data, len);
> >> + if (err) {
> >> + vfree(*data);
> >> + *data = NULL;
> >> + *len = 0;
> >> + }
> >> + return err;
> >> }
> >> --
> >> 2.17.1
> >>
> > security_read_policy() defined a few lines above has the same pattern
> > (just with vmalloc_user() in place of vmalloc()). Would you like to
> > send another patch to fix that function as well?
> No problem, patch already sent.
Wow, you're fast :) Thanks!
--
Ondrej Mosnacek
Software Engineer, Linux Security - SELinux kernel
Red Hat, Inc.
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2022-06-14 14:57 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-06-13 13:59 [PATCH -next] selinux: Fix memleak in security_read_state_kernel Xiu Jianfeng
2022-06-13 20:34 ` Paul Moore
2022-06-14 12:57 ` Ondrej Mosnacek
2022-06-14 13:34 ` xiujianfeng
2022-06-14 14:57 ` Ondrej Mosnacek
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.