On 6/11/20 3:30 PM, Paul Moore wrote: > On Thu, Jun 11, 2020 at 4:48 PM <trix@xxxxxxxxxx> wrote: >> From: Tom Rix <trix@xxxxxxxxxx> >> >> 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@xxxxxxxxxx> >> --- >> 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@xxxxxxxxxxxxxx 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.
