Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Dwarf2json can produce bitfield JSON that doesn't adhere to the schema #14

Closed
ikelos opened this issue Jun 3, 2020 · 12 comments · Fixed by #16
Closed

Dwarf2json can produce bitfield JSON that doesn't adhere to the schema #14

ikelos opened this issue Jun 3, 2020 · 12 comments · Fixed by #16

Comments

@ikelos
Copy link
Member

ikelos commented Jun 3, 2020

Hiya,

Having regenerated a bunch of profiles, including some new ones, it appears there are some bad values being produced. With jsonschema installed, trying to use one of these produces:

DEBUG    volatility.framework.automagic.mac: Identified banner: b'Darwin Kernel Version 13.2.0: Thu Apr 17 23:03:13 PDT 2014; root:xnu-2422.100.13~1/RELEASE_X86_64\x00'
DEBUG    volatility.schemas: Validating JSON against schema...
DEBUG    volatility.schemas: Schema validation error
Traceback (most recent call last):
  File "/home/personal/workspace/volatility3/volatility/schemas/__init__.py", line 68, in valid
    jsonschema.validate(input, schema)
  File "/usr/lib/python3.7/site-packages/jsonschema/validators.py", line 934, in validate
    raise error
jsonschema.exceptions.ValidationError: None is valid under each of {'$ref': '#/definitions/type_enum'}, {'$ref': '#/definitions/type_base'}

Failed validating 'oneOf' in schema[6]['properties']['type']:
    {'oneOf': [{'$ref': '#/definitions/type_base'},
               {'$ref': '#/definitions/type_enum'}]}

On instance['type']:
    None

which turns out to be because of the followin code generated from dwarf2json:

    "IOExternalAsyncMethod": {
      "size": 48,
      "fields": {
        "count0": {
          "type": {
            "kind": "base",
            "name": "long long unsigned int"
          },
          "offset": 32
        },
        "count1": {
          "type": {
            "kind": "base",
            "name": "long long unsigned int"
          },
          "offset": 40
        },
        "flags": {
          "type": {
            "kind": "base",
            "name": "unsigned int"
          },
          "offset": 24
        },
        "func": {
          "type": {
            "bit_length": 128,
            "bit_position": -128,
            "kind": "bitfield",
            "type": null
          },
          "offset": 8
        },
        "object": {
          "type": {
            "kind": "pointer",
            "subtype": {
              "kind": "class",
              "name": "IOService"
            }
          },
          "offset": 0
        }
      },
      "kind": "struct"
    },

which features a null value (which can be both enum or base). The bit_length and bit_position also look wrong. I've got the original KDK files for this that were used, and I'll attach the generated JSON. Shout if you need any more information... 5:)

@ikelos
Copy link
Member Author

ikelos commented Jun 3, 2020

@ikelos ikelos changed the title Dwarf2json can produce JSON that doesn't pass the appropriate schema Dwarf2json can produce bitfield JSON that doesn't adhere to the schema Jun 3, 2020
@ilch1
Copy link
Collaborator

ilch1 commented Jun 4, 2020

The following is the relevant dwarfdump output for this kernel:

0x00cd8c13:   DW_TAG_structure_type
                DW_AT_name      ("IOExternalAsyncMethod")
                DW_AT_byte_size (0x30)
                DW_AT_decl_file ("/SourceCache/xnu/xnu-2422.100.13/iokit/IOKit/IOUserClient.h")
                DW_AT_decl_line (83)

0x00cd8c1b:     DW_TAG_member
                  DW_AT_name    ("object")
                  DW_AT_type    (0x00ce4960 "IOService*")
                  DW_AT_decl_file       ("/SourceCache/xnu/xnu-2422.100.13/iokit/IOKit/IOUserClient.h")
                  DW_AT_decl_line       (84)
                  DW_AT_data_member_location    (DW_OP_plus_uconst 0x0)
                  DW_AT_accessibility   (DW_ACCESS_public)

0x00cd8c2a:     DW_TAG_member
                  DW_AT_name    ("func")
                  DW_AT_type    (0x00cd8c08 "IOAsyncMethod")
                  DW_AT_decl_file       ("/SourceCache/xnu/xnu-2422.100.13/iokit/IOKit/IOUserClient.h")
                  DW_AT_decl_line       (85)
                  DW_AT_byte_size       (0x00)
                  DW_AT_bit_size        (0x80)
                  DW_AT_bit_offset      (0xffffffffffffff80)
                  DW_AT_data_member_location    (DW_OP_plus_uconst 0x8)
                  DW_AT_accessibility   (DW_ACCESS_public)

From the snippet above, I can see where bit_length and bit_position values come from.

@ikelos
Copy link
Member Author

ikelos commented Jun 4, 2020

It's more the null type that we'd need to deal with... 5:S Ironically the schema might allow the negative bit position, but the type isn't allowed to be null. I don't read DWARF output well enough to know how to resolve that, but anything to improve it or handle that case would be good. 5:)

@ilch1
Copy link
Collaborator

ilch1 commented Jun 20, 2020

The reason the "type" is nil is because that dwarf type is not supported by the go dwarf library. One potential solution to handle this is to output "base void" as the type. The issue-14-handle-unsupported-types branch implements this. Let me know if that works or if you prefer a different fix.

@ikelos
Copy link
Member Author

ikelos commented Jun 21, 2020

Thanks, so that generated symbols that passed the schema validation, but the field will be completely unusable (due to the -128 bit_position field). I don't know whether it's better to include the a type for things like func at all, or whether to make the entire thing a void (rather than just the bitfield's subtype a void).

Is it valuable to know that it's a bitfield if all the other information about it is inaccurate? It probably won't matter to the user because I don't expect that type to ever get hit, but if it does I think generating a type with bad data is worse than saying the entire thing's a void. What are people's thoughts?

@ikelos
Copy link
Member Author

ikelos commented Jun 21, 2020

For reference, it looks as though we already decided how to deal with unknown types in #7. 5:)

@ilch1
Copy link
Collaborator

ilch1 commented Jun 22, 2020

I pushed another branch issue-14-omit-fields-with-unsupported-types that omits bitfields that have an unsupported type. This is probably a more consistent behavior, since non-bit fields with unsupported types were already being omitted.

Let me know what you think of this approach or feel free to suggest another one.

@ikelos
Copy link
Member Author

ikelos commented Jun 22, 2020

Hmm, I'm not seeing any change for the func type of IOExternalAsyncMethod. I think the field you're being returned is a (supposedly) valid bitfield, but the subtype is nil, so it isn't skipping on it and we still get:

        "func": {
          "type": {
            "bit_length": 128,
            "bit_position": -128,
            "kind": "bitfield",
            "type": {
              "kind": "base",
              "name": "void"
            }
          },
          "offset": 8
        },

Ideally this would be either (inaccurate but can be cast):

        "func": {
          "type": {
            "kind": "base",
            "name": "void"
          },
          "offset": 8
        },

or nothing at all (slightly less inaccurate, but only due to omission rather than correctness). I'm not sure how to differentiate bad bitfields though (I don't know which is more accurate between negative values for bit_length/position, or a bad subtype)?

@ilch1
Copy link
Collaborator

ilch1 commented Jun 22, 2020

Try the latest issue-14-omit-fields-with-unsupported-types branch.

This is the output that I'm seeing:

   "IOExternalAsyncMethod": {
      "size": 48,
      "fields": {
        "count0": {
          "type": {
            "kind": "base",
            "name": "long long unsigned int"
          },
          "offset": 32
        },
        "count1": {
          "type": {
            "kind": "base",
            "name": "long long unsigned int"
          },
          "offset": 40
        },
        "flags": {
          "type": {
            "kind": "base",
            "name": "unsigned int"
          },
          "offset": 24
        },
        "object": {
          "type": {
            "kind": "pointer",
            "subtype": {
              "kind": "class",
              "name": "IOService"
            }
          },
          "offset": 0
        }
      },
      "kind": "struct"
    },
...

@ikelos
Copy link
Member Author

ikelos commented Jun 22, 2020

Ah cool, I must've screwed up switching branches. Yep, all looks good! 5:)

@ilch1
Copy link
Collaborator

ilch1 commented Jun 22, 2020

If we are happy with this solution, I can merge it into master via #16.

@ikelos
Copy link
Member Author

ikelos commented Jun 22, 2020

Yeah, I think that's reasonable. We should keep an eye on the library, but I guess if it ever does start returning a valid type it won't get rejected anymore. Happy for #16 to be merged if you are... 5:)

@ikelos ikelos linked a pull request Jun 22, 2020 that will close this issue
@ilch1 ilch1 closed this as completed in #16 Jun 28, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants