From 6162a84f807136cf1f5cc8d139cf66da099ee40f Mon Sep 17 00:00:00 2001 From: Gregory Nutt Date: Sun, 18 Jun 2017 13:33:07 -0600 Subject: [PATCH] ieee 802.15.4: Need counting protection on the logic that releases the notification resources. Otherwise, notification handlers may be operating with a stale pointer. --- wireless/ieee802154/mac802154.c | 55 ++++++++----------- wireless/ieee802154/mac802154_assoc.c | 11 ++-- wireless/ieee802154/mac802154_bind.c | 7 +++ wireless/ieee802154/mac802154_internal.h | 4 +- wireless/ieee802154/mac802154_netdev.c | 4 ++ wireless/ieee802154/mac802154_notif.c | 69 +++++++++++++++++++++--- wireless/ieee802154/mac802154_notif.h | 7 ++- wireless/ieee802154/mac802154_poll.c | 9 ++-- 8 files changed, 112 insertions(+), 54 deletions(-) diff --git a/wireless/ieee802154/mac802154.c b/wireless/ieee802154/mac802154.c index fa51213218..2c60866e71 100644 --- a/wireless/ieee802154/mac802154.c +++ b/wireless/ieee802154/mac802154.c @@ -73,11 +73,6 @@ static void mac802154_resetqueues(FAR struct ieee802154_privmac_s *priv); -/* MAC client notification */ - -static void mac802154_notify(FAR struct ieee802154_privmac_s *priv, - FAR struct ieee802154_notif_s *notif); - /* IEEE 802.15.4 PHY Interface OPs */ static int mac802154_radiopoll(FAR const struct ieee802154_radiocb_s *radiocb, @@ -496,34 +491,6 @@ static void mac802154_purge_worker(FAR void *arg) } } -/**************************************************************************** - * Name: mac802154_notify - * - * Description: - * Notify every register MAC client. - * - ****************************************************************************/ - -static void mac802154_notify(FAR struct ieee802154_privmac_s *priv, - FAR struct ieee802154_notif_s *notif) -{ - FAR struct mac802154_maccb_s *cb; - - /* Try to notify every registered MAC client */ - - for (cb = priv->cb; cb != NULL; cb = cb->flink) - { - /* Does this client want notifications? */ - - if (cb->notify != NULL) - { - /* Yes.. Notify */ - - cb->notify(cb, notif); - } - } -} - /**************************************************************************** * Name: mac802154_radiopoll * @@ -667,6 +634,7 @@ static void mac802154_txdone_worker(FAR void *arg) mac802154_takesem(&priv->exclsem, false); } break; + case IEEE802154_FRAME_COMMAND: { switch (priv->curr_cmd) @@ -674,10 +642,13 @@ static void mac802154_txdone_worker(FAR void *arg) case IEEE802154_CMD_ASSOC_REQ: mac802154_txdone_assocreq(priv, txdesc); break; + case IEEE802154_CMD_ASSOC_RESP: break; + case IEEE802154_CMD_DISASSOC_NOT: break; + case IEEE802154_CMD_DATA_REQ: /* Data requests can be sent for 3 different reasons. * @@ -697,23 +668,31 @@ static void mac802154_txdone_worker(FAR void *arg) case MAC802154_OP_ASSOC: mac802154_txdone_datareq_assoc(priv, txdesc); break; + case MAC802154_OP_POLL: mac802154_txdone_datareq_poll(priv, txdesc); break; + default: break; } break; + case IEEE802154_CMD_PANID_CONF_NOT: break; + case IEEE802154_CMD_ORPHAN_NOT: break; + case IEEE802154_CMD_BEACON_REQ: break; + case IEEE802154_CMD_COORD_REALIGN: break; + case IEEE802154_CMD_GTS_REQ: break; + default: /* We can deallocate the data conf notification as it is no * longer needed. We can't use the public function here @@ -722,10 +701,12 @@ static void mac802154_txdone_worker(FAR void *arg) privnotif->flink = priv->notif_free; priv->notif_free = privnotif; + priv->nnotif = 0; break; } } break; + default: { /* We can deallocate the data conf notification as it is no longer @@ -941,22 +922,30 @@ static void mac802154_rxframe_worker(FAR void *arg) case IEEE802154_CMD_ASSOC_REQ: mac802154_rx_assocreq(priv, ind); break; + case IEEE802154_CMD_ASSOC_RESP: mac802154_rx_assocresp(priv, ind); break; + case IEEE802154_CMD_DISASSOC_NOT: break; + case IEEE802154_CMD_DATA_REQ: mac802154_rx_datareq(priv, ind); break; + case IEEE802154_CMD_PANID_CONF_NOT: break; + case IEEE802154_CMD_ORPHAN_NOT: break; + case IEEE802154_CMD_BEACON_REQ: break; + case IEEE802154_CMD_COORD_REALIGN: break; + case IEEE802154_CMD_GTS_REQ: break; } diff --git a/wireless/ieee802154/mac802154_assoc.c b/wireless/ieee802154/mac802154_assoc.c index d9ab6cb19d..c5ef212da0 100644 --- a/wireless/ieee802154/mac802154_assoc.c +++ b/wireless/ieee802154/mac802154_assoc.c @@ -480,7 +480,7 @@ void mac802154_txdone_assocreq(FAR struct ieee802154_privmac_s *priv, /* Release the MAC, call the callback, get exclusive access again */ mac802154_givesem(&priv->exclsem); - priv->cb->notify(priv->cb, notif); + mac802154_notify(priv, notif); mac802154_takesem(&priv->exclsem, false); } else @@ -612,7 +612,7 @@ void mac802154_txdone_datareq_assoc(FAR struct ieee802154_privmac_s *priv, /* Release the MAC, call the callback, get exclusive access again */ mac802154_givesem(&priv->exclsem); - priv->cb->notify(priv->cb, notif); + mac802154_notify(priv, notif); mac802154_takesem(&priv->exclsem, false); } else @@ -708,8 +708,7 @@ void mac802154_rx_assocreq(FAR struct ieee802154_privmac_s *priv, /* Notify the next highest layer of the association status */ - priv->cb->notify(priv->cb, notif); - + mac802154_notify(priv, notif); return; errout_with_sem: @@ -804,7 +803,7 @@ void mac802154_rx_assocresp(FAR struct ieee802154_privmac_s *priv, /* Notify the next highest layer of the association status */ - priv->cb->notify(priv->cb, notif); + mac802154_notify(priv, notif); } /**************************************************************************** @@ -853,5 +852,5 @@ static void mac802154_timeout_assoc(FAR struct ieee802154_privmac_s *priv) notif->u.assocconf.status = IEEE802154_STATUS_NO_DATA; notif->u.assocconf.saddr = IEEE802154_SADDR_UNSPEC; - priv->cb->notify(priv->cb, notif); + mac802154_notify(priv, notif); } diff --git a/wireless/ieee802154/mac802154_bind.c b/wireless/ieee802154/mac802154_bind.c index 9b6bacea39..091d83814a 100644 --- a/wireless/ieee802154/mac802154_bind.c +++ b/wireless/ieee802154/mac802154_bind.c @@ -105,5 +105,12 @@ int mac802154_bind(MACHANDLE mac, FAR struct mac802154_maccb_s *cb) prev->flink = cb; } + /* Keep track of the number of clients requesting notification */ + + if (cb->notify != NULL) + { + priv->nclients++; + } + return OK; } diff --git a/wireless/ieee802154/mac802154_internal.h b/wireless/ieee802154/mac802154_internal.h index adfc176c03..798f48a927 100644 --- a/wireless/ieee802154/mac802154_internal.h +++ b/wireless/ieee802154/mac802154_internal.h @@ -150,7 +150,9 @@ struct ieee802154_privmac_s FAR struct mac802154_maccb_s *cb; /* Head of a list of MAC callbacks */ FAR struct mac802154_radiocb_s radiocb; /* Interface to bind to radio */ - sem_t exclsem; /* Support exclusive access */ + sem_t exclsem; /* Support exclusive access */ + uint8_t nclients; /* Number of notification clients */ + uint8_t nnotif; /* Number of remaining notifications */ /* Only support a single command at any given time. As of now I see no * condition where you need to have more than one command frame simultaneously diff --git a/wireless/ieee802154/mac802154_netdev.c b/wireless/ieee802154/mac802154_netdev.c index 41946846b9..be9c3fc678 100644 --- a/wireless/ieee802154/mac802154_netdev.c +++ b/wireless/ieee802154/mac802154_netdev.c @@ -244,6 +244,10 @@ static void macnet_notify(FAR struct mac802154_maccb_s *maccb, default: break; } + + /* Free the event notification */ + + mac802154_notif_free(priv->md_mac, notif); } /**************************************************************************** diff --git a/wireless/ieee802154/mac802154_notif.c b/wireless/ieee802154/mac802154_notif.c index 15368a5483..bce299c139 100644 --- a/wireless/ieee802154/mac802154_notif.c +++ b/wireless/ieee802154/mac802154_notif.c @@ -74,18 +74,38 @@ int mac802154_notif_free(MACHANDLE mac, { FAR struct ieee802154_privmac_s *priv = (FAR struct ieee802154_privmac_s *)mac; - FAR struct mac802154_notif_s *privnotif = (FAR struct mac802154_notif_s *)notif; + FAR struct mac802154_notif_s *privnotif = + (FAR struct mac802154_notif_s *)notif; /* Get exclusive access to the MAC */ mac802154_takesem(&priv->exclsem, false); - privnotif->flink = priv->notif_free; - priv->notif_free = privnotif; - mac802154_givesem(&priv->notif_sem); + /* We know how many clients have registered for notifications. Each must + * call mac802154_notif_free() before we can release the notification + * resource. + */ + + if (priv->nnotif < 2) + { + /* This is the free from the last notification */ + + privnotif->flink = priv->notif_free; + priv->notif_free = privnotif; + priv->nnotif = 0; + + mac802154_givesem(&priv->notif_sem); + } + else + { + /* More calls are expected. Decrement the count of expected calls + * and preserve the notification resources. + */ + + priv->nnotif--; + } mac802154_givesem(&priv->exclsem); - return -ENOTTY; } @@ -125,6 +145,7 @@ void mac802154_notifpool_init(FAR struct ieee802154_privmac_s *priv) pool++; remaining--; } + sem_init(&priv->notif_sem, 0, CONFIG_MAC802154_NNOTIF); } @@ -165,6 +186,7 @@ int mac802154_notif_alloc(FAR struct ieee802154_privmac_s *priv, { privnotif = priv->notif_free; priv->notif_free = privnotif->flink; + priv->nnotif = 0; } else { @@ -200,9 +222,44 @@ int mac802154_notif_alloc(FAR struct ieee802154_privmac_s *priv, privnotif = priv->notif_free; priv->notif_free = privnotif->flink; + priv->nnotif = 0; } *notif = (FAR struct ieee802154_notif_s *)privnotif; return OK; -} \ No newline at end of file +} + +/**************************************************************************** + * Name: mac802154_notify + * + * Description: + * Notify every register MAC client. + * + ****************************************************************************/ + +void mac802154_notify(FAR struct ieee802154_privmac_s *priv, + FAR struct ieee802154_notif_s *notif) +{ + FAR struct mac802154_maccb_s *cb; + + /* Set the notification count so that the notification resources will be + * preserved until the final notification. + */ + + priv->nnotif = priv->nclients; + + /* Try to notify every registered MAC client */ + + for (cb = priv->cb; cb != NULL; cb = cb->flink) + { + /* Does this client want notifications? */ + + if (cb->notify != NULL) + { + /* Yes.. Notify */ + + cb->notify(cb, notif); + } + } +} diff --git a/wireless/ieee802154/mac802154_notif.h b/wireless/ieee802154/mac802154_notif.h index 3cf088163d..3c6bdd3523 100644 --- a/wireless/ieee802154/mac802154_notif.h +++ b/wireless/ieee802154/mac802154_notif.h @@ -63,8 +63,8 @@ struct mac802154_notif_s { - struct ieee802154_notif_s pub; - FAR struct mac802154_notif_s *flink; + struct ieee802154_notif_s pub; /* Publically visible structure */ + FAR struct mac802154_notif_s *flink; /* Supports a singly linked list */ }; /**************************************************************************** @@ -79,4 +79,7 @@ int mac802154_notif_alloc(FAR struct ieee802154_privmac_s *priv, FAR struct ieee802154_notif_s **notif, bool allow_interrupt); +void mac802154_notify(FAR struct ieee802154_privmac_s *priv, + FAR struct ieee802154_notif_s *notif); + #endif /* __WIRELESS_IEEE802154__MAC802154_NOTIF_H */ \ No newline at end of file diff --git a/wireless/ieee802154/mac802154_poll.c b/wireless/ieee802154/mac802154_poll.c index 3f74b55224..57c3154991 100644 --- a/wireless/ieee802154/mac802154_poll.c +++ b/wireless/ieee802154/mac802154_poll.c @@ -50,8 +50,6 @@ #include #include -#include - #include "mac802154.h" #include "mac802154_internal.h" @@ -83,7 +81,6 @@ int mac802154_req_poll(MACHANDLE mac, FAR struct ieee802154_poll_req_s *req) { FAR struct ieee802154_privmac_s *priv = (FAR struct ieee802154_privmac_s *)mac; - FAR struct iob_s *iob; FAR struct ieee802154_txdesc_s *txdesc; int ret; @@ -221,7 +218,7 @@ void mac802154_txdone_datareq_poll(FAR struct ieee802154_privmac_s *priv, /* Release the MAC, call the callback, get exclusive access again */ mac802154_givesem(&priv->exclsem); - priv->cb->notify(priv->cb, notif); + mac802154_notify(priv, notif); mac802154_takesem(&priv->exclsem, false); } else @@ -261,7 +258,7 @@ void mac802154_txdone_datareq_poll(FAR struct ieee802154_privmac_s *priv, * ****************************************************************************/ -static void mac802154_timeout_poll(FAR struct ieee802154_privmac_s *priv) +void mac802154_timeout_poll(FAR struct ieee802154_privmac_s *priv) { FAR struct ieee802154_notif_s *notif; @@ -286,5 +283,5 @@ static void mac802154_timeout_poll(FAR struct ieee802154_privmac_s *priv) notif->notiftype = IEEE802154_NOTIFY_CONF_POLL; notif->u.pollconf.status = IEEE802154_STATUS_NO_DATA; - priv->cb->notify(priv->cb, notif); + mac802154_notify(priv, notif); } \ No newline at end of file