Re: [PATCH linux dev-5.4 v2] soc: aspeed: xdma: Fix command buffer overrun
Andrew Jeffery
andrew at aj.id.au
Thu Apr 2 10:20:01 AEDT 2020
On Thu, 2 Apr 2020, at 03:01, Eddie James wrote:
> In the case of an operation requiring two commands, the edge case at
> the top of the command buffer was not handled, resulting in buffer
> overrun. Fix this by using a loop to copy the commands into the buffer
> and increment and modulo after each one.
>
> Signed-off-by: Eddie James <eajames at linux.ibm.com>
> ---
> drivers/soc/aspeed/aspeed-xdma.c | 10 +++++++---
> 1 file changed, 7 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/soc/aspeed/aspeed-xdma.c b/drivers/soc/aspeed/aspeed-xdma.c
> index 5d97919d38cf..7baeb20280b6 100644
> --- a/drivers/soc/aspeed/aspeed-xdma.c
> +++ b/drivers/soc/aspeed/aspeed-xdma.c
> @@ -430,15 +430,19 @@ static void aspeed_xdma_start(struct aspeed_xdma *ctx,
> struct aspeed_xdma_op *op, u32 bmc_addr,
> struct aspeed_xdma_client *client)
> {
> + unsigned int i;
> unsigned long flags;
> struct aspeed_xdma_cmd cmds[2];
> unsigned int rc = ctx->chip->set_cmd(ctx, cmds, op, bmc_addr);
>
> mutex_lock(&ctx->start_lock);
>
> - memcpy(&ctx->cmdq[ctx->cmd_idx], cmds,
> - rc * sizeof(struct aspeed_xdma_cmd));
> - ctx->cmd_idx = (ctx->cmd_idx + rc) % XDMA_NUM_CMDS;
> + for (i = 0; i < rc; ++i) {
> + memcpy(&ctx->cmdq[ctx->cmd_idx], &cmds[i],
> + sizeof(struct aspeed_xdma_cmd));
Bit of a nit, but it would be more obviously correct to me to do
`sizeof(ctx->cmdq[ctx->cmd_idx])`. However this wasn't something you changed as
part of this patch, so maybe it's a future enhancement.
Reviewed-by: Andrew Jeffery <andrew at aj.id.au>
> + ctx->cmd_idx = (ctx->cmd_idx + 1) % XDMA_NUM_CMDS;
> + }
> +
> ctx->upstream = !!op->direction;
>
> spin_lock_irqsave(&ctx->client_lock, flags);
> --
> 2.24.0
>
>
More information about the openbmc
mailing list