Messages in this thread | | | From | Denis Efremov <> | Subject | Re: [PATCH] coccinelle: api: add kzfree script | Date | Thu, 4 Jun 2020 20:22:13 +0300 |
| |
> Could you send an example of some C code on which the result is not > suitable?
I've updated the pattern to handle false positives:
@ifok@ position p; expression *E; @@
( if (...) { ... memset(E, 0, ...)@p; ... } | if (...) { ... } else { ... memset(E, 0, ...)@p; ... } )
// Ignore kzfree definition // Ignore kasan test @r depends on !patch && !(file in "lib/test_kasan.c") && !(file in "mm/slab_common.c")@ expression *E; position p != ifok.p; @@
* memset(E, 0, ...)@p; ... when != E when != if (...) { ... E ... } when != for (...;...;...) { ... E ... } when != while (...) { ... E ... } when strict * kfree(E);
Example of false positives:
void test_memset_under_if(void) { char *p = malloc(10, GFP_KERNEL); if (p % 5) { p[5] = 1; } else { memset(p, 0, 10); } kfree(p); }
void test_memset_under_if(void) { int i; char *p = malloc(10, GFP_KERNEL); for (i = 0; i < 10; ++i) { memset(p, 0, 10); } kfree(p); }
void test_E_in_if(void) { char *p = malloc(10, GFP_KERNEL); memset(p, 0, 10); // when != E is not enough if (10) { // when != if (...) { ... E ... } is required p[5] = 1; } kfree(p); }
void test_E_in_for(void) { char *p = malloc(10, GFP_KERNEL); memset(p, 0, 10); for(;;) { p[5] = 1; } kfree(p); }
void test_E_in_while(void) { char *p = malloc(10, GFP_KERNEL); memset(p, 0, 10); while(1) { p[6] = 2; } kfree(p); }
void test_E_in_struct(void) { struct t { int a[3]; }; struct t *p = malloc(10 * sizeof(struct(struct t)), GFP_KERNEL); memset(p, 0, 10); for(;;) { if (1) { p->a[2] = 1; // I give up on this p->a[0] = 10; } } kfree(p); }
After all it seems reasonable to me to add forall and memset_explicit rather than handle all these false positives. Something like this for v2?
@r depends on !patch && !(file in "lib/test_kasan.c") && !(file in "mm/slab_common.c") forall@ expression *E; position p; @@
* \(memset\|memset_explicit\)(E, 0, ...); ... when != E * kfree(E)@p;
Do I need to add "when strict" with forall or it's already enabled in this case? Do I need to enable forall for pathing "-/+"?
Thanks, Denis
| |