Re: apparent bug about check_free_strict

> On 25. Nov 2025, at 16:50, Dan Carpenter <[email protected]> =
wrote:
>=20
> On Tue, Nov 25, 2025 at 04:28:03PM +0200, Toomas Soome wrote:
>> And another interesting case:
>>=20
>> smatch is complaining about about =E2=80=98pptr=E2=80=99 but we do =
free =E2=80=98ptr=E2=80=99.=20
>>=20
>> =
/code/illumos-gate/usr/src/tools/proto/root_i386-nd/opt/onbld/bin/i386/sma=
tch: adm_kef_util.c:1243 filter_mechlist() error: dereferencing freed =
memory 'pptr' (line 1242)
>>=20
>> 1225 filter_mechlist(mechlist_t **pmechlist, const char *mech)
>> 1226 {
>> 1227         int             cnt =3D 0;
>> 1228         mechlist_t      *ptr, *pptr;
>> 1229         boolean_t       mech_present =3D B_FALSE;
>> 1230 =20
>> 1231         ptr =3D pptr =3D *pmechlist;
>> 1232 =20
>> 1233         while (ptr !=3D NULL) {
>> 1234                 if (strncmp(ptr->name, mech, sizeof =
(mech_name_t)) =3D=3D 0) {
>> 1235                         mech_present =3D B_TRUE;
>> 1236                         if (ptr =3D=3D *pmechlist) {
>> 1237                                 pptr =3D *pmechlist =3D =
ptr->next;
>> 1238                                 free(ptr);
>> 1239                                 ptr =3D pptr;
>> 1240                         } else {
>> 1241                                 pptr->next =3D ptr->next;
>> 1242                                 free(ptr);
>> 1243                                 ptr =3D pptr->next;
>=20
> 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.
>=20
> 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 =3D pptr =3D =
*pmechlist".
>=20
> So when we free "ptr" we're also freeing "pptr".
>=20
> 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.
>=20
> 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...
>=20
> regards,
> dan carpenter

I see.

For positive side, the current smatch has been able to detect many bugs =
the previous versions had missed - its definitely good progress. While =
there are some issues, we can disable checks on such cases. Only problem =
is that where previously =E2=80=94disable=3Dcheck_free_strict did work, =
=E2=80=94disable=3Dcheck_free does not seem to:)

thanks,
toomas=