Re: [PATCH v3 3/6] wifi: mwifiex: fix OOB read from firmware sta_count in station list response

[Johannes Berg <johannes@sipsolutions.net>]

On Wed, 2026-04-22 at 11:26 -0700, Brian Norris wrote:
> 
> > +	u16 resp_size = le16_to_cpu(resp->size);
> > +	u16 count = le16_to_cpu(sta_list->sta_count);
> > +	u16 max_count;
> >  
> > -	for (i = 0; i < (le16_to_cpu(sta_list->sta_count)); i++) {
> > +	if (resp_size < sizeof(*resp) - sizeof(resp->params) + sizeof(*sta_list))
> > +		return -EINVAL;
> > +	max_count = (resp_size - sizeof(*resp) + sizeof(resp->params) -
> > +		     sizeof(*sta_list)) / sizeof(*sta_info);
> 
> The repeated arithmetic is a bit weird, but I'm not sure if it'd
> actually be better to stash it in its own variable. Seems good enough I
> suppose.

I think it might be simpler if instead trying to limit:

> > +	count = min(count, max_count);

it'd just check the needed length based on the given count, and reject
anything above that?

Also, the whole sizeof(*resp) - sizeof(resp->params) etc. shouldn't be
there, that should just be

	offsetofend(resp, sta_list.tlv)

and then suddenly it becomes _way_ simpler:

	if (resp_size < offsetofend(resp, sta_list.tlv))
		return -EINVAL;
	if (resp_size < offsetofend(resp, sta_list.tlv) +
			sizeof(*sta_info) * le16_to_cpu(sta_.list->sta_count))
		return -EINVAL;

But regardless, I question the sanity of checking the size against the
size the firmware said the whole thing was going to be, rather than
checking against the actual buffer size ...

johannes