[PATCH] net: prestera: validate num_counters in counter response

[PATCH] net: prestera: validate num_counters in counter response

[Aamir Ahmed]

prestera_hw_counters_get() iterates over resp->num_counters
entries from the firmware response without verifying it does
not exceed the number of entries requested. A firmware response
claiming more counters than requested causes out-of-bounds reads
from the response buffer and out-of-bounds writes to the caller
stats array.

Validate that num_counters does not exceed the requested count
before iterating.

Fixes: 6e36c7bcb461 ("net: prestera: add counter HW API")
Signed-off-by: Aamir Ahmed <elb12345@hotmail.co.uk>
---
 drivers/net/ethernet/marvell/prestera/prestera_hw.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/net/ethernet/marvell/prestera/prestera_hw.c b/drivers/net/ethernet/marvell/prestera/prestera_hw.c
index 7695cbb2ce62..c4cb5f22893c 100644
--- a/drivers/net/ethernet/marvell/prestera/prestera_hw.c
+++ b/drivers/net/ethernet/marvell/prestera/prestera_hw.c
@@ -2324,6 +2324,11 @@ int prestera_hw_counters_get(struct prestera_switch *sw, u32 idx,
 	if (err)
 		goto free_buff;
 
+	if (__le32_to_cpu(resp->num_counters) > *len) {
+		err = -EINVAL;
+		goto free_buff;
+	}
+
 	for (i = 0; i < __le32_to_cpu(resp->num_counters); i++) {
 		stats[i].packets += __le64_to_cpu(resp->stats[i].packets);
 		stats[i].bytes += __le64_to_cpu(resp->stats[i].bytes);
-- 
2.43.0

Re: [PATCH] net: prestera: validate num_counters in counter response

[Paolo Abeni]

On 9/7/26 5:22 AM, Aamir Ahmed wrote:
> prestera_hw_counters_get() iterates over resp->num_counters
> entries from the firmware response without verifying it does
> not exceed the number of entries requested. A firmware response
> claiming more counters than requested causes out-of-bounds reads
> from the response buffer and out-of-bounds writes to the caller
> stats array.
> 
> Validate that num_counters does not exceed the requested count
> before iterating.
> 
> Fixes: 6e36c7bcb461 ("net: prestera: add counter HW API")
> Signed-off-by: Aamir Ahmed <elb12345@hotmail.co.uk>
Patchwork shows more than 20 patches from you in flight. Please read

Documentation/process/maintainer-netdev.rst

carefully, especially the section about the 15 patches limit.

Stop sending new patches until are the pending ones are processed.

Processing for the pending ones will be likely significantly delayed.

/P

Re: [PATCH] net: prestera: validate num_counters in counter response

[Aamir Ahmed]

On Mon, Sep 07, 2026 at 11:03:19AM +0200, Paolo Abeni wrote:
> Patchwork shows more than 20 patches from you in flight. Please read
>
> Documentation/process/maintainer-netdev.rst
>
> carefully, especially the section about the 15 patches limit.
>
> Stop sending new patches until are the pending ones are processed.

Apologies! This is my bad, understandable if anything I sent over is to
be backlogged. Will ensure this doesn't happen again for the future.

Kind Regards

Aamir A.

Re: [PATCH] net: prestera: validate num_counters in counter response

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider:

Critical: 0 · High: 1 · Medium: 0 · Low: 0

- [High] Incomplete bounds check: prestera_hw_counters_get()
  (drivers/net/ethernet/marvell/prestera/prestera_hw.c:2327) validates…

--

net: prestera: validate num_counters in counter response

This adds a check in prestera_hw_counters_get() that rejects a firmware
counter response whose num_counters field exceeds the number of counters
that were requested, returning -EINVAL instead of iterating over the
response.

> diff --git a/drivers/net/ethernet/marvell/prestera/prestera_hw.c b/drivers/net/ethernet/marvell/prestera/prestera_hw.c
> index 7695cbb2ce626..c4cb5f22893c9 100644
> --- a/drivers/net/ethernet/marvell/prestera/prestera_hw.c
> +++ b/drivers/net/ethernet/marvell/prestera/prestera_hw.c
> @@ -2324,6 +2324,11 @@ int prestera_hw_counters_get(struct prestera_switch *sw, u32 idx,
>  	if (err)
>  		goto free_buff;
>  
> +	if (__le32_to_cpu(resp->num_counters) > *len) {
> +		err = -EINVAL;
> +		goto free_buff;
> +	}
> +

[Severity: High]
The commit message says this closes "out-of-bounds writes to the caller
stats array", but does *len actually describe the capacity of stats[]?

The only caller is prestera_counter_stats_work() in prestera_counter.c,
which always requests a fixed chunk size and passes a pointer into the
middle of block->stats:

	u32 count = COUNTER_BULK_SIZE;
	...
	err = prestera_hw_counters_get(counter->sw, counter->total_read,
				       &count, &done,
				       &block->stats[counter->total_read]);

COUNTER_BULK_SIZE is 256, while block->stats is allocated with exactly
block->num_counters elements in prestera_counter_block_get():

	err = prestera_hw_counter_block_get(counter->sw, client,
					    &block->id, &block->offset,
					    &block->num_counters);
	...
	block->stats = kzalloc_objs(*block->stats, block->num_counters);

block->num_counters also comes from the firmware, so it can be smaller
than 256 and need not be a multiple of 256.

Can stats[] still be overflowed with this check in place?  If firmware
reports num_counters = 10 for the block and then answers the
COUNTER_GET with resp->num_counters = 256, the new test (256 > 256) is
false and the loop below writes stats[0..255] into a 10 element array.
Similarly, with block->num_counters = 300, the second poll iteration
passes &block->stats[256] which has only 44 slots left, yet *len is
still 256, so any reply in the range 45..256 passes.

Would clamping the requested count in prestera_counter_stats_work() to
block->num_counters - counter->total_read, or passing the remaining
capacity down to prestera_hw_counters_get(), be a better place for the
bound?  The helper as written never learns the size of the destination
array.

>  	for (i = 0; i < __le32_to_cpu(resp->num_counters); i++) {
>  		stats[i].packets += __le64_to_cpu(resp->stats[i].packets);
>  		stats[i].bytes += __le64_to_cpu(resp->stats[i].bytes);

[Severity: High]
Related to the above: on success *len is set to resp->num_counters, and
prestera_counter_stats_work() then does:

	counter->total_read += count;
	if (!done || counter->total_read < block->num_counters) {
		resched_time = COUNTER_RESCHED_TIME;
		goto resched;
	}

Can counter->total_read end up larger than block->num_counters here?  If
so, and the same reply has done == 0, the work is rescheduled and the
next invocation forms &block->stats[counter->total_read], which is
already past the end of the array before any write happens.

The out-of-bounds read of resp->stats[] does look addressed by this
check, since the response buffer is allocated with
struct_size(resp, stats, *len).  Would it be worth splitting the commit
message so it only claims the read side, or extending the fix to cover
the write side too?

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/AS8P251MB0001CC8A4737B0CC46F41DDFC8B22%40AS8P251MB0001.EURP251.PROD.OUTLOOK.COM