[PATCH v2 0/2] wifi: p54: validate PDA entry data lengths in eeprom parser

[PATCH v2 0/2] wifi: p54: validate PDA entry data lengths in eeprom parser

[Shengzhuo Wei]

p54_parse_eeprom() walks PDA records supplied in the device EEPROM but
several handlers consume entry->data without first checking that the
entry actually carries enough bytes, so a malformed or truncated image
makes them read past the EEPROM buffer. This series adds the missing
length checks in two places: the PA calibration curve data and the
interface-list records. It continues the EEPROM trust-boundary
hardening of da1b9a55ff11 ("wifi: p54: prevent buffer-overflow in
p54_rx_eeprom_readback()") and ebd6d37fa94b ("wifi: p54: validate RX
frame length in p54_rx_eeprom_readback()").

Patch 1 rejects the PA calibration curve entry when the channel and
points-per-channel counts it advertises do not fit in the entry data,
the same check p54_convert_output_limits() and p54_convert_db() already
apply to their counts. Patch 2 advances the interface-list walk only
while a full struct exp_if record still fits, instead of reading one
from a trailing partial record.

---
Changes in v2:
- move the curve data length checks into p54_convert_rev0() and
  p54_convert_rev1() (Christian Lamparter)
- collect Acked-by from Christian for the interface-list patch
- Link to v1: https://lore.kernel.org/r/20260827-p54-pda-validation-v1-0-bdc2b0675056@cherr.cc

---
Shengzhuo Wei (2):
      wifi: p54: validate curve data length in the calibration curve converters
      wifi: p54: require a full exp_if record in PDR_INTERFACE_LIST

 drivers/net/wireless/intersil/p54/eeprom.c | 22 +++++++++++++++++-----
 1 file changed, 17 insertions(+), 5 deletions(-)
---
base-commit: 66fb95a521110da673090294561844c9f76ebe64
change-id: 20260826-p54-pda-validation-7b2e91f4c6a3

Best regards,
--  
Shengzhuo Wei <me@cherr.cc>

[PATCH v2 1/2] wifi: p54: validate curve data length in the calibration curve converters

[Shengzhuo Wei]

p54_convert_rev0() and p54_convert_rev1() read calibration curve
data from the device-supplied EEPROM entry using channel and
points-per-channel counts taken verbatim from that same entry, so
an entry that declares more data than it carries drives an
out-of-bounds read past the EEPROM buffer (verified with a KASAN
reproducer of the conversion loop). The sibling converters
p54_convert_output_limits() and p54_convert_db() already validate
their counts against the entry length; this path was missed.

Reject the entry when the counts do not fit in the entry data.

Fixes: eff1a59c48e3 ("[P54]: add mac80211-based driver for prism54 softmac hardware")
Cc: stable@vger.kernel.org
Assisted-by: GLM:5.3
Signed-off-by: Shengzhuo Wei <me@cherr.cc>
---
 drivers/net/wireless/intersil/p54/eeprom.c | 19 +++++++++++++++----
 1 file changed, 15 insertions(+), 4 deletions(-)

diff --git a/drivers/net/wireless/intersil/p54/eeprom.c b/drivers/net/wireless/intersil/p54/eeprom.c
index 95580921d933..0dc848d77c5e 100644
--- a/drivers/net/wireless/intersil/p54/eeprom.c
+++ b/drivers/net/wireless/intersil/p54/eeprom.c
@@ -414,17 +414,22 @@ static int p54_generate_channel_lists(struct ieee80211_hw *dev)
 }
 
 static int p54_convert_rev0(struct ieee80211_hw *dev,
-			    struct pda_pa_curve_data *curve_data)
+			    struct pda_pa_curve_data *curve_data, size_t len)
 {
 	struct p54_common *priv = dev->priv;
 	struct p54_pa_curve_data_sample *dst;
 	struct pda_pa_curve_data_sample_rev0 *src;
+	size_t needed = curve_data->channels *
+		(sizeof(*src) * curve_data->points_per_channel + 2);
 	size_t cd_len = sizeof(*curve_data) +
 		(curve_data->points_per_channel*sizeof(*dst) + 2) *
 		 curve_data->channels;
 	unsigned int i, j;
 	void *source, *target;
 
+	if (len < sizeof(*curve_data) + needed)
+		return -EINVAL;
+
 	priv->curve_data = kmalloc(sizeof(*priv->curve_data) + cd_len,
 				   GFP_KERNEL);
 	if (!priv->curve_data)
@@ -466,17 +471,22 @@ static int p54_convert_rev0(struct ieee80211_hw *dev,
 }
 
 static int p54_convert_rev1(struct ieee80211_hw *dev,
-			    struct pda_pa_curve_data *curve_data)
+			    struct pda_pa_curve_data *curve_data, size_t len)
 {
 	struct p54_common *priv = dev->priv;
 	struct p54_pa_curve_data_sample *dst;
 	struct pda_pa_curve_data_sample_rev1 *src;
+	size_t needed = curve_data->channels *
+		(sizeof(*src) * curve_data->points_per_channel + 3);
 	size_t cd_len = sizeof(*curve_data) +
 		(curve_data->points_per_channel*sizeof(*dst) + 2) *
 		 curve_data->channels;
 	unsigned int i, j;
 	void *source, *target;
 
+	if (len < sizeof(*curve_data) + needed)
+		return -EINVAL;
+
 	priv->curve_data = kzalloc(cd_len + sizeof(*priv->curve_data),
 				   GFP_KERNEL);
 	if (!priv->curve_data)
@@ -763,6 +773,7 @@ int p54_parse_eeprom(struct ieee80211_hw *dev, void *eeprom, int len)
 		case PDR_PRISM_PA_CAL_CURVE_DATA: {
 			struct pda_pa_curve_data *curve_data =
 				(struct pda_pa_curve_data *)entry->data;
+
 			if (data_len < sizeof(*curve_data)) {
 				err = -EINVAL;
 				goto err;
@@ -770,10 +781,10 @@ int p54_parse_eeprom(struct ieee80211_hw *dev, void *eeprom, int len)
 
 			switch (curve_data->cal_method_rev) {
 			case 0:
-				err = p54_convert_rev0(dev, curve_data);
+				err = p54_convert_rev0(dev, curve_data, data_len);
 				break;
 			case 1:
-				err = p54_convert_rev1(dev, curve_data);
+				err = p54_convert_rev1(dev, curve_data, data_len);
 				break;
 			default:
 				wiphy_err(dev->wiphy,

-- 
2.47.3

Re: [PATCH v2 1/2] wifi: p54: validate curve data length in the calibration curve converters

[Christian Lamparter]

Hi,

On 8/30/26 8:42 PM, Shengzhuo Wei wrote:
> p54_convert_rev0() and p54_convert_rev1() read calibration curve
> data from the device-supplied EEPROM entry using channel and
> points-per-channel counts taken verbatim from that same entry, so
> an entry that declares more data than it carries drives an
> out-of-bounds read past the EEPROM buffer (verified with a KASAN
> reproducer of the conversion loop). The sibling converters
> p54_convert_output_limits() and p54_convert_db() already validate
> their counts against the entry length; this path was missed.
> 
> Reject the entry when the counts do not fit in the entry data.
> 
> Fixes: eff1a59c48e3 ("[P54]: add mac80211-based driver for prism54 softmac hardware")
> Cc: stable@vger.kernel.org
> Assisted-by: GLM:5.3
> Signed-off-by: Shengzhuo Wei <me@cherr.cc>
> ---
>   drivers/net/wireless/intersil/p54/eeprom.c | 19 +++++++++++++++----
>   1 file changed, 15 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/net/wireless/intersil/p54/eeprom.c b/drivers/net/wireless/intersil/p54/eeprom.c
> index 95580921d933..0dc848d77c5e 100644
> --- a/drivers/net/wireless/intersil/p54/eeprom.c
> +++ b/drivers/net/wireless/intersil/p54/eeprom.c
> @@ -414,17 +414,22 @@ static int p54_generate_channel_lists(struct ieee80211_hw *dev)
>   }
>   
>   static int p54_convert_rev0(struct ieee80211_hw *dev,
> -			    struct pda_pa_curve_data *curve_data)
> +			    struct pda_pa_curve_data *curve_data, size_t len)
>   {
>   	struct p54_common *priv = dev->priv;
>   	struct p54_pa_curve_data_sample *dst;
>   	struct pda_pa_curve_data_sample_rev0 *src;
> +	size_t needed = curve_data->channels *
> +		(sizeof(*src) * curve_data->points_per_channel + 2);
>   	size_t cd_len = sizeof(*curve_data) +
>   		(curve_data->points_per_channel*sizeof(*dst) + 2) *
>   		 curve_data->channels;
>   	unsigned int i, j;
>   	void *source, *target;
>   
> +	if (len < sizeof(*curve_data) + needed)
> +		return -EINVAL;
> +

Hmm, Puh. Interessting. Several things. But yeah, this should work.

Acked-by: Christian Lamparter <chunkeey@gmail.com>

Still I have some questions: Did you write/touch any of this yourself?
Or is this patch straight from the model?

It's because I can grok (heh) why "needed" ended up as a separate variable next to cd_len.
But why was the sizeof(*curve_data) not included there too? It's only used once in the
if check so and this sounds like the "needed" needed some extra? Maybe because it was
already checked?

Well, I'm positive there will be an update from someone else to make it look "neat".
Probably they will complain that it looks like this functions use curve_data->points_per_channel
and curve_data->channels without being checked... Only to find out that it was checked already
because the code needs to know the revision before actually calling the functions and this all
being part of the information struct.

oh, well.

Re: [PATCH v2 1/2] wifi: p54: validate curve data length in the calibration curve converters

[Shengzhuo Wei]

On 2026-09-06 11:31, Christian Lamparter wrote:
> > diff --git a/drivers/net/wireless/intersil/p54/eeprom.c b/drivers/net/wireless/intersil/p54/eeprom.c
> > index 95580921d933..0dc848d77c5e 100644
> > --- a/drivers/net/wireless/intersil/p54/eeprom.c
> > +++ b/drivers/net/wireless/intersil/p54/eeprom.c
> > @@ -414,17 +414,22 @@ static int p54_generate_channel_lists(struct ieee80211_hw *dev)
> >   }
> >   static int p54_convert_rev0(struct ieee80211_hw *dev,
> > -			    struct pda_pa_curve_data *curve_data)
> > +			    struct pda_pa_curve_data *curve_data, size_t len)
> >   {
> >   	struct p54_common *priv = dev->priv;
> >   	struct p54_pa_curve_data_sample *dst;
> >   	struct pda_pa_curve_data_sample_rev0 *src;
> > +	size_t needed = curve_data->channels *
> > +		(sizeof(*src) * curve_data->points_per_channel + 2);
> >   	size_t cd_len = sizeof(*curve_data) +
> >   		(curve_data->points_per_channel*sizeof(*dst) + 2) *
> >   		 curve_data->channels;
> >   	unsigned int i, j;
> >   	void *source, *target;
> > +	if (len < sizeof(*curve_data) + needed)
> > +		return -EINVAL;
> > +
> 
> Hmm, Puh. Interessting. Several things. But yeah, this should work.
> 
> Acked-by: Christian Lamparter <chunkeey@gmail.com>

Hi Christian,

Thanks for the review and the Ack.

> Still I have some questions: Did you write/touch any of this yourself?
> Or is this patch straight from the model?

AI found the bug. I wrote the fix myself and used AI to review it
afterwards.
 
> It's because I can grok (heh) why "needed" ended up as a separate variable next to cd_len.
> But why was the sizeof(*curve_data) not included there too? It's only used once in the
> if check so and this sounds like the "needed" needed some extra? Maybe because it was
> already checked?

I kept sizeof(*curve_data) separate because I was thinking of needed
as the input data size without the header. But since it is only used
in that check, adding the header there too would be simpler.

Would you like me to send a v3 that makes just this change in both
converters?

Thanks,
Shengzhuo

Re: [PATCH v2 1/2] wifi: p54: validate curve data length in the calibration curve converters

[Christian Lamparter]

On 9/6/26 1:43 PM, Shengzhuo Wei wrote:
> On 2026-09-06 11:31, Christian Lamparter wrote:
>>> diff --git a/drivers/net/wireless/intersil/p54/eeprom.c b/drivers/net/wireless/intersil/p54/eeprom.c
>>> index 95580921d933..0dc848d77c5e 100644
>>> --- a/drivers/net/wireless/intersil/p54/eeprom.c
>>> +++ b/drivers/net/wireless/intersil/p54/eeprom.c
>>> @@ -414,17 +414,22 @@ static int p54_generate_channel_lists(struct ieee80211_hw *dev)
>>>    }
>>>    static int p54_convert_rev0(struct ieee80211_hw *dev,
>>> -			    struct pda_pa_curve_data *curve_data)
>>> +			    struct pda_pa_curve_data *curve_data, size_t len)
>>>    {
>>>    	struct p54_common *priv = dev->priv;
>>>    	struct p54_pa_curve_data_sample *dst;
>>>    	struct pda_pa_curve_data_sample_rev0 *src;
>>> +	size_t needed = curve_data->channels *
>>> +		(sizeof(*src) * curve_data->points_per_channel + 2);
>>>    	size_t cd_len = sizeof(*curve_data) +
>>>    		(curve_data->points_per_channel*sizeof(*dst) + 2) *
>>>    		 curve_data->channels;
>>>    	unsigned int i, j;
>>>    	void *source, *target;
>>> +	if (len < sizeof(*curve_data) + needed)
>>> +		return -EINVAL;
>>> +
>>
>> Hmm, Puh. Interessting. Several things. But yeah, this should work.
>>
>> Acked-by: Christian Lamparter <chunkeey@gmail.com>
> 
> Hi Christian,
> 
> Thanks for the review and the Ack.
> 
>> Still I have some questions: Did you write/touch any of this yourself?
>> Or is this patch straight from the model?
> 
> AI found the bug. I wrote the fix myself and used AI to review it
> afterwards.
>   
>> It's because I can grok (heh) why "needed" ended up as a separate variable next to cd_len.
>> But why was the sizeof(*curve_data) not included there too? It's only used once in the
>> if check so and this sounds like the "needed" needed some extra? Maybe because it was
>> already checked?
> 
> I kept sizeof(*curve_data) separate because I was thinking of needed
> as the input data size without the header. But since it is only used
> in that check, adding the header there too would be simpler.
> 
> Would you like me to send a v3 that makes just this change in both
> converters?

Well... I think Johannes already added v2 two days ago to wireless + wireless-next.
<https://git.kernel.org/pub/scm/linux/kernel/git/wireless/wireless.git/commit/?id=ce858fa6b8a214dee5adb82358885fa024cdd887>

So: 🤷. I don't think you need to bother with making/sending a v3.

Cheers,
Christian

[PATCH v2 2/2] wifi: p54: require a full exp_if record in PDR_INTERFACE_LIST

[Shengzhuo Wei]

The PDR_INTERFACE_LIST loop only checks that the record start is within
the entry before reading an entire struct exp_if from it. A truncated
trailing record makes the if_id/variant reads cross the entry boundary
into the heap beyond the EEPROM buffer (verified with a KASAN
reproducer of the loop). The variant also feeds the synth front-end
selection, so this is not only a leak.

Advance only while a full record still fits in the entry.

Fixes: eff1a59c48e3 ("[P54]: add mac80211-based driver for prism54 softmac hardware")
Cc: stable@vger.kernel.org
Acked-by: Christian Lamparter <chunkeey@gmail.com>
Assisted-by: GLM:5.3
Signed-off-by: Shengzhuo Wei <me@cherr.cc>
---
 drivers/net/wireless/intersil/p54/eeprom.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/net/wireless/intersil/p54/eeprom.c b/drivers/net/wireless/intersil/p54/eeprom.c
index 0dc848d77c5e..0475222d54fc 100644
--- a/drivers/net/wireless/intersil/p54/eeprom.c
+++ b/drivers/net/wireless/intersil/p54/eeprom.c
@@ -812,7 +812,8 @@ int p54_parse_eeprom(struct ieee80211_hw *dev, void *eeprom, int len)
 			break;
 		case PDR_INTERFACE_LIST:
 			tmp = entry->data;
-			while ((u8 *)tmp < entry->data + data_len) {
+			while ((u8 *)tmp + sizeof(struct exp_if) <=
+			       entry->data + data_len) {
 				struct exp_if *exp_if = tmp;
 				if (exp_if->if_id == cpu_to_le16(IF_ID_ISL39000))
 					synth = le16_to_cpu(exp_if->variant);

-- 
2.47.3