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

Hi Kazu, On Tue, Dec 10, 2024 at 7:02 PM HAGIO KAZUHITO(萩尾 一仁) <k-hagio-ab(a)nec.com&gt; wrote: > > 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 for your comments, so ack for the above code changes.

Thanks, Tao Liu > > Thanks, > Kazu