Re: [PATCH v2] dt-bindings: misc: add binding for Xilinx AXI-Stream FIFO

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:

New issues:
- [Low] Missing `$ref` type definition for custom string properties in DT binding.

Pre-existing issues:
- [Critical] Use-After-Free on device unbind due to lack of synchronization with open file descriptors.
- [High] Integer underflow in bounds check allows bypassing transmit packet size restriction.
--

commit 97e86b437f0cf66fc60eec47c4742a54db8053f8
Author: Aditya Chari <adi25charis@gmail.com>

dt-bindings: misc: add binding for Xilinx AXI-Stream FIFO

The axis-fifo driver's compatible strings were undocumented, flagged
by checkpatch.pl as UNDOCUMENTED_DT_STRING. Add a YAML devicetree
binding document for drivers/staging/axis-fifo, converted from and
replacing the existing free-form text binding (axis-fifo.txt), which
this patch removes.

> diff --git a/Documentation/devicetree/bindings/misc/xlnx,axi-fifo-mm-s.yaml b/Documentation/devicetree/bindings/misc/xlnx,axi-fifo-mm-s.yaml
> new file mode 100644
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/misc/xlnx,axi-fifo-mm-s.yaml
> @@ -0,0 +1,221 @@
[ ... ]
> +  xlnx,axi-str-rxd-protocol:
> +    description: AXI-Stream receive data protocol. Ignored by the driver.
> +    enum: [ XIL_AXI_STREAM_ETH_DATA ]
> +
> +  xlnx,axi-str-txd-protocol:
> +    description: AXI-Stream transmit data protocol. Ignored by the driver.
> +    enum: [ XIL_AXI_STREAM_ETH_DATA ]
> +
> +  xlnx,axi-str-txc-protocol:
> +    description: AXI-Stream transmit control protocol. Ignored by the driver.
> +    enum: [ XIL_AXI_STREAM_ETH_CTRL ]

[Severity: Low]
Do these custom string properties need a $ref to types.yaml?

Device tree schema validation (dt_binding_check) requires custom properties
to explicitly specify their type. Providing an enum of strings might not be
sufficient for dt-schema to infer the type, which could cause type missing
or unknown schema validation errors.

Should these properties include a $ref: /schemas/types.yaml#/definitions/string
constraint?

[ ... ]
> +  xlnx,tx-fifo-depth:
> +    description:
> +      Depth in 32-bit words of the transmit FIFO, as configured in the
> +      IP core.
> +    $ref: /schemas/types.yaml#/definitions/uint32

[Severity: High]
This isn't a bug introduced by this patch, but should we add a minimum: 4
constraint here to protect against an integer underflow in the driver?

In drivers/staging/axis-fifo/axis-fifo.c:axis_fifo_write(), the bounds check
assumes tx_fifo_depth is at least 4:

    if (!words_to_write || (len % sizeof(u32)) ||
        (words_to_write > (fifo->tx_fifo_depth - 4)))
        return -EINVAL;

If the device tree configures xlnx,tx-fifo-depth to a value less than 4, this
subtraction underflows to a massive unsigned integer, bypassing the bounds
check. This could allow an oversized packet to trigger a Transmit Packet
Overrun Error in the hardware.

[ ... ]

[Severity: Critical]
This is a pre-existing issue in the driver documented by this binding, but is
there a use-after-free vulnerability during device unbind?

The driver allocates its core state using devm_kzalloc():

drivers/staging/axis-fifo/axis-fifo.c:axis_fifo_probe() {
    ...
    fifo = devm_kzalloc(dev, sizeof(*fifo), GFP_KERNEL);
    ...
}

During removal, it deregisters the misc device but doesn't wait for active
VFS file references to be dropped:

drivers/staging/axis-fifo/axis-fifo.c:axis_fifo_remove() {
    ...
    misc_deregister(&fifo->miscdev);
    ...
}

When axis_fifo_remove() returns, the devres layer automatically frees the
axis_fifo structure. If a user process holds an open file descriptor and
subsequently calls write() or read(), it will dereference the freed pointer:

drivers/staging/axis-fifo/axis-fifo.c:axis_fifo_write() {
    struct axis_fifo *fifo = f->private_data;
    ...
}

Should the driver use a different allocation and reference counting strategy
to ensure the memory remains valid as long as the file descriptor is open?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260621091953.53281-1-adi25charis@gmail.com?part=1