Re: [PATCH] fdc: check for illegal dma length calculation

[Jon Maloy <jmaloy@redhat.com>]

On 1/13/22 20:33, Jon Maloy wrote:
> The function fdctrl_start_transfer() calculates the dma data length
> wrongly when certain boundary conditions are fulfilled. We have
> noticed that the if ((fdctrl->fifo[5] - fdctrl->fifo[6]) > 1) we get
> a dma length that will be interpreted as negative by the next function
> in the chain, fdctrl_transfer_handler(). This leads to a crash.
>
> Rather than trying to fix this obscure calculation, we just check if
> the harmful condition is fulfilled, and return without action if that
> is the case. Since this is a condition that can only be created by a
> malicious user we deem this solution safe.
>
> This fix is intended to address CVE-2021-3507.
>
> Signed-off-by: Jon Maloy <jmaloy@redhat.com>
> ---
>   hw/block/fdc.c | 5 +++++
>   1 file changed, 5 insertions(+)
>
> diff --git a/hw/block/fdc.c b/hw/block/fdc.c
> index 21d18ac2e3..80a1f1750a 100644
> --- a/hw/block/fdc.c
> +++ b/hw/block/fdc.c
> @@ -1532,6 +1532,11 @@ static void fdctrl_start_transfer(FDCtrl *fdctrl, int direction)
>           if (fdctrl->fifo[0] & 0x80)
>               tmp += fdctrl->fifo[6];
>           fdctrl->data_len *= tmp;
> +        if (tmp < 0) {
> +            FLOPPY_DPRINTF("calculated illegal data_len=%u, tmp=%i\n",
> +                           fdctrl->data_len, tmp);
> +            return;
> +        }
>       }
>       fdctrl->eot = fdctrl->fifo[6];
>       if (fdctrl->dor & FD_DOR_DMAEN) {
I never received any feedback on this one.
Is there any?

///jon