fw_parse() reads a fixed record header, a firmware-provided payload,
and a trailing checksum without knowing the end of the firmware blob. A
truncated record can therefore make those reads exceed the blob.

The same record also supplies addresses and lengths for writes into
DPRAM. The generic loader uses wrap-prone mixed signed arithmetic for its
bounds check, while the application loader does not bound the staging
copy at all.

Pass the firmware end to the parser and validate the full source record.
Use a signed wide offset for generic DPRAM records and validate the
application staging span against the mapped DPRAM before copying.

Fixes: 03fd3cf5a179 ("can: add driver for Softing card")
Signed-off-by: Pengpeng Hou <pengpeng@iscas.ac.cn>
---
 drivers/net/can/softing/softing_fw.c | 46 +++++++++++++++++++---------
 1 file changed, 32 insertions(+), 14 deletions(-)

diff --git a/drivers/net/can/softing/softing_fw.c b/drivers/net/can/softing/softing_fw.c
index 721df91cdbfb..282570daf3ef 100644
--- a/drivers/net/can/softing/softing_fw.c
+++ b/drivers/net/can/softing/softing_fw.c
@@ -91,12 +91,12 @@ int softing_bootloader_command(struct softing *card, int16_t cmd,
 	return ret;
 }
 
-static int fw_parse(const uint8_t **pmem, uint16_t *ptype, uint32_t *paddr,
-		uint16_t *plen, const uint8_t **pdat)
+static int fw_parse(const u8 **pmem, const u8 *limit, u16 *ptype,
+		    u32 *paddr, u16 *plen, const u8 **pdat)
 {
 	uint16_t checksum[2];
-	const uint8_t *mem;
-	const uint8_t *end;
+	const u8 *mem;
+	const u8 *record_end;
 
 	/*
 	 * firmware records are a binary, unaligned stream composed of:
@@ -114,14 +114,21 @@ static int fw_parse(const uint8_t **pmem, uint16_t *ptype, uint32_t *paddr,
 	 * endianness & alignment.
 	 */
 	mem = *pmem;
+	/* A record needs an 8-byte prefix and a 2-byte checksum. */
+	if (mem > limit || limit - mem < 10)
+		return -EINVAL;
+
 	*ptype = le16_to_cpup((void *)&mem[0]);
 	*paddr = le32_to_cpup((void *)&mem[2]);
 	*plen = le16_to_cpup((void *)&mem[6]);
+	if (*plen > limit - mem - 10)
+		return -EINVAL;
+
 	*pdat = &mem[8];
 	/* verify checksum */
-	end = &mem[8 + *plen];
-	checksum[0] = le16_to_cpup((void *)end);
-	for (checksum[1] = 0; mem < end; ++mem)
+	record_end = &mem[8 + *plen];
+	checksum[0] = le16_to_cpup((void *)record_end);
+	for (checksum[1] = 0; mem < record_end; ++mem)
 		checksum[1] += *mem;
 	if (checksum[0] != checksum[1])
 		return -EINVAL;
@@ -139,6 +146,7 @@ int softing_load_fw(const char *file, struct softing *card,
 	uint16_t type, len;
 	uint32_t addr;
 	uint8_t *buf = NULL, *new_buf;
+	s64 dpram_offset;
 	int buflen = 0;
 	int8_t type_end = 0;
 
@@ -153,7 +161,7 @@ int softing_load_fw(const char *file, struct softing *card,
 	mem = fw->data;
 	end = &mem[fw->size];
 	/* look for header record */
-	ret = fw_parse(&mem, &type, &addr, &len, &dat);
+	ret = fw_parse(&mem, end, &type, &addr, &len, &dat);
 	if (ret < 0)
 		goto failed;
 	if (type != 0xffff)
@@ -164,7 +172,7 @@ int softing_load_fw(const char *file, struct softing *card,
 	}
 	/* ok, we had a header */
 	while (mem < end) {
-		ret = fw_parse(&mem, &type, &addr, &len, &dat);
+		ret = fw_parse(&mem, end, &type, &addr, &len, &dat);
 		if (ret < 0)
 			goto failed;
 		if (type == 3) {
@@ -179,9 +187,13 @@ int softing_load_fw(const char *file, struct softing *card,
 			goto failed;
 		}
 
-		if ((addr + len + offset) > size)
+		dpram_offset = (s64)addr + offset;
+		if (dpram_offset < 0 || dpram_offset > size ||
+		    len > size - dpram_offset) {
+			ret = -EINVAL;
 			goto failed;
-		memcpy_toio(&dpram[addr + offset], dat, len);
+		}
+		memcpy_toio(&dpram[dpram_offset], dat, len);
 		/* be sure to flush caches from IO space */
 		mb();
 		if (len > buflen) {
@@ -195,7 +207,7 @@ int softing_load_fw(const char *file, struct softing *card,
 			buf = new_buf;
 		}
 		/* verify record data */
-		memcpy_fromio(buf, &dpram[addr + offset], len);
+		memcpy_fromio(buf, &dpram[dpram_offset], len);
 		if (memcmp(buf, dat, len)) {
 			/* is not ok */
 			dev_alert(&card->pdev->dev, "DPRAM readback failed\n");
@@ -237,7 +249,7 @@ int softing_load_app_fw(const char *file, struct softing *card)
 	mem = fw->data;
 	end = &mem[fw->size];
 	/* look for header record */
-	ret = fw_parse(&mem, &type, &addr, &len, &dat);
+	ret = fw_parse(&mem, end, &type, &addr, &len, &dat);
 	if (ret)
 		goto failed;
 	ret = -EINVAL;
@@ -253,7 +265,7 @@ int softing_load_app_fw(const char *file, struct softing *card)
 	}
 	/* ok, we had a header */
 	while (mem < end) {
-		ret = fw_parse(&mem, &type, &addr, &len, &dat);
+		ret = fw_parse(&mem, end, &type, &addr, &len, &dat);
 		if (ret)
 			goto failed;
 
@@ -279,6 +291,12 @@ int softing_load_app_fw(const char *file, struct softing *card)
 		/* work in 16bit (target) */
 		sum &= 0xffff;
 
+		if (card->pdat->app.offs > card->dpram_size ||
+		    len > card->dpram_size - card->pdat->app.offs) {
+			ret = -EINVAL;
+			goto failed;
+		}
+
 		memcpy_toio(&card->dpram[card->pdat->app.offs], dat, len);
 		iowrite32(card->pdat->app.offs + card->pdat->app.addr,
 				&card->dpram[DPRAM_COMMAND + 2]);
-- 
2.50.1 (Apple Git-155)

Hi all,

thanks for the patch!

In fact I was thinking about entirely removing this driver.

I am not aware if anyone still uses this over 30 y/o hardware in 2026.
Since I changed my department in 2017 my last Softing CANCard was 
accidentally thrown away at a clean up attempt.

IMO neither this PCMCIA Card from 1994 nor some Linux hardware with 
PCMCIA slot would still run Linux 7.2 today where a 13€ CANdelight USB 
adapter would do a better job.

So thanks but I tend to remove the softing driver instead of creating 
the effort for a stable patch series.

@Marc/Vincent: I still have two SJA1000 memory-mapped (no firmware!) 
PCMCIA CAN cards from PEAK and EMS Wuensche on my desk - and a Centrino 
Laptop from 2006 with PCMCIA slot. But I wonder if we should remove the 
entire PCMCIA CAN drivers while we are at it?

The last fix that I reviewed and tested on my Laptop was from 2021:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit?id=3ec6ca6b1a8e64389f0212b5a1b0f6fed1909e45

Best regards,
Oliver


On 22.07.26 06:43, Pengpeng Hou wrote:
> fw_parse() reads a fixed record header, a firmware-provided payload,
> and a trailing checksum without knowing the end of the firmware blob. A
> truncated record can therefore make those reads exceed the blob.
> 
> The same record also supplies addresses and lengths for writes into
> DPRAM. The generic loader uses wrap-prone mixed signed arithmetic for its
> bounds check, while the application loader does not bound the staging
> copy at all.
> 
> Pass the firmware end to the parser and validate the full source record.
> Use a signed wide offset for generic DPRAM records and validate the
> application staging span against the mapped DPRAM before copying.
> 
> Fixes: 03fd3cf5a179 ("can: add driver for Softing card")
> Signed-off-by: Pengpeng Hou <pengpeng@iscas.ac.cn>
> ---
>   drivers/net/can/softing/softing_fw.c | 46 +++++++++++++++++++---------
>   1 file changed, 32 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/net/can/softing/softing_fw.c b/drivers/net/can/softing/softing_fw.c
> index 721df91cdbfb..282570daf3ef 100644
> --- a/drivers/net/can/softing/softing_fw.c
> +++ b/drivers/net/can/softing/softing_fw.c
> @@ -91,12 +91,12 @@ int softing_bootloader_command(struct softing *card, int16_t cmd,
>   	return ret;
>   }
>   
> -static int fw_parse(const uint8_t **pmem, uint16_t *ptype, uint32_t *paddr,
> -		uint16_t *plen, const uint8_t **pdat)
> +static int fw_parse(const u8 **pmem, const u8 *limit, u16 *ptype,
> +		    u32 *paddr, u16 *plen, const u8 **pdat)
>   {
>   	uint16_t checksum[2];
> -	const uint8_t *mem;
> -	const uint8_t *end;
> +	const u8 *mem;
> +	const u8 *record_end;
>   
>   	/*
>   	 * firmware records are a binary, unaligned stream composed of:
> @@ -114,14 +114,21 @@ static int fw_parse(const uint8_t **pmem, uint16_t *ptype, uint32_t *paddr,
>   	 * endianness & alignment.
>   	 */
>   	mem = *pmem;
> +	/* A record needs an 8-byte prefix and a 2-byte checksum. */
> +	if (mem > limit || limit - mem < 10)
> +		return -EINVAL;
> +
>   	*ptype = le16_to_cpup((void *)&mem[0]);
>   	*paddr = le32_to_cpup((void *)&mem[2]);
>   	*plen = le16_to_cpup((void *)&mem[6]);
> +	if (*plen > limit - mem - 10)
> +		return -EINVAL;
> +
>   	*pdat = &mem[8];
>   	/* verify checksum */
> -	end = &mem[8 + *plen];
> -	checksum[0] = le16_to_cpup((void *)end);
> -	for (checksum[1] = 0; mem < end; ++mem)
> +	record_end = &mem[8 + *plen];
> +	checksum[0] = le16_to_cpup((void *)record_end);
> +	for (checksum[1] = 0; mem < record_end; ++mem)
>   		checksum[1] += *mem;
>   	if (checksum[0] != checksum[1])
>   		return -EINVAL;
> @@ -139,6 +146,7 @@ int softing_load_fw(const char *file, struct softing *card,
>   	uint16_t type, len;
>   	uint32_t addr;
>   	uint8_t *buf = NULL, *new_buf;
> +	s64 dpram_offset;
>   	int buflen = 0;
>   	int8_t type_end = 0;
>   
> @@ -153,7 +161,7 @@ int softing_load_fw(const char *file, struct softing *card,
>   	mem = fw->data;
>   	end = &mem[fw->size];
>   	/* look for header record */
> -	ret = fw_parse(&mem, &type, &addr, &len, &dat);
> +	ret = fw_parse(&mem, end, &type, &addr, &len, &dat);
>   	if (ret < 0)
>   		goto failed;
>   	if (type != 0xffff)
> @@ -164,7 +172,7 @@ int softing_load_fw(const char *file, struct softing *card,
>   	}
>   	/* ok, we had a header */
>   	while (mem < end) {
> -		ret = fw_parse(&mem, &type, &addr, &len, &dat);
> +		ret = fw_parse(&mem, end, &type, &addr, &len, &dat);
>   		if (ret < 0)
>   			goto failed;
>   		if (type == 3) {
> @@ -179,9 +187,13 @@ int softing_load_fw(const char *file, struct softing *card,
>   			goto failed;
>   		}
>   
> -		if ((addr + len + offset) > size)
> +		dpram_offset = (s64)addr + offset;
> +		if (dpram_offset < 0 || dpram_offset > size ||
> +		    len > size - dpram_offset) {
> +			ret = -EINVAL;
>   			goto failed;
> -		memcpy_toio(&dpram[addr + offset], dat, len);
> +		}
> +		memcpy_toio(&dpram[dpram_offset], dat, len);
>   		/* be sure to flush caches from IO space */
>   		mb();
>   		if (len > buflen) {
> @@ -195,7 +207,7 @@ int softing_load_fw(const char *file, struct softing *card,
>   			buf = new_buf;
>   		}
>   		/* verify record data */
> -		memcpy_fromio(buf, &dpram[addr + offset], len);
> +		memcpy_fromio(buf, &dpram[dpram_offset], len);
>   		if (memcmp(buf, dat, len)) {
>   			/* is not ok */
>   			dev_alert(&card->pdev->dev, "DPRAM readback failed\n");
> @@ -237,7 +249,7 @@ int softing_load_app_fw(const char *file, struct softing *card)
>   	mem = fw->data;
>   	end = &mem[fw->size];
>   	/* look for header record */
> -	ret = fw_parse(&mem, &type, &addr, &len, &dat);
> +	ret = fw_parse(&mem, end, &type, &addr, &len, &dat);
>   	if (ret)
>   		goto failed;
>   	ret = -EINVAL;
> @@ -253,7 +265,7 @@ int softing_load_app_fw(const char *file, struct softing *card)
>   	}
>   	/* ok, we had a header */
>   	while (mem < end) {
> -		ret = fw_parse(&mem, &type, &addr, &len, &dat);
> +		ret = fw_parse(&mem, end, &type, &addr, &len, &dat);
>   		if (ret)
>   			goto failed;
>   
> @@ -279,6 +291,12 @@ int softing_load_app_fw(const char *file, struct softing *card)
>   		/* work in 16bit (target) */
>   		sum &= 0xffff;
>   
> +		if (card->pdat->app.offs > card->dpram_size ||
> +		    len > card->dpram_size - card->pdat->app.offs) {
> +			ret = -EINVAL;
> +			goto failed;
> +		}
> +
>   		memcpy_toio(&card->dpram[card->pdat->app.offs], dat, len);
>   		iowrite32(card->pdat->app.offs + card->pdat->app.addr,
>   				&card->dpram[DPRAM_COMMAND + 2]);