netdev: Make sure set_powered calls are cancelable

Since all netdevs share the rtnl l_netlink object, it was possible for
netdevs to be destroyed with outstanding commands still executing on the
rtnl object.  This can lead to crashes and other nasty situations.

This patch makes sure that Powered requests are always tracked via
set_powered_cmd_id and the request is canceled when netdev is destroyed.
This also implies that netdev_set_powered can now return an -EBUSY error
in case a request is already outstanding.
This commit is contained in:
Denis Kenzior 2018-08-17 12:37:47 -05:00
parent c530667ed1
commit 015e8625bf
1 changed files with 79 additions and 55 deletions

View File

@ -104,6 +104,11 @@ struct netdev {
struct l_timeout *rssi_poll_timeout; struct l_timeout *rssi_poll_timeout;
uint32_t rssi_poll_cmd_id; uint32_t rssi_poll_cmd_id;
uint32_t set_powered_cmd_id;
netdev_set_powered_cb_t set_powered_cb;
void *set_powered_user_data;
netdev_destroy_func_t set_powered_destroy;
struct watchlist event_watches; struct watchlist event_watches;
struct watchlist frame_watches; struct watchlist frame_watches;
@ -305,45 +310,39 @@ struct handshake_state *netdev_get_handshake(struct netdev *netdev)
return netdev->handshake; return netdev->handshake;
} }
struct set_powered_cb_data {
struct netdev *netdev;
netdev_set_powered_cb_t callback;
void *user_data;
l_netlink_destroy_func_t destroy;
};
static void netdev_set_powered_result(int error, uint16_t type, static void netdev_set_powered_result(int error, uint16_t type,
const void *data, const void *data,
uint32_t len, void *user_data) uint32_t len, void *user_data)
{ {
struct set_powered_cb_data *cb_data = user_data; struct netdev *netdev = user_data;
if (!cb_data) if (netdev->set_powered_cb)
return; netdev->set_powered_cb(netdev, error,
netdev->set_powered_user_data);
cb_data->callback(cb_data->netdev, error, cb_data->user_data); netdev->set_powered_cb = NULL;
} }
static void netdev_set_powered_destroy(void *user_data) static void netdev_set_powered_destroy(void *user_data)
{ {
struct set_powered_cb_data *cb_data = user_data; struct netdev *netdev = user_data;
if (!cb_data) netdev->set_powered_cmd_id = 0;
return;
if (cb_data->destroy) if (netdev->set_powered_destroy)
cb_data->destroy(cb_data->user_data); netdev->set_powered_destroy(netdev->set_powered_user_data);
l_free(cb_data); netdev->set_powered_destroy = NULL;
netdev->set_powered_user_data = NULL;
} }
int netdev_set_powered(struct netdev *netdev, bool powered, static uint32_t rtnl_set_powered(int ifindex, bool powered,
netdev_set_powered_cb_t callback, void *user_data, l_netlink_command_func_t cb, void *user_data,
netdev_destroy_func_t destroy) l_netlink_destroy_func_t destroy)
{ {
struct ifinfomsg *rtmmsg; struct ifinfomsg *rtmmsg;
size_t bufsize; size_t bufsize;
struct set_powered_cb_data *cb_data = NULL; uint32_t id;
bufsize = NLMSG_ALIGN(sizeof(struct ifinfomsg)); bufsize = NLMSG_ALIGN(sizeof(struct ifinfomsg));
@ -351,24 +350,34 @@ int netdev_set_powered(struct netdev *netdev, bool powered,
memset(rtmmsg, 0, bufsize); memset(rtmmsg, 0, bufsize);
rtmmsg->ifi_family = AF_UNSPEC; rtmmsg->ifi_family = AF_UNSPEC;
rtmmsg->ifi_index = netdev->index; rtmmsg->ifi_index = ifindex;
rtmmsg->ifi_change = 0xffffffff; rtmmsg->ifi_change = IFF_UP;
rtmmsg->ifi_flags = powered ? (netdev->ifi_flags | IFF_UP) : rtmmsg->ifi_flags = powered ? IFF_UP : 0;
(netdev->ifi_flags & ~IFF_UP);
if (callback) { id = l_netlink_send(rtnl, RTM_SETLINK, 0, rtmmsg, bufsize,
cb_data = l_new(struct set_powered_cb_data, 1); cb, user_data, destroy);
cb_data->netdev = netdev;
cb_data->callback = callback;
cb_data->user_data = user_data;
cb_data->destroy = destroy;
}
l_netlink_send(rtnl, RTM_SETLINK, 0, rtmmsg, bufsize,
netdev_set_powered_result, cb_data,
netdev_set_powered_destroy);
l_free(rtmmsg); l_free(rtmmsg);
return id;
}
int netdev_set_powered(struct netdev *netdev, bool powered,
netdev_set_powered_cb_t callback, void *user_data,
netdev_destroy_func_t destroy)
{
if (netdev->set_powered_cmd_id)
return -EBUSY;
netdev->set_powered_cmd_id =
rtnl_set_powered(netdev->index, powered,
netdev_set_powered_result, netdev,
netdev_set_powered_destroy);
if (!netdev->set_powered_cmd_id)
return -EIO;
netdev->set_powered_cb = callback;
netdev->set_powered_user_data = user_data;
netdev->set_powered_destroy = destroy;
return 0; return 0;
} }
@ -608,6 +617,11 @@ static void netdev_free(void *data)
netdev->leave_adhoc_cmd_id = 0; netdev->leave_adhoc_cmd_id = 0;
} }
if (netdev->set_powered_cmd_id) {
l_netlink_cancel(rtnl, netdev->set_powered_cmd_id);
netdev->set_powered_cmd_id = 0;
}
device_remove(netdev->device); device_remove(netdev->device);
watchlist_destroy(&netdev->event_watches); watchlist_destroy(&netdev->event_watches);
watchlist_destroy(&netdev->frame_watches); watchlist_destroy(&netdev->frame_watches);
@ -626,7 +640,7 @@ static void netdev_shutdown_one(void *data, void *user_data)
netdev_set_iftype(netdev, NETDEV_IFTYPE_STATION); netdev_set_iftype(netdev, NETDEV_IFTYPE_STATION);
if (netdev_get_is_up(netdev)) if (netdev_get_is_up(netdev))
netdev_set_powered(netdev, false, NULL, NULL, NULL); rtnl_set_powered(netdev->index, false, NULL, NULL, NULL);
} }
static bool netdev_match(const void *a, const void *b) static bool netdev_match(const void *a, const void *b)
@ -4037,14 +4051,18 @@ static void netdev_dellink_notify(const struct ifinfomsg *ifi, int bytes)
netdev_free(netdev); netdev_free(netdev);
} }
static void netdev_initial_up_cb(struct netdev *netdev, int result, static void netdev_initial_up_cb(int error, uint16_t type, const void *data,
void *user_data) uint32_t len, void *user_data)
{ {
if (result != 0) { struct netdev *netdev = user_data;
l_error("Error bringing interface %i up: %s", netdev->index,
strerror(-result));
if (result != -ERFKILL) netdev->set_powered_cmd_id = 0;
if (error != 0) {
l_error("Error bringing interface %i up: %s", netdev->index,
strerror(-error));
if (error != -ERFKILL)
return; return;
} }
@ -4064,18 +4082,22 @@ static void netdev_initial_up_cb(struct netdev *netdev, int result,
netdev->device = device_create(netdev->wiphy, netdev); netdev->device = device_create(netdev->wiphy, netdev);
} }
static void netdev_initial_down_cb(struct netdev *netdev, int result, static void netdev_initial_down_cb(int error, uint16_t type, const void *data,
void *user_data) uint32_t len, void *user_data)
{ {
if (result != 0) { struct netdev *netdev = user_data;
l_error("Error taking interface %i down: %s", netdev->index,
strerror(-result));
if (error != 0) {
l_error("Error taking interface %i down: %s", netdev->index,
strerror(-error));
netdev->set_powered_cmd_id = 0;
return; return;
} }
netdev_set_powered(netdev, true, netdev_initial_up_cb, netdev->set_powered_cmd_id =
NULL, NULL); rtnl_set_powered(netdev->index, true, netdev_initial_up_cb,
netdev, NULL);
} }
static void netdev_getlink_cb(int error, uint16_t type, const void *data, static void netdev_getlink_cb(int error, uint16_t type, const void *data,
@ -4084,6 +4106,8 @@ static void netdev_getlink_cb(int error, uint16_t type, const void *data,
const struct ifinfomsg *ifi = data; const struct ifinfomsg *ifi = data;
unsigned int bytes; unsigned int bytes;
struct netdev *netdev; struct netdev *netdev;
l_netlink_command_func_t cb;
bool powered;
if (error != 0 || ifi->ifi_type != ARPHRD_ETHER || if (error != 0 || ifi->ifi_type != ARPHRD_ETHER ||
type != RTM_NEWLINK) { type != RTM_NEWLINK) {
@ -4104,11 +4128,11 @@ static void netdev_getlink_cb(int error, uint16_t type, const void *data,
* If the interface is UP, reset it to ensure a clean state, * If the interface is UP, reset it to ensure a clean state,
* otherwise just bring it UP. * otherwise just bring it UP.
*/ */
if (netdev_get_is_up(netdev)) { powered = netdev_get_is_up(netdev);
netdev_set_powered(netdev, false, netdev_initial_down_cb, cb = powered ? netdev_initial_down_cb : netdev_initial_up_cb;
NULL, NULL);
} else netdev->set_powered_cmd_id =
netdev_initial_down_cb(netdev, 0, NULL); rtnl_set_powered(ifi->ifi_index, !powered, cb, netdev, NULL);
} }
static bool netdev_is_managed(const char *ifname) static bool netdev_is_managed(const char *ifname)