lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Thu, 11 Jun 2020 15:41:11 -0700 From: Tom Rix <trix@...hat.com> To: Paul Moore <paul@...l-moore.com> Cc: Stephen Smalley <stephen.smalley.work@...il.com>, Eric Paris <eparis@...isplace.org>, Ondrej Mosnacek <omosnace@...hat.com>, weiyongjun1@...wei.com, selinux@...r.kernel.org, linux-kernel@...r.kernel.org Subject: Re: [PATCH v2 1/1] selinux: fix another double free On 6/11/20 3:30 PM, Paul Moore wrote: > On Thu, Jun 11, 2020 at 4:48 PM <trix@...hat.com> wrote: >> From: Tom Rix <trix@...hat.com> >> >> Clang static analysis reports this double free error >> >> security/selinux/ss/conditional.c:139:2: warning: Attempt to free released memory [unix.Malloc] >> kfree(node->expr.nodes); >> ^~~~~~~~~~~~~~~~~~~~~~~ >> >> When cond_read_node fails, it calls cond_node_destroy which frees the >> node but does not poison the entry in the node list. So when it >> returns to its caller cond_read_list, cond_read_list deletes the >> partial list. The latest entry in the list will be deleted twice. >> >> So instead of freeing the node in cond_read_node, let list freeing in >> code_read_list handle the freeing the problem node along with all of the >> earlier nodes. >> >> Because cond_read_node no longer does any error handling, the goto's >> the error case are redundant. Instead just return the error code. >> >> Fixes a problem was introduced by commit >> >> selinux: convert cond_list to array >> >> Signed-off-by: Tom Rix <trix@...hat.com> >> --- >> security/selinux/ss/conditional.c | 11 +++-------- >> 1 file changed, 3 insertions(+), 8 deletions(-) > Hi Tom, > > Thanks for the patch! A few more notes, in no particular order: > > * There is no need to send a cover letter for just a single patch. > Typically cover letters are reserved for large patchsets that require > some additional explanation and/or instructions beyond the individual > commit descriptions. I was doing this to carry the repo name and tag info. So how do folks know which repo and commit the change applies to ? > * Thank you for including a changelog with your patch updates, but it > would be helpful if you included them in the patch by using a "---" > delimiter in the commit description after your signoff but before the > diffstat. Here is a recent example: > -> https://lore.kernel.org/selinux/20200611135303.19538-3-cgzones@googlemail.com Ok got it. > > * When referencing a patch which you are "fixing", the proper syntax > is 'Fixes: <12char_commitID> ("<subject_line")'. Look at commit > 46619b44e431 in Linus' tree to see an example. Ok > If you have any questions, let us know.
Powered by blists - more mailing lists