linux-kernel - Re: [PATCH v6] coccinelle: api: add kvfree script

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <alpine.DEB.2.22.394.2008031416301.27678@hadrien>
Date:   Mon, 3 Aug 2020 14:18:03 +0200 (CEST)
From:   Julia Lawall <julia.lawall@...ia.fr>
To:     Denis Efremov <efremov@...ux.com>
cc:     cocci@...teme.lip6.fr, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v6] coccinelle: api: add kvfree script



On Mon, 3 Aug 2020, Denis Efremov wrote:

>
>
> On 8/2/20 11:24 PM, Julia Lawall wrote:
> >> +@...tialize:python@
> >> +@@
> >> +# low-level memory api
> >> +filter = frozenset(['__vmalloc_area_node'])
> >> +
> >> +def relevant(p):
> >> +    return not (filter & {el.current_element for el in p})
> >
> > Is this used?
>
> I'll remove it in v8. Or do you want me to add iterate_dir_item() in the list?

What is that?

>
> >
> > Otherwise, I think it would be good to not warn about a use of kvfree
> > if that use is reachable from a kvmalloc.  There seems to be such a false
> > positive in fs/btrfs/send.c, on line 1118.
>
> I don't know how to handle this case without position filter.
> It's too complex. In iterate_dir_item() there is:
> buf = kmalloc(buf_len, GFP_KERNEL);
> while(...) {
> 	if (...) {
> 		if (is_vmalloc_addr(buf)) {
> 			vfree(buf);
> 			...
> 		} else {
> 			char *tmp = krealloc(buf, ...);
>
> 			if (!tmp)
> 				kfree(buf);
> 			...
> 		}
> 		if (!buf) {
> 			buf = kvmalloc(buf_len, GFP_KERNEL);
> 			...
> 		}
> 	}
> }
> kvfree(buf);
>
> Adding "when != kvfree(E)" is not enough:
> * E = \(kvmalloc\|kvzalloc\|kvcalloc\|kvzalloc_node\|kvmalloc_node\|
> *       kvmalloc_array\)(...)@k
> 	... when != is_vmalloc_addr(E)
> +	when != kvfree(E)
> 	when any
> * \(kfree\|kzfree\|vfree\|vfree_atomic\)(E)@p

Why not just

@ok exists@
position p;
expression E;
@@

E = kvalloc(...)
...
kvfree@p(...)

Probably that is what you mean by a position filter, but why not add a
position filter?

julia


> >
> > It also seems that when there are both a kmalloc and a vmalloc, there is
> > no warning if kfree or vfree is used.  Is that intentional?
> >
>
> No, I will try to address it in v8.
>
> Regards,
> Denis
>