Re: [PATCH] coccinelle: api: add kzfree script

From: Denis Efremov
Date: Thu Jun 04 2020 - 13:22:24 EST


> 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