| Message ID | 20200610181021.19209-2-trix@redhat.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | selinux: fix double free | expand |
On Wed, Jun 10, 2020 at 2:10 PM <trix@redhat.com> wrote: > > From: Tom Rix <trix@redhat.com> > > Clang's static analysis tool reports these double free memory errors. > > security/selinux/ss/services.c:2987:4: warning: Attempt to free released memory [unix.Malloc] > kfree(bnames[i]); > ^~~~~~~~~~~~~~~~ > security/selinux/ss/services.c:2990:2: warning: Attempt to free released memory [unix.Malloc] > kfree(bvalues); > ^~~~~~~~~~~~~~ > > So improve the security_get_bools error handling by freeing these variables > and setting their return pointers to NULL and the return len to 0 > > Signed-off-by: Tom Rix <trix@redhat.com> > --- > security/selinux/ss/services.c | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c > index 313919bd42f8..2dffae1feaff 100644 > --- a/security/selinux/ss/services.c > +++ b/security/selinux/ss/services.c > @@ -2888,8 +2888,12 @@ int security_get_bools(struct selinux_state *state, > if (*names) { > for (i = 0; i < *len; i++) > kfree((*names)[i]); > + kfree(names); kfree(*names)? > } > kfree(*values); > + *len = 0; > + *names = NULL; > + *values = NULL; > goto out; > } Wondering if the caller handling ought to be changed too even though this should avoid the problem.
>> +++ b/security/selinux/ss/services.c >> @@ -2888,8 +2888,12 @@ int security_get_bools(struct selinux_state *state, >> if (*names) { >> for (i = 0; i < *len; i++) >> kfree((*names)[i]); >> + kfree(names); > kfree(*names)? Yes. > kfree(*values); >> + *len = 0; >> + *names = NULL; >> + *values = NULL; >> goto out; >> } > Wondering if the caller handling ought to be changed too even though > this should avoid the problem. > The poisoning of the returns avoids this.
diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c index 313919bd42f8..2dffae1feaff 100644 --- a/security/selinux/ss/services.c +++ b/security/selinux/ss/services.c @@ -2888,8 +2888,12 @@ int security_get_bools(struct selinux_state *state, if (*names) { for (i = 0; i < *len; i++) kfree((*names)[i]); + kfree(names); } kfree(*values); + *len = 0; + *names = NULL; + *values = NULL; goto out; }