devicetree-org / dt-schema

Devicetree schema tools
http://www.devicetree.org
BSD 2-Clause "Simplified" License
67 stars 67 forks source link

fixups: Fix improper properties #114

Closed eberman-quic closed 10 months ago

eberman-quic commented 1 year ago

An schema which passes non-strict validation could contain a properties value which isn't an object. Later fixups (like fixups_interrupts) assume that properties, if it exists, is an object. Pop the properties from the schema if it's not a dictionary to preserve the assumption.

Using the following schema patch as an example, dt-mk-schema can crash with the stack below:

Schema

diff --git a/Documentation/devicetree/bindings/arm/psci.yaml b/Documentation/devicetree/bindings/arm/psci.yaml
index 5e22d63f814e..b709b4a640bd 100644
--- a/Documentation/devicetree/bindings/arm/psci.yaml
+++ b/Documentation/devicetree/bindings/arm/psci.yaml
@@ -98,6 +98,10 @@ properties:
       [1] Kernel documentation - ARM idle states bindings
         Documentation/devicetree/bindings/cpu/idle-states.yaml

+  test:
+    properties:
+      hello
+
 patternProperties:
   "^power-domain-":
     $ref: "../power/power-domain.yaml#"

Stack:

$ make dt_binding_check DT_SCHEMA_FILES=arm/psci.yaml
  LINT    Documentation/devicetree/bindings
  CHKDT   Documentation/devicetree/bindings/processed-schema.json
/workspace/linux/Documentation/devicetree/bindings/arm/psci.yaml: properties:test:properties: 'hello' is not of type 'object'
        from schema $id: http://json-schema.org/draft-07/schema#
/workspace/linux/Documentation/devicetree/bindings/arm/psci.yaml: properties:test: 'anyOf' conditional failed, one must be fixed:
        'type' is a required property
        '$ref' is a required property
        hint: node schemas must have a type or $ref
        from schema $id: http://devicetree.org/meta-schemas/core.yaml#
  SCHEMA  Documentation/devicetree/bindings/processed-schema.json
Traceback (most recent call last):
  File "/workspace/linux/venv/bin/dt-mk-schema", line 38, in <module>
    schemas = dtschema.DTValidator(args.schemas).schemas
              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/workspace/linux/venv/lib/python3.11/site-packages/dtschema/validator.py", line 360, in __init__
    self.schemas = process_schemas(schema_files)
                   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/workspace/linux/venv/lib/python3.11/site-packages/dtschema/validator.py", line 288, in process_schemas
    _add_schema(schemas, filename)
  File "/workspace/linux/venv/lib/python3.11/site-packages/dtschema/validator.py", line 270, in _add_schema
    sch = process_schema(os.path.abspath(filename))
          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/workspace/linux/venv/lib/python3.11/site-packages/dtschema/validator.py", line 259, in process_schema
    schema = dtsch.fixup()
             ^^^^^^^^^^^^^
  File "/workspace/linux/venv/lib/python3.11/site-packages/dtschema/schema.py", line 141, in fixup
    dtschema.fixups.fixup_schema(processed_schema)
  File "/workspace/linux/venv/lib/python3.11/site-packages/dtschema/fixups.py", line 483, in fixup_schema
    fixup_sub_schema(schema)
  File "/workspace/linux/venv/lib/python3.11/site-packages/dtschema/fixups.py", line 370, in fixup_sub_schema
    fixup_sub_schema(v[prop])
  File "/workspace/linux/venv/lib/python3.11/site-packages/dtschema/fixups.py", line 341, in fixup_sub_schema
    fixup_interrupts(schema)
  File "/workspace/linux/venv/lib/python3.11/site-packages/dtschema/fixups.py", line 305, in fixup_interrupts
    if schema['properties'].keys() & {'interrupts', 'interrupt-controller'} and \
       ^^^^^^^^^^^^^^^^^^^^^^^^^
AttributeError: 'str' object has no attribute 'keys'
make[1]: *** [Documentation/devicetree/bindings/Makefile:68: Documentation/devicetree/bindings/processed-schema.json] Error 1
make[1]: *** Deleting file 'Documentation/devicetree/bindings/processed-schema.json'
robherring commented 11 months ago

We should bail out if the meta-schema fails which would catch many other errors. However, we need to allow DT meta schema errors, but not draft7 errors because we want to use existing schemas which have new DT meta schema errors.

eberman-quic commented 10 months ago

I think I got that we should treat this as an error? Could we improve the error reporting? I can try to spend some time on that front if you agree.

robherring commented 10 months ago

This should now be fixed in main.