volatilityfoundation / dwarf2json

convert ELF/DWARF symbol and type information into vol3's intermediate JSON
Other
99 stars 28 forks source link

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

Closed ikelos closed 4 years ago

ikelos commented 4 years ago

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 commented 4 years ago

kernel_debug_kit_10.9.3_13d65.dmg.zip

ilch1 commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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

ilch1 commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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

ilch1 commented 4 years ago

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

ikelos commented 4 years ago

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:)