All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] mfd: rave-sp: validate received frame lengths
@ 2026-07-06  9:23 Pengpeng Hou
  2026-07-16 12:24 ` Lee Jones
  0 siblings, 1 reply; 2+ messages in thread
From: Pengpeng Hou @ 2026-07-06  9:23 UTC (permalink / raw)
  To: Lee Jones; +Cc: Pengpeng Hou, mfd, linux-kernel

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) {
+		dev_warn(&sp->serdev->dev, "Dropping short event\n");
+		return;
+	}
+
+	cmd[0] = rave_sp_reply_code(data[0]);
+	cmd[1] = data[1];
 
 	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");
@@ -457,6 +468,9 @@ static void rave_sp_receive_frame(struct rave_sp *sp,
 		return;
 	}
 
+	payload_length = length - checksum_length;
+	crc_reported = &data[payload_length];
+
 	sp->variant->checksum->subroutine(data, payload_length,
 					  crc_calculated);
 
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: [PATCH] mfd: rave-sp: validate received frame lengths
  2026-07-06  9:23 [PATCH] mfd: rave-sp: validate received frame lengths Pengpeng Hou
@ 2026-07-16 12:24 ` Lee Jones
  0 siblings, 0 replies; 2+ messages in thread
From: Lee Jones @ 2026-07-16 12:24 UTC (permalink / raw)
  To: Pengpeng Hou; +Cc: mfd, linux-kernel

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

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2026-07-16 12:24 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-06  9:23 [PATCH] mfd: rave-sp: validate received frame lengths Pengpeng Hou
2026-07-16 12:24 ` Lee Jones

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.