Re: apparent bug about check_free_strict

[Dan Carpenter <dan.carpenter@linaro.org>]

On Tue, Nov 25, 2025 at 04:28:03PM +0200, Toomas Soome wrote:
> And another interesting case:
> 
> smatch is complaining about about ‘pptr’ but we do free ‘ptr’. 
> 
> /code/illumos-gate/usr/src/tools/proto/root_i386-nd/opt/onbld/bin/i386/smatch: adm_kef_util.c:1243 filter_mechlist() error: dereferencing freed memory 'pptr' (line 1242)
> 
> 1225 filter_mechlist(mechlist_t **pmechlist, const char *mech)
> 1226 {
> 1227         int             cnt = 0;
> 1228         mechlist_t      *ptr, *pptr;
> 1229         boolean_t       mech_present = B_FALSE;
> 1230  
> 1231         ptr = pptr = *pmechlist;
> 1232  
> 1233         while (ptr != NULL) {
> 1234                 if (strncmp(ptr->name, mech, sizeof (mech_name_t)) == 0) {
> 1235                         mech_present = B_TRUE;
> 1236                         if (ptr == *pmechlist) {
> 1237                                 pptr = *pmechlist = ptr->next;
> 1238                                 free(ptr);
> 1239                                 ptr = pptr;
> 1240                         } else {
> 1241                                 pptr->next = ptr->next;
> 1242                                 free(ptr);
> 1243                                 ptr = pptr->next;

This one is explainable...  Smatch is crap at loops, and only parses the
loop one time.  It might look like Smatch parses loops but it's all
hacks and special cases.

So, in this case, instead of seeing that "this is the second iteration
through the loop", Smatch says "this is dead code, but all of our other
assumptions are probably correct including that "ptr = pptr = *pmechlist".

So when we free "ptr" we're also freeing "pptr".

I've known the correct way to handle loops for over ten years now and
I partially wrote the code ten years ago.  But I've never wanted to do
it because it will slow everything down a lot.  It's quite a bit of
work as well, but mostly it was the slow down that was the issue.
But I think I'm going to try to make Smatch work better on other
projects outside the kernel so adding more and more loop hacks will
become less feasible and I will care less about slow downs so I
have decided I am going to do this work soon.

Basically you just parse every function twice and you store the next
iteration states for every loop.  Then you parse the functions again
and merge in the next iteration states.  It's a 2x slow down in
parsing.  I already have the --two-passes option but I haven't looked
at the output in a while...

regards,
dan carpenter