[PATCH] p54: validate firmware descriptor RX range

[PATCH] p54: validate firmware descriptor RX range

[Heyang Tan]

The BR_CODE_DESCR parser reads fields through rx_keycache_size without
checking that the record contains those fields. The record length is
measured in 32-bit words, so the fields accessed by the parser require at
least five words.

It also subtracts the RX offset from rx_end without checking the result.
Reject descriptors that are too short, underflow the offset, or produce an
empty RX range before updating the driver state.

Signed-off-by: Heyang Tan <thy15333007817@163.com>
---
 drivers/net/wireless/intersil/p54/fwio.c | 23 ++++++++++++++++++++---
 1 file changed, 20 insertions(+), 3 deletions(-)

diff --git a/drivers/net/wireless/intersil/p54/fwio.c b/drivers/net/wireless/intersil/p54/fwio.c
index a3d9053f0..5d82e0a43 100644
--- a/drivers/net/wireless/intersil/p54/fwio.c
+++ b/drivers/net/wireless/intersil/p54/fwio.c
@@ -79,9 +79,26 @@ int p54_parse_firmware(struct ieee80211_hw *dev, const struct firmware *fw)
 		case BR_CODE_DESCR: {
 			struct bootrec_desc *desc =
 				(struct bootrec_desc *)bootrec->data;
-			priv->rx_start = le32_to_cpu(desc->rx_start);
-			/* FIXME add sanity checking */
-			priv->rx_end = le32_to_cpu(desc->rx_end) - 0x3500;
+			u32 rx_start, rx_end;
+
+			if (len < DIV_ROUND_UP(offsetofend(struct bootrec_desc,
+							   rx_keycache_size),
+						       sizeof(*bootrec->data))) {
+				wiphy_err(priv->hw->wiphy,
+					  "firmware descriptor is too short\n");
+				return -EINVAL;
+			}
+
+			rx_start = le32_to_cpu(desc->rx_start);
+			rx_end = le32_to_cpu(desc->rx_end);
+			if (rx_end < 0x3500 || rx_end - 0x3500 <= rx_start) {
+				wiphy_err(priv->hw->wiphy,
+					  "firmware descriptor has invalid RX range\n");
+				return -EINVAL;
+			}
+
+			priv->rx_start = rx_start;
+			priv->rx_end = rx_end - 0x3500;
 			priv->headroom = desc->headroom;
 			priv->tailroom = desc->tailroom;
 			priv->privacy_caps = desc->privacy_caps;
-- 
2.34.1

[PATCH] p54: validate firmware descriptor RX range

[Heyang Tan]

The BR_CODE_DESCR parser reads fields through rx_keycache_size without
checking that the record contains those fields. The record length is
measured in 32-bit words, so the fields accessed by the parser require at
least five words.

It also subtracts the RX offset from rx_end without checking the result.
Reject descriptors that are too short, underflow the offset, or produce an
empty RX range before updating the driver state.

Signed-off-by: Heyang Tan <thy15333007817@163.com>
---
 drivers/net/wireless/intersil/p54/fwio.c | 23 ++++++++++++++++++++---
 1 file changed, 20 insertions(+), 3 deletions(-)

diff --git a/drivers/net/wireless/intersil/p54/fwio.c b/drivers/net/wireless/intersil/p54/fwio.c
index a3d9053f0..5d82e0a43 100644
--- a/drivers/net/wireless/intersil/p54/fwio.c
+++ b/drivers/net/wireless/intersil/p54/fwio.c
@@ -79,9 +79,26 @@ int p54_parse_firmware(struct ieee80211_hw *dev, const struct firmware *fw)
 		case BR_CODE_DESCR: {
 			struct bootrec_desc *desc =
 				(struct bootrec_desc *)bootrec->data;
-			priv->rx_start = le32_to_cpu(desc->rx_start);
-			/* FIXME add sanity checking */
-			priv->rx_end = le32_to_cpu(desc->rx_end) - 0x3500;
+			u32 rx_start, rx_end;
+
+			if (len < DIV_ROUND_UP(offsetofend(struct bootrec_desc,
+							   rx_keycache_size),
+						       sizeof(*bootrec->data))) {
+				wiphy_err(priv->hw->wiphy,
+					  "firmware descriptor is too short\n");
+				return -EINVAL;
+			}
+
+			rx_start = le32_to_cpu(desc->rx_start);
+			rx_end = le32_to_cpu(desc->rx_end);
+			if (rx_end < 0x3500 || rx_end - 0x3500 <= rx_start) {
+				wiphy_err(priv->hw->wiphy,
+					  "firmware descriptor has invalid RX range\n");
+				return -EINVAL;
+			}
+
+			priv->rx_start = rx_start;
+			priv->rx_end = rx_end - 0x3500;
 			priv->headroom = desc->headroom;
 			priv->tailroom = desc->tailroom;
 			priv->privacy_caps = desc->privacy_caps;
-- 
2.34.1

Re: [PATCH] p54: validate firmware descriptor RX range

[Christian Lamparter]

On 9/6/26 3:34 PM, Heyang Tan wrote:
> The BR_CODE_DESCR parser reads fields through rx_keycache_size without
> checking that the record contains those fields. The record length is
> measured in 32-bit words, so the fields accessed by the parser require at
> least five words.
> 
> It also subtracts the RX offset from rx_end without checking the result.
> Reject descriptors that are too short, underflow the offset, or produce an
> empty RX range before updating the driver state.
> 
> Signed-off-by: Heyang Tan <thy15333007817@163.com>

Sure. Can you please add, if this is/was assisted by an AI?
https://docs.kernel.org/process/coding-assistants.html

Please give me some time (like a week). I can this run on real hardware
with real firmware. I just need to dig up a PC which still has pci.

> ---
>   drivers/net/wireless/intersil/p54/fwio.c | 23 ++++++++++++++++++++---
>   1 file changed, 20 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/wireless/intersil/p54/fwio.c b/drivers/net/wireless/intersil/p54/fwio.c
> index a3d9053f0..5d82e0a43 100644
> --- a/drivers/net/wireless/intersil/p54/fwio.c
> +++ b/drivers/net/wireless/intersil/p54/fwio.c
> @@ -79,9 +79,26 @@ int p54_parse_firmware(struct ieee80211_hw *dev, const struct firmware *fw)
>   		case BR_CODE_DESCR: {
>   			struct bootrec_desc *desc =
>   				(struct bootrec_desc *)bootrec->data;
> -			priv->rx_start = le32_to_cpu(desc->rx_start);
> -			/* FIXME add sanity checking */
> -			priv->rx_end = le32_to_cpu(desc->rx_end) - 0x3500;
> +			u32 rx_start, rx_end;
> +
> +			if (len < DIV_ROUND_UP(offsetofend(struct bootrec_desc,
> +							   rx_keycache_size),
> +						       sizeof(*bootrec->data))) {
> +				wiphy_err(priv->hw->wiphy,
> +					  "firmware descriptor is too short\n");
> +				return -EINVAL;
> +			}
> +
> +			rx_start = le32_to_cpu(desc->rx_start);
> +			rx_end = le32_to_cpu(desc->rx_end);
> +			if (rx_end < 0x3500 || rx_end - 0x3500 <= rx_start) {
> +				wiphy_err(priv->hw->wiphy,
> +					  "firmware descriptor has invalid RX range\n");
> +				return -EINVAL;
> +			}
> +
> +			priv->rx_start = rx_start;
> +			priv->rx_end = rx_end - 0x3500;
>   			priv->headroom = desc->headroom;
>   			priv->tailroom = desc->tailroom;
>   			priv->privacy_caps = desc->privacy_caps;