Re: apparent bug about check_free_strict

On Wed, Nov 26, 2025 at 04:47:04PM +0200, Toomas Soome wrote:
>=20
>=20
> > On 25. Nov 2025, at 16:50, Dan Carpenter <[email protected]> wro=
te:
> >=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 fre=
e =E2=80=98ptr=E2=80=99.=20
> >>=20
> >> /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 m=
emory '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 *pmec=
hlist".
> >=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
>=20
> example about =E2=80=94two-passes:
>=20
> tsoome@balrog:/code/illumos-gate/usr/src/cmd/cmd-inet/usr.lib/in.mpathd$ =
timex /code/illumos-gate/usr/src/tools/proto/root_i386-nd/opt/onbld/bin/i38=
6/smatch -fident -finline -fno-inline-functions -fno-builtin -fno-asm -fdia=
gnostics-show-option -nodefaultlibs -D__sun -O -m32 -Wall -Wextra -Werror -=
Wno-missing-braces -Wno-sign-compare -Wno-unused-parameter -Wno-missing-fie=
ld-initializers -Wno-array-bounds -p=3Dillumos_user --disable=3Duninitializ=
ed,check_check_deref -Wno-vla -Wno-one-bit-signed-bitfield -Wno-external-fu=
nction-has-definition -Wno-old-style-definition -Wno-strict-prototypes --fa=
tal-checks --timeout=3D0 -Wno-maybe-uninitialized -std=3Dgnu99 -fno-inline-=
small-functions -fno-inline-functions-called-once -fno-ipa-cp -fno-ipa-icf =
-fno-clone-functions -fno-reorder-functions -fno-reorder-blocks-and-partiti=
on -fno-aggressive-loop-optimizations --param=3Dmax-inline-insns-single=3D4=
50 -fstack-protector-strong -g -gdwarf-4 -gstrict-dwarf -std=3Dgnu99 -DTEXT=
_DOMAIN=3D"SUNW_OST_OSCMD" -D_TS_ERRNO -I/code/illumos-gate/proto/root_i386=
/usr/include -D_XOPEN_SOURCE=3D600 -D__EXTENSIONS__ -c mpd_main.c -o /tmp/c=
w.f6aqiX/cwi6aOiX.o=20
> /code/illumos-gate/usr/src/tools/proto/root_i386-nd/opt/onbld/bin/i386/sm=
atch: mpd_main.c:154 poll_add() warn: potentially one past the end of array=
 'newfds[i]'
> /code/illumos-gate/usr/src/tools/proto/root_i386-nd/opt/onbld/bin/i386/sm=
atch: mpd_main.c:154 poll_add() warn: potentially one past the end of array=
 'newfds[i]'
> /code/illumos-gate/usr/src/tools/proto/root_i386-nd/opt/onbld/bin/i386/sm=
atch: mpd_main.c:155 poll_add() warn: potentially one past the end of array=
 'newfds[i]'
> /code/illumos-gate/usr/src/tools/proto/root_i386-nd/opt/onbld/bin/i386/sm=
atch: mpd_main.c:155 poll_add() warn: potentially one past the end of array=
 'newfds[i]'
>=20
> real           1.16
> user           0.99
> sys            0.15
>=20
> tsoome@balrog:/code/illumos-gate/usr/src/cmd/cmd-inet/usr.lib/in.mpathd$ =
timex /code/illumos-gate/usr/src/tools/proto/root_i386-nd/opt/onbld/bin/i38=
6/smatch --two-passes -fident -finline -fno-inline-functions -fno-builtin -=
fno-asm -fdiagnostics-show-option -nodefaultlibs -D__sun -O -m32 -Wall -Wex=
tra -Werror -Wno-missing-braces -Wno-sign-compare -Wno-unused-parameter -Wn=
o-missing-field-initializers -Wno-array-bounds -p=3Dillumos_user --disable=
=3Duninitialized,check_check_deref -Wno-vla -Wno-one-bit-signed-bitfield -W=
no-external-function-has-definition -Wno-old-style-definition -Wno-strict-p=
rototypes --fatal-checks --timeout=3D0 -Wno-maybe-uninitialized -std=3Dgnu9=
9 -fno-inline-small-functions -fno-inline-functions-called-once -fno-ipa-cp=
 -fno-ipa-icf -fno-clone-functions -fno-reorder-functions -fno-reorder-bloc=
ks-and-partition -fno-aggressive-loop-optimizations --param=3Dmax-inline-in=
sns-single=3D450 -fstack-protector-strong -g -gdwarf-4 -gstrict-dwarf -std=
=3Dgnu99 -DTEXT_DOMAIN=3D"SUNW_OST_OSCMD" -D_TS_ERRNO -I/code/illumos-gate/=
proto/root_i386/usr/include -D_XOPEN_SOURCE=3D600 -D__EXTENSIONS__ -c mpd_m=
ain.c -o /tmp/cw.3UaygX/cw5UaWgX.o =20
>=20
> real          19.70
> user          18.86
> sys            0.79
>=20
> tsoome@balrog:/code/illumos-gate/usr/src/cmd/cmd-inet/usr.lib/in.mpathd$
>=20
> Yes, it took longer, but also no complaints:)

Ugh...  19 times longer.  :(

I'm super not excited about that.  TBH, I haven't tested this in years...

regards,
dan carpenter