[PATCH 0/1][SRU][X/B/D/E] CVE-2019-17666: rtlwifi buffer overflow

[Tyler Hicks]

On 2019-10-21 09:24:32, Alex Murray wrote:
> 
> On Fri, 2019-10-18 at 17:43:02 +1030, Tyler Hicks wrote:
> 
> > https://people.canonical.com/~ubuntu-security/cve/2019/CVE-2019-17666.html
> >
> >  rtl_p2p_noa_ie in drivers/net/wireless/realtek/rtlwifi/ps.c in the
> >  Linux kernel through 5.3.6 lacks a certain upper-bound check, leading
> >  to a buffer overflow.
> >
> > I've followed the suggestion from the rtlwifi maintainer here:
> >
> >  https://lore.kernel.org/lkml/5B2DA6FDDF928F4E855344EE0A5C39D1D5C84368@RTITMBSVM04.realtek.com.tw/
> >
> > A fix is not yet available upstream, which is why this is labeled a
> > SAUCE patch.
> >
> > Clean cherry pick to all releases. Build tested with clean build logs.
> >
> > Tyler
> >
> > Tyler Hicks (1):
> >   UBUNTU: SAUCE: rtlwifi: Fix potential overflow on P2P code
> >
> >  drivers/net/wireless/realtek/rtlwifi/ps.c | 6 ++++--
> >  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> The P2P standard does not impose a limit on the number of NOA
> descriptors within a frame - so the P2P_MAX_NOA_NUM is an artificial
> limit - in which case it seems to make most sense to use min() as you
> have done as and was suggested by the rtlwifi maintainer, rather than
> dropping the entire frame as Laura did in her original patch.

Thanks for the review!

> 
> However I notice that Laura now has an updated patch
> https://lkml.org/lkml/2019/10/18/557 that does _not_ use min() but does
> the size comparison and clamping directly. This has been queued for 5.4
> - https://lkml.org/lkml/2019/10/20/21 - so perhaps it would be worth
> resubmitting this patch based on her latest upstream patch?

I think that min() is still a fine approach. It was even suggested as a
cleanup to v2:

 https://lore.kernel.org/netdev/871rv9xb2l.fsf@kamboji.qca.qualcomm.com/

Considering that the stable kernel team is trying to finalize the trees
and get candidate kernels building, I think we can keep the min() based
patch for this SRU cycle and then switch it out for whatever actually
lands upstream as we inherit the patch through the upstream linux-stable
trees.

Tyler