Skip to content

Add mnt_parent check to kernel version validation#1590

Merged
ikelos merged 2 commits intodevelopfrom
mnt_parent_missing
Feb 1, 2025
Merged

Add mnt_parent check to kernel version validation#1590
ikelos merged 2 commits intodevelopfrom
mnt_parent_missing

Conversation

@atcuno
Copy link
Contributor

@atcuno atcuno commented Jan 31, 2025

@gcmoreira I ran into the following backtrace when testing the latest develop version with the Linux updates. The code is breaking as mnt_parent isn't in the structure.

I then saw the check about this member was recently removed (changed?):

74a834b

So I then changed to what you see in the PR and file descriptors are again returned correctly.

The kernel version for the sample is:

Platform: Ubuntu 22.04 LTS (Jammy Jellyfish) version 5.15.0-33-generic (buildd@lcy02-amd64-037) (gcc (Ubuntu 11.2.0-19ubuntu1) 11.2.0, GNU ld (GNU Binutils for Ubuntu) 2.38) #34-Ubuntu SMP Wed May 18 13:34:26 UTC 2022

Is my PR accurate or do you want another fix?

Traceback (most recent call last):
  File "/home/ub/volatility3/vol.py", line 11, in <module>
    volatility3.cli.main()
  File "/home/ub/volatility3/volatility3/cli/__init__.py", line 924, in main
    CommandLine().run()
  File "/home/ub/volatility3/volatility3/cli/__init__.py", line 512, in run
    renderer.render(grid)
  File "/home/ub/volatility3/volatility3/cli/text_renderer.py", line 232, in render
    grid.populate(visitor, outfd)
  File "/home/ub/volatility3/volatility3/framework/renderers/__init__.py", line 240, in populate
    for level, item in self._generator:
  File "/home/ub/volatility3/volatility3/framework/plugins/linux/lsof.py", line 173, in _generator
    for fd_internal in self.list_fds(
  File "/home/ub/volatility3/volatility3/framework/plugins/linux/lsof.py", line 168, in list_fds
    for fd_fields in fd_generator:
  File "/home/ub/volatility3/volatility3/framework/symbols/linux/__init__.py", line 361, in files_descriptors_for_process
    full_path = LinuxUtilities.path_for_file(context, task, filp)
                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/ub/volatility3/volatility3/framework/symbols/linux/__init__.py", line 328, in path_for_file
    ret = LinuxUtilities._get_path_file(task, filp)
          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/ub/volatility3/volatility3/framework/symbols/linux/__init__.py", line 115, in _get_path_file
    return cls.do_get_path(rdentry, rmnt, dentry, vfsmnt)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/ub/volatility3/volatility3/framework/symbols/linux/__init__.py", line 175, in do_get_path
    if not vfsmnt.has_parent():
           ^^^^^^^^^^^^^^^^^^^
  File "/home/ub/volatility3/volatility3/framework/symbols/linux/extensions/__init__.py", line 1689, in has_parent
    return self.mnt_parent != self.vol.offset
           ^^^^^^^^^^^^^^^
  File "/home/ub/volatility3/volatility3/framework/objects/__init__.py", line 970, in __getattr__
    raise AttributeError(
AttributeError: StructType has no attribute: symbol_table_name1!vfsmount.mnt_parent. Did you mean: 'has_parent'?

@gcmoreira
Copy link
Contributor

gcmoreira commented Jan 31, 2025

Hm that's very weird.. that means there wasn't a mount type for that 5.5 kernel???.
Also, it cannot be the mount struct isn't present and the vfsmount struct doesn't have mnt_parent member. That was an atomic change where it was removed from vfsmount and added to the mount struct here and which was merged on 3.3. Also, the mount struct was added in a previous commit and was also merged in 3.3. That's why checking for the presence of mount or the absence of mnt_parent in vfsmount is interchangeable, the comment above that line explains all this too.
I see mount and mnt_parent still on 5.15 and even on the latest 6.13.

Not sure what happened there, maybe 5.15.0-33-generic isn't a stable kernel. I don't see that in ddebs
or maybe you have a corrupt ISF or vmlinux?

Unfortunately, I don't have that specific vmlinux version or ISF, but what you get with these commands?

$ pahole vmlinux-5.15.0-47-generic -C mount | head -1
struct mount {

$ pahole vmlinux-5.15.0-47-generic -C vfsmount | grep mnt_parent | wc -l
0


$ xzcat Ubuntu_5.15.0-87-generic_5.15.0-87.97_amd64_jammy_22.04.json.xz | jq ".user_types.mount" | head
{
  "size": 320,
  "fields": {
...

$ xzcat Ubuntu_5.15.0-87-generic_5.15.0-87.97_amd64_jammy_22.04.json.xz | jq ".user_types.vfsmount" | grep mnt_parent | wc -l
0

@gcmoreira
Copy link
Contributor

@atcuno Anyway, your changes won’t cause any harm, but they are somewhat redundant. Themount type check cannot fail on a 5.15 kernel, so there must be another underlying issue that requires further debugging. It would be interesting and useful to investigate it further.

@atcuno
Copy link
Contributor Author

atcuno commented Feb 1, 2025

$ cat Ubuntu_5.15.0-33.34_5.15.0-33-generic_x64.json | jq ".user_types.mount" | head
{
  "size": 320,
  "fields": {
    "mnt": {
      "type": {
        "kind": "struct",
        "name": "vfsmount"
      },
      "offset": 32
    },
$ cat Ubuntu_5.15.0-33.34_5.15.0-33-generic_x64.json | jq ".user_types.vfsmount" | grep mnt_parent | wc -l
0

Does this help @gcmoreira

@gcmoreira
Copy link
Contributor

If the mount type is present, not self._context.symbol_space.has_type("mount") should return False. In the backtrace above, in has_parent() it's executing the "True" part of the if statement, which doesn't make sense.

    def has_parent(self) -> bool:
        if self._is_kernel_prior_to_struct_mount():
            return self.mnt_parent != self.vol.offset
        else:
            return self._get_real_mnt().has_parent()

@atcuno
Copy link
Contributor Author

atcuno commented Feb 1, 2025

@ikelos Gus and I discussed this quite a bit on Slack and it was decided that reverting 74a834b is the best fix for this.

@atcuno atcuno requested a review from ikelos February 1, 2025 01:08
@gcmoreira
Copy link
Contributor

This bug was introduced in #1545. To avoid the overhead of doing

vmlinux = linux.LinuxUtilities.get_module_from_volobj_type(self._context, self)
vmlinux.has_type("mount")

I did:

self._context.symbol_space.has_type("mount")

But it's not the same. Since, as I explained above, checking for the presence of mount or the absence of mnt_parent in vfsmount leads to the same result in this case, we should revert that line to its original state:

return self.has_member("mnt_parent")

@ikelos ikelos merged commit 5a046ea into develop Feb 1, 2025
24 checks passed
@ikelos ikelos deleted the mnt_parent_missing branch February 1, 2025 01:47
@ikelos
Copy link
Member

ikelos commented Feb 1, 2025

I can live with the redundancy, so happy to have merged this version.

@ikelos
Copy link
Member

ikelos commented Feb 1, 2025

If you'd like to change it again as better, then please send a new PR. Sorry, it's 2am my time and trying to revert an old commit is a bit too much for me right now... I'm off to bed. 5:P

@gcmoreira
Copy link
Contributor

gcmoreira commented Feb 1, 2025

@ikelos The problem is that self._context.symbol_space.has_type() is not working as expected. So, it's better to go back to its original state

ikelos added a commit that referenced this pull request Feb 1, 2025
This should resolve the issue experienced #1590 was trying to resolve.
@ikelos
Copy link
Member

ikelos commented Feb 1, 2025

Ok, I manually reverted the functionality. Hopefully everything's as it should be now?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants