Re: [PATCH] mfd: rave-sp: validate received frame lengths

[Lee Jones <lee@kernel.org>]

On Mon, 06 Jul 2026, Pengpeng Hou wrote:

> The RAVE SP receive path derives event, reply and checksum
> fields from variable-length frames.
> 
> Validate event and reply minimum lengths before reading their fixed
> fields, and derive the checksum payload pointer only after checking the
> frame length.
> 
> Signed-off-by: Pengpeng Hou <pengpeng@iscas.ac.cn>
> ---
>  drivers/mfd/rave-sp.c | 28 +++++++++++++++++++++-------
>  1 file changed, 21 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/mfd/rave-sp.c b/drivers/mfd/rave-sp.c
> index c1b78d127a26..0057fbfa1abf 100644
> --- a/drivers/mfd/rave-sp.c
> +++ b/drivers/mfd/rave-sp.c
> @@ -388,10 +388,15 @@ EXPORT_SYMBOL_GPL(rave_sp_exec);
>  static void rave_sp_receive_event(struct rave_sp *sp,
>  				  const unsigned char *data, size_t length)
>  {
> -	u8 cmd[] = {
> -		[0] = rave_sp_reply_code(data[0]),
> -		[1] = data[1],
> -	};
> +	u8 cmd[2];
> +
> +	if (length < 3) {

What's in data[2]?

> +		dev_warn(&sp->serdev->dev, "Dropping short event\n");
> +		return;
> +	}
> +
> +	cmd[0] = rave_sp_reply_code(data[0]);
> +	cmd[1] = data[1];

Come to think of it, what's in 0 and 1 as well?  Defines?

>  	rave_sp_write(sp, cmd, sizeof(cmd));
>  
> @@ -405,8 +410,14 @@ static void rave_sp_receive_reply(struct rave_sp *sp,
>  {
>  	struct device *dev = &sp->serdev->dev;
>  	struct rave_sp_reply *reply;
> -	const  size_t payload_length = length - 2;
> +	size_t payload_length;
>  
> +	if (length < 2) {
> +		dev_warn(dev, "Dropping short reply\n");
> +		return;
> +	}
> +
> +	payload_length = length - 2;
>  	mutex_lock(&sp->reply_lock);
>  	reply = sp->reply;
>  
> @@ -439,10 +450,10 @@ static void rave_sp_receive_frame(struct rave_sp *sp,
>  				  size_t length)
>  {
>  	const size_t checksum_length = sp->variant->checksum->length;
> -	const size_t payload_length  = length - checksum_length;
> -	const u8 *crc_reported       = &data[payload_length];
>  	struct device *dev           = &sp->serdev->dev;
>  	u8 crc_calculated[RAVE_SP_CHECKSUM_SIZE];
> +	size_t payload_length;
> +	const u8 *crc_reported;
>  
>  	if (unlikely(checksum_length > sizeof(crc_calculated))) {
>  		dev_warn(dev, "Checksum too long, dropping\n");
> -- 
> 2.43.0
> 

-- 
Lee Jones