Hi Brian, > > + if (is_broadcast_ether_addr(pattern)) > > I'm pretty sure it's not valid to look at 'pkt_pattern->pattern' without > also accounting for the ->mask. Same for all the other if/else here. Indeed, I'm not even sure what we fill in there. Perhaps whatever userspace happened to have there, including potentially uninitialized data? > This also hints at a deficiency in the wowlan APIs: nl80211_set_wowlan() > only honors a pre-set set of restrictions, like min/max pattern length, > max offset. For restrictions like this, we either need a wiphy callback, > such that rtw88 can reject arbitrary patterns, or else some additional > declarative fields in 'struct wiphy_wowlan_support'. Yeah, well, we didn't dream up arbitrary restrictions when the API was added :-) It's maybe a bit harder now to add them, but we can do it in cfg80211, and reject it if the conditions are not met, and then older userspace will simply not be able to figure out easily why it was rejected, if it doesn't understand some new features bits/capability advertisement, I guess? I guess I'm not a huge fan of just arbitrarily returning errors when something went wrong and userspace cannot figure out why (vs. advertised restrictions like what we have now), but I guess that'd still be better than nothing ... Ideally, this can just be fixed; if not, IMHO better to add some advertisement bits, but if not then we can surely add some kind of filter callback that's invoked at config time, rather than only at suspend time when it's way too late to do anything about it. johannes
