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.

This commit is contained in:
Gregory Nutt
2017-06-18 13:33:07 -06:00
parent 50df37931d
commit 6162a84f80
8 changed files with 112 additions and 54 deletions
+22 -33
View File
@@ -73,11 +73,6 @@
static void mac802154_resetqueues(FAR struct ieee802154_privmac_s *priv); 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 */ /* IEEE 802.15.4 PHY Interface OPs */
static int mac802154_radiopoll(FAR const struct ieee802154_radiocb_s *radiocb, 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 * Name: mac802154_radiopoll
* *
@@ -667,6 +634,7 @@ static void mac802154_txdone_worker(FAR void *arg)
mac802154_takesem(&priv->exclsem, false); mac802154_takesem(&priv->exclsem, false);
} }
break; break;
case IEEE802154_FRAME_COMMAND: case IEEE802154_FRAME_COMMAND:
{ {
switch (priv->curr_cmd) switch (priv->curr_cmd)
@@ -674,10 +642,13 @@ static void mac802154_txdone_worker(FAR void *arg)
case IEEE802154_CMD_ASSOC_REQ: case IEEE802154_CMD_ASSOC_REQ:
mac802154_txdone_assocreq(priv, txdesc); mac802154_txdone_assocreq(priv, txdesc);
break; break;
case IEEE802154_CMD_ASSOC_RESP: case IEEE802154_CMD_ASSOC_RESP:
break; break;
case IEEE802154_CMD_DISASSOC_NOT: case IEEE802154_CMD_DISASSOC_NOT:
break; break;
case IEEE802154_CMD_DATA_REQ: case IEEE802154_CMD_DATA_REQ:
/* Data requests can be sent for 3 different reasons. /* 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: case MAC802154_OP_ASSOC:
mac802154_txdone_datareq_assoc(priv, txdesc); mac802154_txdone_datareq_assoc(priv, txdesc);
break; break;
case MAC802154_OP_POLL: case MAC802154_OP_POLL:
mac802154_txdone_datareq_poll(priv, txdesc); mac802154_txdone_datareq_poll(priv, txdesc);
break; break;
default: default:
break; break;
} }
break; break;
case IEEE802154_CMD_PANID_CONF_NOT: case IEEE802154_CMD_PANID_CONF_NOT:
break; break;
case IEEE802154_CMD_ORPHAN_NOT: case IEEE802154_CMD_ORPHAN_NOT:
break; break;
case IEEE802154_CMD_BEACON_REQ: case IEEE802154_CMD_BEACON_REQ:
break; break;
case IEEE802154_CMD_COORD_REALIGN: case IEEE802154_CMD_COORD_REALIGN:
break; break;
case IEEE802154_CMD_GTS_REQ: case IEEE802154_CMD_GTS_REQ:
break; break;
default: default:
/* We can deallocate the data conf notification as it is no /* We can deallocate the data conf notification as it is no
* longer needed. We can't use the public function here * 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; privnotif->flink = priv->notif_free;
priv->notif_free = privnotif; priv->notif_free = privnotif;
priv->nnotif = 0;
break; break;
} }
} }
break; break;
default: default:
{ {
/* We can deallocate the data conf notification as it is no longer /* 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: case IEEE802154_CMD_ASSOC_REQ:
mac802154_rx_assocreq(priv, ind); mac802154_rx_assocreq(priv, ind);
break; break;
case IEEE802154_CMD_ASSOC_RESP: case IEEE802154_CMD_ASSOC_RESP:
mac802154_rx_assocresp(priv, ind); mac802154_rx_assocresp(priv, ind);
break; break;
case IEEE802154_CMD_DISASSOC_NOT: case IEEE802154_CMD_DISASSOC_NOT:
break; break;
case IEEE802154_CMD_DATA_REQ: case IEEE802154_CMD_DATA_REQ:
mac802154_rx_datareq(priv, ind); mac802154_rx_datareq(priv, ind);
break; break;
case IEEE802154_CMD_PANID_CONF_NOT: case IEEE802154_CMD_PANID_CONF_NOT:
break; break;
case IEEE802154_CMD_ORPHAN_NOT: case IEEE802154_CMD_ORPHAN_NOT:
break; break;
case IEEE802154_CMD_BEACON_REQ: case IEEE802154_CMD_BEACON_REQ:
break; break;
case IEEE802154_CMD_COORD_REALIGN: case IEEE802154_CMD_COORD_REALIGN:
break; break;
case IEEE802154_CMD_GTS_REQ: case IEEE802154_CMD_GTS_REQ:
break; break;
} }
+5 -6
View File
@@ -480,7 +480,7 @@ void mac802154_txdone_assocreq(FAR struct ieee802154_privmac_s *priv,
/* Release the MAC, call the callback, get exclusive access again */ /* Release the MAC, call the callback, get exclusive access again */
mac802154_givesem(&priv->exclsem); mac802154_givesem(&priv->exclsem);
priv->cb->notify(priv->cb, notif); mac802154_notify(priv, notif);
mac802154_takesem(&priv->exclsem, false); mac802154_takesem(&priv->exclsem, false);
} }
else 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 */ /* Release the MAC, call the callback, get exclusive access again */
mac802154_givesem(&priv->exclsem); mac802154_givesem(&priv->exclsem);
priv->cb->notify(priv->cb, notif); mac802154_notify(priv, notif);
mac802154_takesem(&priv->exclsem, false); mac802154_takesem(&priv->exclsem, false);
} }
else else
@@ -708,8 +708,7 @@ void mac802154_rx_assocreq(FAR struct ieee802154_privmac_s *priv,
/* Notify the next highest layer of the association status */ /* Notify the next highest layer of the association status */
priv->cb->notify(priv->cb, notif); mac802154_notify(priv, notif);
return; return;
errout_with_sem: 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 */ /* 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.status = IEEE802154_STATUS_NO_DATA;
notif->u.assocconf.saddr = IEEE802154_SADDR_UNSPEC; notif->u.assocconf.saddr = IEEE802154_SADDR_UNSPEC;
priv->cb->notify(priv->cb, notif); mac802154_notify(priv, notif);
} }
+7
View File
@@ -105,5 +105,12 @@ int mac802154_bind(MACHANDLE mac, FAR struct mac802154_maccb_s *cb)
prev->flink = cb; prev->flink = cb;
} }
/* Keep track of the number of clients requesting notification */
if (cb->notify != NULL)
{
priv->nclients++;
}
return OK; return OK;
} }
+3 -1
View File
@@ -150,7 +150,9 @@ struct ieee802154_privmac_s
FAR struct mac802154_maccb_s *cb; /* Head of a list of MAC callbacks */ FAR struct mac802154_maccb_s *cb; /* Head of a list of MAC callbacks */
FAR struct mac802154_radiocb_s radiocb; /* Interface to bind to radio */ 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 /* 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 * condition where you need to have more than one command frame simultaneously
+4
View File
@@ -244,6 +244,10 @@ static void macnet_notify(FAR struct mac802154_maccb_s *maccb,
default: default:
break; break;
} }
/* Free the event notification */
mac802154_notif_free(priv->md_mac, notif);
} }
/**************************************************************************** /****************************************************************************
+63 -6
View File
@@ -74,18 +74,38 @@ int mac802154_notif_free(MACHANDLE mac,
{ {
FAR struct ieee802154_privmac_s *priv = FAR struct ieee802154_privmac_s *priv =
(FAR struct ieee802154_privmac_s *)mac; (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 */ /* Get exclusive access to the MAC */
mac802154_takesem(&priv->exclsem, false); mac802154_takesem(&priv->exclsem, false);
privnotif->flink = priv->notif_free; /* We know how many clients have registered for notifications. Each must
priv->notif_free = privnotif; * call mac802154_notif_free() before we can release the notification
mac802154_givesem(&priv->notif_sem); * 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); mac802154_givesem(&priv->exclsem);
return -ENOTTY; return -ENOTTY;
} }
@@ -125,6 +145,7 @@ void mac802154_notifpool_init(FAR struct ieee802154_privmac_s *priv)
pool++; pool++;
remaining--; remaining--;
} }
sem_init(&priv->notif_sem, 0, CONFIG_MAC802154_NNOTIF); 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; privnotif = priv->notif_free;
priv->notif_free = privnotif->flink; priv->notif_free = privnotif->flink;
priv->nnotif = 0;
} }
else else
{ {
@@ -200,9 +222,44 @@ int mac802154_notif_alloc(FAR struct ieee802154_privmac_s *priv,
privnotif = priv->notif_free; privnotif = priv->notif_free;
priv->notif_free = privnotif->flink; priv->notif_free = privnotif->flink;
priv->nnotif = 0;
} }
*notif = (FAR struct ieee802154_notif_s *)privnotif; *notif = (FAR struct ieee802154_notif_s *)privnotif;
return OK; return OK;
} }
/****************************************************************************
* 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);
}
}
}
+5 -2
View File
@@ -63,8 +63,8 @@
struct mac802154_notif_s struct mac802154_notif_s
{ {
struct ieee802154_notif_s pub; struct ieee802154_notif_s pub; /* Publically visible structure */
FAR struct mac802154_notif_s *flink; 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, FAR struct ieee802154_notif_s **notif,
bool allow_interrupt); bool allow_interrupt);
void mac802154_notify(FAR struct ieee802154_privmac_s *priv,
FAR struct ieee802154_notif_s *notif);
#endif /* __WIRELESS_IEEE802154__MAC802154_NOTIF_H */ #endif /* __WIRELESS_IEEE802154__MAC802154_NOTIF_H */
+3 -6
View File
@@ -50,8 +50,6 @@
#include <debug.h> #include <debug.h>
#include <string.h> #include <string.h>
#include <nuttx/mm/iob.h>
#include "mac802154.h" #include "mac802154.h"
#include "mac802154_internal.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 *priv =
(FAR struct ieee802154_privmac_s *)mac; (FAR struct ieee802154_privmac_s *)mac;
FAR struct iob_s *iob;
FAR struct ieee802154_txdesc_s *txdesc; FAR struct ieee802154_txdesc_s *txdesc;
int ret; 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 */ /* Release the MAC, call the callback, get exclusive access again */
mac802154_givesem(&priv->exclsem); mac802154_givesem(&priv->exclsem);
priv->cb->notify(priv->cb, notif); mac802154_notify(priv, notif);
mac802154_takesem(&priv->exclsem, false); mac802154_takesem(&priv->exclsem, false);
} }
else 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; 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->notiftype = IEEE802154_NOTIFY_CONF_POLL;
notif->u.pollconf.status = IEEE802154_STATUS_NO_DATA; notif->u.pollconf.status = IEEE802154_STATUS_NO_DATA;
priv->cb->notify(priv->cb, notif); mac802154_notify(priv, notif);
} }