[Crash-utility] Re: [PATCH] Fix infinite loop during module symbols initialization

Hi Tao, On 2024/12/10 12:39, Tao Liu wrote: > I can also reproduce this issue on 6.13.0-rc2 machine. The > mod_symname_hash_install() function missed the case if one spn was > installed 2 more times, which as a result, will cause the infinite > loop. > > The reason for installing 2 more times is because the addresses > overlap between 2 kernel modules. See the following: > > crash> sym -l > ffffffffc0800000 MODULE TEXT START: qemu_fw_cfg > ffffffffc0800000 (t) __pfx_fw_cfg_sysfs_attr_show > ffffffffc0800010 (t) fw_cfg_sysfs_attr_show > ... > ffffffffc0801000 MODULE TEXT START: serio_raw > ffffffffc0801000 (t) __pfx_serio_raw_poll > ffffffffc0801000 MODULE TEXT END: qemu_fw_cfg > ... > ffffffffc0802000 MODULE TEXT END: serio_raw > > For module qemu_fw_cfg, 3 symbols of ffffffffc0801000 will be > installed. Then for module serio_raw, 3 symbols of ffffffffc0801000 > will be reinstalled. Due to the lack of repeat symbol check, there > will be symbol reinstall, and infinite loop: > > sp->name_hash_next = spn; <<----- name_hash_next point to itself if sp==spn > > I think Kazu's "base + size - 1" solution is reasonable, this will let > qemu_fw_cfg install range from ffffffffc0800000 to > (ffffffffc0801000-1), so no overlap for 2 modules. In addition, I'd > like to add the following code as well, to skip if a symbol has > already been installed: > > diff --git a/symbols.c b/symbols.c > index a26bdc1..e1a62bb 100644 > --- a/symbols.c > +++ b/symbols.c > @@ -1233,6 +1233,9 @@ mod_symname_hash_install(struct syment *spn) > return; > } > for (; sp; sp = sp->name_hash_next) { > + if (spn == sp) > + return; > + > if (!sp->name_hash_next || > spn->value < sp->name_hash_next->value) { > spn->name_hash_next = sp->name_hash_next; > > What do you think? thank you for checking the issue, it's ok to add it for future safety.

Thanks, Kazu