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=