apparent bug about check_free_strict

Toomas Soome <[email protected]> Mon, 18 Nov 2024 13:55:30 +0200
Newsgroups org.kernel.vger.smatch
Message-ID <[email protected]>
hi!

I did enable illumos kernel memory allocation/free checks =
(kmem_alloc/kmem_free) and apparently I did find something interesting.

The warning is:
=
/code/illumos-gate/usr/src/tools/proto/root_i386-nd/opt/onbld/bin/i386/sma=
tch: ../../common/os/devcfg.c:8583 e_ddi_retire_device() warn: passing =
freed memory 'pdip'
=
/code/illumos-gate/usr/src/tools/proto/root_i386-nd/opt/onbld/bin/i386/sma=
tch: ../../common/os/devcfg.c:8612 e_ddi_retire_device() warn: passing =
freed memory 'dip'
=
/code/illumos-gate/usr/src/tools/proto/root_i386-nd/opt/onbld/bin/i386/sma=
tch: ../../common/os/devcfg.c:8621 e_ddi_retire_device() warn: passing =
freed memory =E2=80=98dip'

The code for first error about pdip is:

  8572          pdip =3D ddi_get_parent(dip);
  8573          ndi_hold_devi(pdip);
  8574     8575          /*
  8576           * Run devfs_clean() in case dip has no constraints and =
is
  8577           * not in use, so is retireable but there are dv_nodes =
holding
  8578           * ref-count on the dip. Note that devfs_clean() always =
returns
  8579           * success.
  8580           */
  8581          devnm =3D kmem_alloc(MAXNAMELEN + 1, KM_SLEEP);
  8582          (void) ddi_deviname(dip, devnm);
  8583          (void) devfs_clean(pdip, devnm + 1, DV_CLEAN_FORCE);
  8584          kmem_free(devnm, MAXNAMELEN + 1);
  8585     8586          ndi_devi_enter(pdip);

We get this error about pdip with devfs_clean(), but apparently the =
=E2=80=98freed=E2=80=99 state is set with ndi_hold_devi(pdip) call; of =
course the call itself is not the quilty one, but the construct is =E2=80=94=
 as soon as I either comment the ndi_hold_devi() out *or* if I move it =
down before devfs_clean(), then the error disappears.

Therefore, it appears that code segment such as:

var =3D f();
g(var);

is causing state of var to be set =E2=80=98freed=E2=80=99 and =
check_free_strict.c is ending up spitting out the warning about passing =
freed memory with next function call.


now the next warning is about code:

  8609          constraint =3D 1; /* assume constraints allow retire */
  8610          (void) e_ddi_retire_notify(dip, &constraint);
  8611          if (!is_leaf_node(dip)) {
  8612                  ndi_devi_enter(dip);
  8613                  ddi_walk_devs(ddi_get_child(dip), =
e_ddi_retire_notify,
  8614                      &constraint);
  8615                  ndi_devi_exit(dip);
  8616          }
  8617
  8618          /*
  8619           * Now finalize the retire
  8620           */
  8621          (void) e_ddi_retire_finalize(dip, &constraint);
  8622          if (!is_leaf_node(dip)) {
  8623                  ndi_devi_enter(dip);
  8624                  ddi_walk_devs(ddi_get_child(dip), =
e_ddi_retire_finalize,
  8625                      &constraint);
  8626                  ndi_devi_exit(dip);
  8627          }

Here we do get warning about ndi_devi_enter(), but if I replace dip in =
is_leaf_node() by NULL, we do not get any more warnings about =E2=80=98dip=
=E2=80=99.

PS: the line number differences with git is because my branch has other =
change fixing memory leak discovered by smatch:D

rgds,
toomas=