On Wed, Dec 11, 2024 at 5:21 AM Tao Liu <ltao@xxxxxxxxxx> wrote:
Hi Kazu,
On Tue, Dec 10, 2024 at 7:02 PM HAGIO KAZUHITO(萩尾 一仁)
<k-hagio-ab@xxxxxxx> 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.
Applied(with the code changes):
Thanks,
Tao Liu
>
> Thanks,
> Kazu
-- Crash-utility mailing list -- devel@xxxxxxxxxxxxxxxxxxxxxxxxxxx To unsubscribe send an email to devel-leave@xxxxxxxxxxxxxxxxxxxxxxxxxxx https://${domain_name}/admin/lists/devel.lists.crash-utility.osci.io/ Contribution Guidelines: https://github.com/crash-utility/crash/wiki
