Messages in this thread | | | Subject | Re: [PATCH] samples/landlock: fix path_list memory leak | From | Tom Rix <> | Date | Wed, 28 Apr 2021 08:36:27 -0700 |
| |
On 4/28/21 2:58 AM, Mickaël Salaün wrote: > On 27/04/2021 21:13, Nick Desaulniers wrote: >> On Tue, Apr 27, 2021 at 11:38 AM <trix@redhat.com> wrote: >>> From: Tom Rix <trix@redhat.com> >>> >>> Clang static analysis reports this error >>> >>> sandboxer.c:134:8: warning: Potential leak of memory >>> pointed to by 'path_list' >>> ret = 0; >>> ^ >>> path_list is allocated in parse_path() but never freed. >>> >>> Signed-off-by: Tom Rix <trix@redhat.com> >>> --- >>> samples/landlock/sandboxer.c | 2 ++ >>> 1 file changed, 2 insertions(+) >>> >>> diff --git a/samples/landlock/sandboxer.c b/samples/landlock/sandboxer.c >>> index 7a15910d2171..4629d011ed61 100644 >>> --- a/samples/landlock/sandboxer.c >>> +++ b/samples/landlock/sandboxer.c >>> @@ -134,6 +134,8 @@ static int populate_ruleset( >>> ret = 0; >>> >>> out_free_name: >>> + if (path_list) >>> + free(path_list); >> I don't think the conditional is even necessary? By our first `goto >> out_free_name;`, `parse_path` has already been called/memory for >> `path_list` has already been allocated. `parse_path` doesn't check >> whether `malloc` has failed. > Indeed, no need for the path_list check. In practice, this memory leak > doesn't stay long because of the execve, but I missed this free anyway. > Thanks!
Ok, the general problem of not checking if malloc and friends succeeds is a different problem.
So remove the check and keep the free ?
Tom
> Reviewed-by: Mickaël Salaün <mic@linux.microsoft.com> > >>> free(env_path_name); >>> return ret; >>> } >>> -- >>> 2.26.3 >>> >>
| |