| Message ID | 20161107145943.16761-9-benjamin@sipsolutions.net (mailing list archive) |
|---|---|
| State | Accepted |
| Delegated to: | Johannes Berg |
| Headers | show |
On Mon, Nov 07, 2016 at 15:59:43 +0100, Benjamin Berg wrote: > From: Benjamin Berg <benjamin.berg@open-mesh.com> > > Signed-off-by: Simon Wunderlich <sw@simonwunderlich.de> > Signed-off-by: Benjamin Berg <benjamin.berg@open-mesh.com> > --- > event.c | 48 +++++++++++++++++++++++++----------------------- > 1 file changed, 25 insertions(+), 23 deletions(-) > > diff --git a/event.c b/event.c > index 446debb..8014d73 100644 > --- a/event.c > +++ b/event.c > @@ -584,30 +584,32 @@ static int print_event(struct nl_msg *msg, void *arg) > nla_data(tb[NL80211_ATTR_VENDOR_DATA]), > nla_len(tb[NL80211_ATTR_VENDOR_DATA])); > break; > - case NL80211_CMD_RADAR_DETECT: > - printf("radar event "); > - if (tb[NL80211_ATTR_RADAR_EVENT]) { > - switch (nla_get_u32(tb[NL80211_ATTR_RADAR_EVENT])) { > - case NL80211_RADAR_DETECTED: > - printf("(radar detected)"); > - break; > - case NL80211_RADAR_CAC_FINISHED: > - printf("(cac finished)"); > - break; > - case NL80211_RADAR_CAC_ABORTED: > - printf("(cac aborted)"); > - break; > - case NL80211_RADAR_NOP_FINISHED: > - printf("(nop finished)"); > - break; > - default: > - printf("(unknown)"); > - break; > - }; > - } else { > - printf("(unknown)"); > + case NL80211_CMD_RADAR_DETECT: { > + enum nl80211_radar_event event_type; > + uint32_t freq; > + > + if (!tb[NL80211_ATTR_RADAR_EVENT] || !tb[NL80211_ATTR_WIPHY_FREQ]) > + printf("BAD radar event"); Should not this end the parsing here or at least avoid getting the value of the NULL attributes below? I do not know if libnl nla_get_u32() is intended to be NULL safe, but following https://www.infradead.org/~tgr/libnl/doc/api/attr_8c_source.html#l00624 it seems like you will get whatever u32 value is at address (NULL+)NLA_HDRLEN, assuming it is readable. The original behavior was to do nothing if tb[NL80211_ATTR_RADAR_EVENT] was not set. > + freq = nla_get_u32(tb[NL80211_ATTR_WIPHY_FREQ]); > + event_type = nla_get_u32(tb[NL80211_ATTR_RADAR_EVENT]); br, henrik
On Thu, 2016-11-17 at 18:21 +0100, Henrik Eriksson wrote: > On Mon, Nov 07, 2016 at 15:59:43 +0100, Benjamin Berg wrote: > > > > + if (!tb[NL80211_ATTR_RADAR_EVENT] || !tb[NL80211_ATTR_WIPHY_FREQ]) > > > > + printf("BAD radar event"); > > Should not this end the parsing here or at least avoid getting the value of > the NULL attributes below? I do not know if libnl nla_get_u32() is > intended to be NULL safe, but following > https://www.infradead.org/~tgr/libnl/doc/api/attr_8c_source.html#l00624 > it seems like you will get whatever u32 value is at address > (NULL+)NLA_HDRLEN, assuming it is readable. The original behavior was to > do nothing if tb[NL80211_ATTR_RADAR_EVENT] was not set. > > > > > + freq = nla_get_u32(tb[NL80211_ATTR_WIPHY_FREQ]); > > > > + event_type = nla_get_u32(tb[NL80211_ATTR_RADAR_EVENT]); Yes, my version of the patch was somewhat broken in that regard. Johannes fixed it before merging and it will now correctly print "BAD radar event\n" and stop processing in case one of the entries is missing. Benjamin
diff --git a/event.c b/event.c index 446debb..8014d73 100644 --- a/event.c +++ b/event.c @@ -584,30 +584,32 @@ static int print_event(struct nl_msg *msg, void *arg) nla_data(tb[NL80211_ATTR_VENDOR_DATA]), nla_len(tb[NL80211_ATTR_VENDOR_DATA])); break; - case NL80211_CMD_RADAR_DETECT: - printf("radar event "); - if (tb[NL80211_ATTR_RADAR_EVENT]) { - switch (nla_get_u32(tb[NL80211_ATTR_RADAR_EVENT])) { - case NL80211_RADAR_DETECTED: - printf("(radar detected)"); - break; - case NL80211_RADAR_CAC_FINISHED: - printf("(cac finished)"); - break; - case NL80211_RADAR_CAC_ABORTED: - printf("(cac aborted)"); - break; - case NL80211_RADAR_NOP_FINISHED: - printf("(nop finished)"); - break; - default: - printf("(unknown)"); - break; - }; - } else { - printf("(unknown)"); + case NL80211_CMD_RADAR_DETECT: { + enum nl80211_radar_event event_type; + uint32_t freq; + + if (!tb[NL80211_ATTR_RADAR_EVENT] || !tb[NL80211_ATTR_WIPHY_FREQ]) + printf("BAD radar event"); + freq = nla_get_u32(tb[NL80211_ATTR_WIPHY_FREQ]); + event_type = nla_get_u32(tb[NL80211_ATTR_RADAR_EVENT]); + + switch (event_type) { + case NL80211_RADAR_DETECTED: + printf("%d MHz: radar detected\n", freq); + break; + case NL80211_RADAR_CAC_FINISHED: + printf("%d MHz: CAC finished\n", freq); + break; + case NL80211_RADAR_CAC_ABORTED: + printf("%d MHz: CAC was aborted\n", freq); + break; + case NL80211_RADAR_NOP_FINISHED: + printf("%d MHz: NOP finished\n", freq); + break; + default: + printf("%d MHz: unknown radar event\n", freq); + } } - printf("\n"); break; case NL80211_CMD_DEL_WIPHY: printf("delete wiphy\n");