Skip to content

Bleeding Edge Kernel versions drop module_sect_attr #1761

Closed

Description

@atcuno

This is a struct we currently rely on to extract kernel modules. This structure has been there since the beginning of the kernel but was removed with this commit:

torvalds/linux@34f5ec0

A slack user reported the bug from a preview/bleeding edge Ubuntu kernel, so this will hit mainstream kernels soon enough. I will investigate this week so its fixed for parity.

Activity

  1. self-assigned this
    on Apr 8, 2025
  2. ikelos commented on Apr 8, 2025

    @ikelos
    Member

    I hate to say it, but this is a new feature ("support recent kernels") rather than a fix for something that currently works incorrectly, and I don't think it's enough to break the code freeze. It doesn't yet affect mainstream kernels (you said it was bleeding edge), the existing code isn't broken it just can't handle all cases at the moment (which will always be the case in a moving world) and we have to cut at some point. Feel free to get it fixed in develop, and it'll get released in July or August, whenever our next release is due to be rolled.

  3. added this to the 2.26.1 milestone on Apr 8, 2025
  4. ikelos commented on Apr 8, 2025

    @ikelos
    Member

    It's not a parity release issue since I'm pretty sure volatility 2 will not handle this case.

  5. atcuno commented on Apr 8, 2025

    @atcuno
    ContributorAuthor

    The updated method for module attributes (the reason for this ticket) is in released Ubuntu preview kernels now, which means over the next couple weeks they will go into stable kernels being used in production. It is also not a silent issue, it fully backtraces all of Vol3 for any Linux analysis as we set the extension for module_sect_attr on load. So even running something like linux.pslist on the kernel (unrelated to module extraction) on an affected symbol table will backtrace the whole framework. This is a bug fix type ticket which is supposed to be the purpose of the gap time between now and the parity release. It really does need to be fixed for parity.

  6. ikelos commented on Apr 8, 2025

    @ikelos
    Member

    Well, I'll take a look at the PR once it comes out to see how invasive it is, but targetting a fix for something that comes out in two weeks, is still pre-empting the future, so I'm absolutely middle of the line on this and it could go either way...

  7. ikelos commented on Apr 8, 2025

    @ikelos
    Member

    (It also proves we need to be careful about whether we add class overrides or optional class overrides when we add extensions).

  8. atcuno commented on Apr 8, 2025

    @atcuno
    ContributorAuthor

    Yes, but two weeks from now is during the bug release period. Also, that would put 2-3 months of Linux analysis on up to date kernels backtracking and we just be telling people to use devleop for 2 months instead of the release.

    Also, we do use the optional classes correctly, and it is good we didn't use it here as its a core data structure that hasn't changed since ~15 years - but of course it gets changed right before release. It would be like EPROCESS suddenly disappearing on Windows.

  9. ikelos commented on Apr 8, 2025

    @ikelos
    Member

    Ok, I'm glad we chose the right type for the structure, and yes, it's very disappointing they decided to make that change now.

    As I said, I take a look and see how risky a change it is, but we have to balance it against putting in the change and us getting it wrong or missing something and breaking volatility (perhaps for every linux image, not just new ones) which is why I'm not giving you a guaranteed yes, or a guaranteed no now, but we need to see what the patch looks like before we decide whether it's better to include it now or not. The fact that it breaks all linux analysis rather than just one plugin does swing things in favour of including it.

  10. Abyss-W4tcher commented on Apr 8, 2025

    @Abyss-W4tcher
    Contributor

    Here are my researches towards a fix, if it can help you determine which way to go.

    Doc.

    At first glance, the only place where we require "module_sect_attr" is in get_sections (which by the way doesn't seem referenced anywhere?):

    arr = self._context.object(
    symbol_table_name + constants.BANG + "array",
    layer_name=self.vol.layer_name,
    offset=self.sect_attrs.attrs.vol.offset,
    subtype=self._context.symbol_space.get_type(
    symbol_table_name + constants.BANG + "module_sect_attr"
    ),
    count=self.number_of_sections,
    )

    sect_attrs is of type module_sect_attrs

    Diff is here:

    // https://elixir.bootlin.com/linux/v6.13/source/kernel/module/sysfs.c#L27
    struct module_sect_attrs {
    	struct attribute_group grp;
    	unsigned int nsections;
    	struct module_sect_attr attrs[];
    };

    ->

    // https://elixir.bootlin.com/linux/v6.14-rc6/source/kernel/module/sysfs.c#L22
    struct module_sect_attrs {
    	struct attribute_group grp;
    	struct bin_attribute attrs[];
    };

    And the now removed module_sect_attr was:

    struct module_sect_attr {
    	struct bin_attribute battr;
    	unsigned long address;
    };

    module_sect_attr was removed because it was indeed only a proxy to a bin_attribute structure through its battr member.

    Fix

    • Make module_sect_attr optional
    diff --git a/volatility3/framework/symbols/linux/extensions/__init__.py b/volatility3/framework/symbols/linux/extensions/__init__.py
    index b4bde1ab..74ad205b 100644
    --- a/volatility3/framework/symbols/linux/extensions/__init__.py
    +++ b/volatility3/framework/symbols/linux/extensions/__init__.py
    @@ -212,9 +212,7 @@ class module(generic.GenericIntelProcess):
                 symbol_table_name + constants.BANG + "array",
                 layer_name=self.vol.layer_name,
                 offset=self.sect_attrs.attrs.vol.offset,
    -            subtype=self._context.symbol_space.get_type(
    -                symbol_table_name + constants.BANG + "module_sect_attr"
    -            ),
    +            subtype=self.sect_attrs.attrs.vol.subtype,
                 count=self.number_of_sections,
             )

    Then add an optional "bin_attribute" class override with a "get_name" method that should basically only need to mimic

    return utility.pointer_to_string(self.battr.attr.name, count=32)

  11. Abyss-W4tcher commented on Apr 8, 2025

    @Abyss-W4tcher
    Contributor

    I'll potentially update the previous comment if I find anything else, so consider this WiP for today.

  12. ikelos commented on Apr 8, 2025

    @ikelos
    Member

    So it sounds like the fix would be to make a module_sect_attrs class override, that determines whether it contains the needed classes, and then either processes attrs to yields each of the bin_attributes out of the module_sect_attrs or yields through a direct array of bin_attributes?

  13. Abyss-W4tcher commented on Apr 9, 2025

    @Abyss-W4tcher
    Contributor

    I don't think there is a need to tamper with module_sect_attrs, as the attrs array subtype class override (bin_attribute or module_sect_attr) will already handle members checking. This makes it transparent to get_name() callers, like here (because this one does not hardcode module_sect_attr in the subtype):

    attribute_type = module.sect_attrs.attrs.vol.subtype
    sect_array = vmlinux.object(
    object_type="array",
    subtype=attribute_type,
    offset=module.sect_attrs.attrs.vol.offset,
    count=num_sections,
    absolute=True,
    )
    sections: Dict[int, str] = {}
    # for each section, gather its name and address
    for index, section in enumerate(sect_array):
    name = section.get_name()

    I can make a PR with how I think it could be resolved, so that it would a bit clearer of what would move and if it's too risky or not before 2.26.0.

  14. atcuno commented on Sep 27, 2025

    @atcuno
    ContributorAuthor

    @Abyss-W4tcher is this ready / close to ready?

  15. added a commit that references this issue on Dec 29, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Type

No type

Projects

No projects

    Milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions