RE: [PATCH v2] misc: xilinx_sdfec: validate LDPC code register offsets

[Cvetic, Dragan]

AMD General

The patch looks good to me.
Reviewed-by: Cvetic, Dragan <dragan.cvetic@xxxxxxx>


> -----Original Message-----
> From: Yousef Alhouseen <alhouseenyousef@xxxxxxxxx>
> Sent: Monday 29 June 2026 16:29
> To: Kiernan, Derek <derek.kiernan@xxxxxxx>; Cvetic, Dragan
> <dragan.cvetic@xxxxxxx>
> Cc: Arnd Bergmann <arnd@xxxxxxxx>; Greg Kroah-Hartman
> <gregkh@xxxxxxxxxxxxxxxxxxx>; Simek, Michal <michal.simek@xxxxxxx>;
> linux-arm-kernel@xxxxxxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; Yousef
> Alhouseen <alhouseenyousef@xxxxxxxxx>
> Subject: [PATCH v2] misc: xilinx_sdfec: validate LDPC code register offsets
>
> The LDPC code register helpers check the target MMIO address after
> adding code_id * XSDFEC_LDPC_REG_JUMP to the register base. code_id is
> supplied through the ioctl path, so the multiplication and addition can
> wrap before the bounds check.
>
> Validate the code_id against the register window size before computing
> the final address, then write using the checked address.
>
> Signed-off-by: Yousef Alhouseen <alhouseenyousef@xxxxxxxxx>
> ---
> Changes in v2:
> - Guard against swapped high/base constants before subtracting.
> - Preserve debug output and include offset/base/high in the message.
>
>  drivers/misc/xilinx_sdfec.c | 74 +++++++++++++++++--------------------
>  1 file changed, 34 insertions(+), 40 deletions(-)
>
> diff --git a/drivers/misc/xilinx_sdfec.c b/drivers/misc/xilinx_sdfec.c
> index 3135ba3a58ee..e73a139f3ffc 100644
> --- a/drivers/misc/xilinx_sdfec.c
> +++ b/drivers/misc/xilinx_sdfec.c
> @@ -456,10 +456,25 @@ static int xsdfec_get_turbo(struct xsdfec_dev
> *xsdfec, void __user *arg)
>       return err;
>  }
>
> +static int xsdfec_ldpc_reg_addr(struct xsdfec_dev *xsdfec, u32 base, u32
> high,
> +                             u32 offset, u32 *addr)
> +{
> +     if (high < base || offset > (high - base) / XSDFEC_LDPC_REG_JUMP) {
> +             dev_dbg(xsdfec->dev,
> +                     "LDPC register offset %u outside space 0x%x-0x%x",
> +                     offset, base, high);
> +             return -EINVAL;
> +     }
> +
> +     *addr = base + offset * XSDFEC_LDPC_REG_JUMP;
> +     return 0;
> +}
> +
>  static int xsdfec_reg0_write(struct xsdfec_dev *xsdfec, u32 n, u32 k, u32
> psize,
>                            u32 offset)
>  {
>       u32 wdata;
> +     u32 addr;
>
>       if (n < XSDFEC_REG0_N_MIN || n > XSDFEC_REG0_N_MAX || psize ==
> 0 ||
>           (n > XSDFEC_REG0_N_MUL_P * psize) || n <= k || ((n % psize) != 0))
> {
> @@ -476,17 +491,11 @@ static int xsdfec_reg0_write(struct xsdfec_dev
> *xsdfec, u32 n, u32 k, u32 psize,
>       k = k << XSDFEC_REG0_K_LSB;
>       wdata = k | n;
>
> -     if (XSDFEC_LDPC_CODE_REG0_ADDR_BASE + (offset *
> XSDFEC_LDPC_REG_JUMP) >
> -         XSDFEC_LDPC_CODE_REG0_ADDR_HIGH) {
> -             dev_dbg(xsdfec->dev, "Writing outside of LDPC reg0 space
> 0x%x",
> -                     XSDFEC_LDPC_CODE_REG0_ADDR_BASE +
> -                             (offset * XSDFEC_LDPC_REG_JUMP));
> +     if (xsdfec_ldpc_reg_addr(xsdfec,
> XSDFEC_LDPC_CODE_REG0_ADDR_BASE,
> +                              XSDFEC_LDPC_CODE_REG0_ADDR_HIGH,
> offset,
> +                              &addr))
>               return -EINVAL;
> -     }
> -     xsdfec_regwrite(xsdfec,
> -                     XSDFEC_LDPC_CODE_REG0_ADDR_BASE +
> -                             (offset * XSDFEC_LDPC_REG_JUMP),
> -                     wdata);
> +     xsdfec_regwrite(xsdfec, addr, wdata);
>       return 0;
>  }
>
> @@ -494,6 +503,7 @@ static int xsdfec_reg1_write(struct xsdfec_dev
> *xsdfec, u32 psize,
>                            u32 no_packing, u32 nm, u32 offset)
>  {
>       u32 wdata;
> +     u32 addr;
>
>       if (psize < XSDFEC_REG1_PSIZE_MIN || psize >
> XSDFEC_REG1_PSIZE_MAX) {
>               dev_dbg(xsdfec->dev, "Psize is not in range");
> @@ -510,17 +520,11 @@ static int xsdfec_reg1_write(struct xsdfec_dev
> *xsdfec, u32 psize,
>       nm = (nm << XSDFEC_REG1_NM_LSB) & XSDFEC_REG1_NM_MASK;
>
>       wdata = nm | no_packing | psize;
> -     if (XSDFEC_LDPC_CODE_REG1_ADDR_BASE + (offset *
> XSDFEC_LDPC_REG_JUMP) >
> -         XSDFEC_LDPC_CODE_REG1_ADDR_HIGH) {
> -             dev_dbg(xsdfec->dev, "Writing outside of LDPC reg1 space
> 0x%x",
> -                     XSDFEC_LDPC_CODE_REG1_ADDR_BASE +
> -                             (offset * XSDFEC_LDPC_REG_JUMP));
> +     if (xsdfec_ldpc_reg_addr(xsdfec,
> XSDFEC_LDPC_CODE_REG1_ADDR_BASE,
> +                              XSDFEC_LDPC_CODE_REG1_ADDR_HIGH,
> offset,
> +                              &addr))
>               return -EINVAL;
> -     }
> -     xsdfec_regwrite(xsdfec,
> -                     XSDFEC_LDPC_CODE_REG1_ADDR_BASE +
> -                             (offset * XSDFEC_LDPC_REG_JUMP),
> -                     wdata);
> +     xsdfec_regwrite(xsdfec, addr, wdata);
>       return 0;
>  }
>
> @@ -529,6 +533,7 @@ static int xsdfec_reg2_write(struct xsdfec_dev
> *xsdfec, u32 nlayers, u32 nmqc,
>                            u32 max_schedule, u32 offset)
>  {
>       u32 wdata;
> +     u32 addr;
>
>       if (nlayers < XSDFEC_REG2_NLAYERS_MIN ||
>           nlayers > XSDFEC_REG2_NLAYERS_MAX) {
> @@ -563,17 +568,11 @@ static int xsdfec_reg2_write(struct xsdfec_dev
> *xsdfec, u32 nlayers, u32 nmqc,
>       wdata = (max_schedule | no_final_parity | special_qc | norm_type |
>                nmqc | nlayers);
>
> -     if (XSDFEC_LDPC_CODE_REG2_ADDR_BASE + (offset *
> XSDFEC_LDPC_REG_JUMP) >
> -         XSDFEC_LDPC_CODE_REG2_ADDR_HIGH) {
> -             dev_dbg(xsdfec->dev, "Writing outside of LDPC reg2 space
> 0x%x",
> -                     XSDFEC_LDPC_CODE_REG2_ADDR_BASE +
> -                             (offset * XSDFEC_LDPC_REG_JUMP));
> +     if (xsdfec_ldpc_reg_addr(xsdfec,
> XSDFEC_LDPC_CODE_REG2_ADDR_BASE,
> +                              XSDFEC_LDPC_CODE_REG2_ADDR_HIGH,
> offset,
> +                              &addr))
>               return -EINVAL;
> -     }
> -     xsdfec_regwrite(xsdfec,
> -                     XSDFEC_LDPC_CODE_REG2_ADDR_BASE +
> -                             (offset * XSDFEC_LDPC_REG_JUMP),
> -                     wdata);
> +     xsdfec_regwrite(xsdfec, addr, wdata);
>       return 0;
>  }
>
> @@ -581,20 +580,15 @@ static int xsdfec_reg3_write(struct xsdfec_dev
> *xsdfec, u8 sc_off, u8 la_off,
>                            u16 qc_off, u32 offset)
>  {
>       u32 wdata;
> +     u32 addr;
>
>       wdata = ((qc_off << XSDFEC_REG3_QC_OFF_LSB) |
>                (la_off << XSDFEC_REG3_LA_OFF_LSB) | sc_off);
> -     if (XSDFEC_LDPC_CODE_REG3_ADDR_BASE + (offset *
> XSDFEC_LDPC_REG_JUMP) >
> -         XSDFEC_LDPC_CODE_REG3_ADDR_HIGH) {
> -             dev_dbg(xsdfec->dev, "Writing outside of LDPC reg3 space
> 0x%x",
> -                     XSDFEC_LDPC_CODE_REG3_ADDR_BASE +
> -                             (offset * XSDFEC_LDPC_REG_JUMP));
> +     if (xsdfec_ldpc_reg_addr(xsdfec,
> XSDFEC_LDPC_CODE_REG3_ADDR_BASE,
> +                              XSDFEC_LDPC_CODE_REG3_ADDR_HIGH,
> offset,
> +                              &addr))
>               return -EINVAL;
> -     }
> -     xsdfec_regwrite(xsdfec,
> -                     XSDFEC_LDPC_CODE_REG3_ADDR_BASE +
> -                             (offset * XSDFEC_LDPC_REG_JUMP),
> -                     wdata);
> +     xsdfec_regwrite(xsdfec, addr, wdata);
>       return 0;
>  }
>
> --
> 2.54.0