Messages in this thread | | | Subject | Re: [PATCH v2] livepatch: Reorder to use before freeing a pointer | From | Tom Rix <> | Date | Mon, 21 Mar 2022 07:05:59 -0700 |
| |
On 3/21/22 6:39 AM, Joe Lawrence wrote: > On 3/19/22 9:51 PM, trix@redhat.com wrote: >> From: Tom Rix <trix@redhat.com> >> >> Clang static analysis reports this issue >> livepatch-shadow-fix1.c:113:2: warning: Use of >> memory after it is freed >> pr_info("%s: dummy @ %p, prevented leak @ %p\n", >> ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ >> >> The pointer is freed in the previous statement. >> Reorder the pr_info to report before the free. >> >> Similar issue in livepatch-shadow-fix2.c >> >> Signed-off-by: Tom Rix <trix@redhat.com> >> --- >> v2: Fix similar issue in livepatch-shadow-fix2.c >> >> samples/livepatch/livepatch-shadow-fix1.c | 2 +- >> samples/livepatch/livepatch-shadow-fix2.c | 2 +- >> 2 files changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/samples/livepatch/livepatch-shadow-fix1.c b/samples/livepatch/livepatch-shadow-fix1.c >> index 918ce17b43fda..6701641bf12d4 100644 >> --- a/samples/livepatch/livepatch-shadow-fix1.c >> +++ b/samples/livepatch/livepatch-shadow-fix1.c >> @@ -109,9 +109,9 @@ static void livepatch_fix1_dummy_leak_dtor(void *obj, void *shadow_data) >> void *d = obj; >> int **shadow_leak = shadow_data; >> >> - kfree(*shadow_leak); >> pr_info("%s: dummy @ %p, prevented leak @ %p\n", >> __func__, d, *shadow_leak); >> + kfree(*shadow_leak); >> } >> >> static void livepatch_fix1_dummy_free(struct dummy *d) >> diff --git a/samples/livepatch/livepatch-shadow-fix2.c b/samples/livepatch/livepatch-shadow-fix2.c >> index 29fe5cd420472..361046a4f10cf 100644 >> --- a/samples/livepatch/livepatch-shadow-fix2.c >> +++ b/samples/livepatch/livepatch-shadow-fix2.c >> @@ -61,9 +61,9 @@ static void livepatch_fix2_dummy_leak_dtor(void *obj, void *shadow_data) >> void *d = obj; >> int **shadow_leak = shadow_data; >> >> - kfree(*shadow_leak); >> pr_info("%s: dummy @ %p, prevented leak @ %p\n", >> __func__, d, *shadow_leak); >> + kfree(*shadow_leak); >> } >> >> static void livepatch_fix2_dummy_free(struct dummy *d) >> > Hi Tom, > > Ordering doesn't matter for the example, so let's clean up the static > analysis. > > Acked-by: Joe Lawrence <joe.lawrence@redhat.com> > > But for my sanity, isn't this a false positive? There shouldn't be harm > in printing the pointer itself, even after what it points to has been > freed, i.e. > > int *i = malloc(sizeof(*i)); > free(i); > printf("%p\n", i); << ok > printf("%d\n", *i); << NOT ok > > But I suppose clang doesn't know that the passed pointer isn't getting > dereferenced by the function, so it throws up a warning? Just curious > what your experience has been with respect to these reports.
The analysis it good for static functions, for extern functions it has nothing to analyze so a worst case is assumed.
I agree this is likely a false positive.
Tom
> > Thanks,
| |