linux-kernel - Re: [PATCH v3 1/7] staging: r8188eu: remove unused member free_bss

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 for Android: free password hash cracker in your pocket

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

Date:   Wed, 20 Apr 2022 14:55:34 +0300
From:   Dan Carpenter <dan.carpenter@...cle.com>
To:     Pavel Skripkin <paskripkin@...il.com>
Cc:     Jaehee Park <jhpark1013@...il.com>, Larry.Finger@...inger.net,
        phil@...lpotter.co.uk, gregkh@...uxfoundation.org,
        linux-staging@...ts.linux.dev, linux-kernel@...r.kernel.org,
        outreachy@...ts.linux.dev
Subject: Re: [PATCH v3 1/7] staging: r8188eu: remove unused member
 free_bss_buf

As a kernel community, I don't think we are pro-active enough about
preventing bugs.  One of my co-workers and I have started a bi weekly
phone call to look at CVEs from the previous week and discuss how they
could have been prevented or detected earlier.  Of course, my solutions
are always centered around static analysis because that's my deal but
some bugs are caused by process issues, or could be detected with better
testing.

In this case there were two bugs proposed bugs.

1) The memory leak that Fabio noticed.  Smatch is bad at detecting
   memory leaks.  It's a hard problem, because in this case it's
   across function boundaries.

Fabio caught the leak.  I don't know if I would have.

2) The NULL dereference.

   The "&pnetwork->list" expression is not a dereference so this is also
   a cross function thing.  I thought I used to have an unpublished check
   for bogus addresses like that where pnetwork is NULL.

   Another is idea is that when you have pnetwork++ and it's a NULL
   pointer then print an error message.  Or even potentially NULL.
   There are various heuristics to use which mean that "A reasonable
   person would think this could be NULL".

   Or another idea would be that we could test patches.  Right now we
   don't really have a way to test these.  But, of course, we wish we
   did.

It's not super likely that we would have committed the NULL deref
patch.  I would have caught that one if you didn't and Fabio likely
would have as well.  But I like to remove the human error whenever I
can.

regards,
dan carpenter