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  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]
Date:   Thu, 3 Mar 2022 13:56:02 +0300
From:   Dan Carpenter <dan.carpenter@...cle.com>
To:     Rasmus Villemoes <linux@...musvillemoes.dk>,
        Julia Lawall <Julia.Lawall@...ia.fr>
Cc:     Linus Torvalds <torvalds@...ux-foundation.org>,
        David Laight <David.Laight@...lab.com>,
        James Bottomley <James.Bottomley@...senpartnership.com>,
        linux-wireless <linux-wireless@...r.kernel.org>,
        "alsa-devel@...a-project.org" <alsa-devel@...a-project.org>,
        KVM list <kvm@...r.kernel.org>,
        "Gustavo A. R. Silva" <gustavo@...eddedor.com>,
        "linux-iio@...r.kernel.org" <linux-iio@...r.kernel.org>,
        "nouveau@...ts.freedesktop.org" <nouveau@...ts.freedesktop.org>,
        dri-devel <dri-devel@...ts.freedesktop.org>,
        Cristiano Giuffrida <c.giuffrida@...nl>,
        "Bos, H.J." <h.j.bos@...nl>,
        "linux1394-devel@...ts.sourceforge.net" 
        <linux1394-devel@...ts.sourceforge.net>,
        "drbd-dev@...ts.linbit.com" <drbd-dev@...ts.linbit.com>,
        linux-arch <linux-arch@...r.kernel.org>,
        CIFS <linux-cifs@...r.kernel.org>,
        "linux-aspeed@...ts.ozlabs.org" <linux-aspeed@...ts.ozlabs.org>,
        linux-scsi <linux-scsi@...r.kernel.org>,
        linux-rdma <linux-rdma@...r.kernel.org>,
        "linux-staging@...ts.linux.dev" <linux-staging@...ts.linux.dev>,
        amd-gfx list <amd-gfx@...ts.freedesktop.org>,
        Jason Gunthorpe <jgg@...pe.ca>,
        "intel-wired-lan@...ts.osuosl.org" <intel-wired-lan@...ts.osuosl.org>,
        "kgdb-bugreport@...ts.sourceforge.net" 
        <kgdb-bugreport@...ts.sourceforge.net>,
        "bcm-kernel-feedback-list@...adcom.com" 
        <bcm-kernel-feedback-list@...adcom.com>,
        Linux Media Mailing List <linux-media@...r.kernel.org>,
        Kees Cook <keescook@...omium.org>,
        Arnd Bergman <arnd@...db.de>,
        Linux PM <linux-pm@...r.kernel.org>,
        intel-gfx <intel-gfx@...ts.freedesktop.org>,
        Brian Johannesmeyer <bjohannesmeyer@...il.com>,
        Nathan Chancellor <nathan@...nel.org>,
        dma <dmaengine@...r.kernel.org>,
        Christophe JAILLET <christophe.jaillet@...adoo.fr>,
        Jakob Koschel <jakobkoschel@...il.com>,
        "v9fs-developer@...ts.sourceforge.net" 
        <v9fs-developer@...ts.sourceforge.net>,
        linux-tegra <linux-tegra@...r.kernel.org>,
        Thomas Gleixner <tglx@...utronix.de>,
        Andy Shevchenko <andriy.shevchenko@...ux.intel.com>,
        Linux ARM <linux-arm-kernel@...ts.infradead.org>,
        "linux-sgx@...r.kernel.org" <linux-sgx@...r.kernel.org>,
        linux-block <linux-block@...r.kernel.org>,
        Netdev <netdev@...r.kernel.org>,
        "linux-usb@...r.kernel.org" <linux-usb@...r.kernel.org>,
        "samba-technical@...ts.samba.org" <samba-technical@...ts.samba.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Linux F2FS Dev Mailing List 
        <linux-f2fs-devel@...ts.sourceforge.net>,
        "tipc-discussion@...ts.sourceforge.net" 
        <tipc-discussion@...ts.sourceforge.net>,
        Linux Crypto Mailing List <linux-crypto@...r.kernel.org>,
        linux-fsdevel <linux-fsdevel@...r.kernel.org>,
        "linux-mediatek@...ts.infradead.org" 
        <linux-mediatek@...ts.infradead.org>,
        Andrew Morton <akpm@...ux-foundation.org>,
        linuxppc-dev <linuxppc-dev@...ts.ozlabs.org>,
        Christian K├Ânig <christian.koenig@....com>,
        Mike Rapoport <rppt@...nel.org>
Subject: Re: [PATCH 2/6] treewide: remove using list iterator after loop body
 as a ptr

On Wed, Mar 02, 2022 at 10:29:31AM +0100, Rasmus Villemoes wrote:
> This won't help the current issue (because it doesn't exist and might
> never), but just in case some compiler people are listening, I'd like to
> have some sort of way to tell the compiler "treat this variable as
> uninitialized from here on". So one could do
> 
> #define kfree(p) do { __kfree(p); __magic_uninit(p); } while (0)
> 

I think this is a good idea.

Smatch can already find all the iterator used outside the loop bugs that
Jakob did with a manageably small number of false positives.  The
problems are that:
1) It would be better to find it in the compile stage instead of later.
2) I hadn't published that check.  Will do shortly.
3) A couple weeks back I noticed that the list_for_each_entry() check
   was no longer working.  Fixed now.
4) Smatch was only looking at cases which dereferenced the iterator and
   not checks for NULL.  I will test the fix for that tonight.
5) Smatch is broken on PowerPC.

Coccinelle also has checks for iterator used outside the loop.
Coccinelle had these checks before Smatch did.  I copied Julia's idea.

If your annotation was added to GCC it would solve all those problems.

But it's kind of awkward that we can't annotate kfree() directly
instead of creating the kfree() macro.  And there are lots of other
functions which free things so you'd have to create a ton of macros
like:

#define gr_free_dma_desc(a, b) do { __gr_free_dma_desc(a, b); __magic_uninit(b); } while (0)

And then there are functions which free a struct member:

void free_bar(struct foo *p) { kfree(p->bar); }

Or functions which free a container_of().

Smatch is more evolved than designed but what I do these days is use $0,
$1, $2 to represent the parameters.  So you can say a function frees
$0->bar.  For container_of() then is "(168<~$0)->bar" which means 168
bytes from $0.  Returns are parameter -1 so I guess it would be $(-1),
but as I said Smatch evolved so right now places that talk about
returned values use a different format.

What you could do is just make a parseable table next to the function
definition with all the information.  Then you would use a Perl script
to automatically generate a Coccinelle check to warn about use after
frees.

diff --git a/mm/slab.c b/mm/slab.c
index ddf5737c63d9..c9dffa5c40a2 100644
--- a/mm/slab.c
+++ b/mm/slab.c
@@ -3771,6 +3771,9 @@ EXPORT_SYMBOL(kmem_cache_free_bulk);
  *
  * Don't free memory not originally allocated by kmalloc()
  * or you will run into trouble.
+ *
+ * CHECKER information
+ * frees: $0
  */
 void kfree(const void *objp)
 {

regards,
dan carpenter

Powered by blists - more mailing lists