Commit 5ff3fda9 authored by Jakub Kicinski's avatar Jakub Kicinski

Merge branch 'net-sysfs-fix-race-conditions-in-the-xps-code'

Antoine Tenart says:

====================
net-sysfs: fix race conditions in the xps code

This series fixes race conditions in the xps code, where out of bound
accesses can occur when dev->num_tc is updated, triggering oops. The
root cause is linked to locking issues. An explanation is given in each
of the commit logs.

We had a discussion on the v1 of this series about using the xps_map
mutex instead of the rtnl lock. While that seemed a better compromise,
v2 showed the added complexity wasn't best for fixes. So we decided to
go back to v1 and use the rtnl lock.

Because of this, the only differences between v1 and v3 are improvements
in the commit messages.
====================

Link: https://lore.kernel.org/r/20201223212323.3603139-1-atenart@kernel.orgSigned-off-by: default avatarJakub Kicinski <kuba@kernel.org>
parents 59b4a8fa 4ae2bb81
...@@ -1317,8 +1317,8 @@ static const struct attribute_group dql_group = { ...@@ -1317,8 +1317,8 @@ static const struct attribute_group dql_group = {
static ssize_t xps_cpus_show(struct netdev_queue *queue, static ssize_t xps_cpus_show(struct netdev_queue *queue,
char *buf) char *buf)
{ {
int cpu, len, ret, num_tc = 1, tc = 0;
struct net_device *dev = queue->dev; struct net_device *dev = queue->dev;
int cpu, len, num_tc = 1, tc = 0;
struct xps_dev_maps *dev_maps; struct xps_dev_maps *dev_maps;
cpumask_var_t mask; cpumask_var_t mask;
unsigned long index; unsigned long index;
...@@ -1328,22 +1328,31 @@ static ssize_t xps_cpus_show(struct netdev_queue *queue, ...@@ -1328,22 +1328,31 @@ static ssize_t xps_cpus_show(struct netdev_queue *queue,
index = get_netdev_queue_index(queue); index = get_netdev_queue_index(queue);
if (!rtnl_trylock())
return restart_syscall();
if (dev->num_tc) { if (dev->num_tc) {
/* Do not allow XPS on subordinate device directly */ /* Do not allow XPS on subordinate device directly */
num_tc = dev->num_tc; num_tc = dev->num_tc;
if (num_tc < 0) if (num_tc < 0) {
return -EINVAL; ret = -EINVAL;
goto err_rtnl_unlock;
}
/* If queue belongs to subordinate dev use its map */ /* If queue belongs to subordinate dev use its map */
dev = netdev_get_tx_queue(dev, index)->sb_dev ? : dev; dev = netdev_get_tx_queue(dev, index)->sb_dev ? : dev;
tc = netdev_txq_to_tc(dev, index); tc = netdev_txq_to_tc(dev, index);
if (tc < 0) if (tc < 0) {
return -EINVAL; ret = -EINVAL;
goto err_rtnl_unlock;
}
} }
if (!zalloc_cpumask_var(&mask, GFP_KERNEL)) if (!zalloc_cpumask_var(&mask, GFP_KERNEL)) {
return -ENOMEM; ret = -ENOMEM;
goto err_rtnl_unlock;
}
rcu_read_lock(); rcu_read_lock();
dev_maps = rcu_dereference(dev->xps_cpus_map); dev_maps = rcu_dereference(dev->xps_cpus_map);
...@@ -1366,9 +1375,15 @@ static ssize_t xps_cpus_show(struct netdev_queue *queue, ...@@ -1366,9 +1375,15 @@ static ssize_t xps_cpus_show(struct netdev_queue *queue,
} }
rcu_read_unlock(); rcu_read_unlock();
rtnl_unlock();
len = snprintf(buf, PAGE_SIZE, "%*pb\n", cpumask_pr_args(mask)); len = snprintf(buf, PAGE_SIZE, "%*pb\n", cpumask_pr_args(mask));
free_cpumask_var(mask); free_cpumask_var(mask);
return len < PAGE_SIZE ? len : -EINVAL; return len < PAGE_SIZE ? len : -EINVAL;
err_rtnl_unlock:
rtnl_unlock();
return ret;
} }
static ssize_t xps_cpus_store(struct netdev_queue *queue, static ssize_t xps_cpus_store(struct netdev_queue *queue,
...@@ -1396,7 +1411,13 @@ static ssize_t xps_cpus_store(struct netdev_queue *queue, ...@@ -1396,7 +1411,13 @@ static ssize_t xps_cpus_store(struct netdev_queue *queue,
return err; return err;
} }
if (!rtnl_trylock()) {
free_cpumask_var(mask);
return restart_syscall();
}
err = netif_set_xps_queue(dev, mask, index); err = netif_set_xps_queue(dev, mask, index);
rtnl_unlock();
free_cpumask_var(mask); free_cpumask_var(mask);
...@@ -1408,22 +1429,29 @@ static struct netdev_queue_attribute xps_cpus_attribute __ro_after_init ...@@ -1408,22 +1429,29 @@ static struct netdev_queue_attribute xps_cpus_attribute __ro_after_init
static ssize_t xps_rxqs_show(struct netdev_queue *queue, char *buf) static ssize_t xps_rxqs_show(struct netdev_queue *queue, char *buf)
{ {
int j, len, ret, num_tc = 1, tc = 0;
struct net_device *dev = queue->dev; struct net_device *dev = queue->dev;
struct xps_dev_maps *dev_maps; struct xps_dev_maps *dev_maps;
unsigned long *mask, index; unsigned long *mask, index;
int j, len, num_tc = 1, tc = 0;
index = get_netdev_queue_index(queue); index = get_netdev_queue_index(queue);
if (!rtnl_trylock())
return restart_syscall();
if (dev->num_tc) { if (dev->num_tc) {
num_tc = dev->num_tc; num_tc = dev->num_tc;
tc = netdev_txq_to_tc(dev, index); tc = netdev_txq_to_tc(dev, index);
if (tc < 0) if (tc < 0) {
return -EINVAL; ret = -EINVAL;
goto err_rtnl_unlock;
}
} }
mask = bitmap_zalloc(dev->num_rx_queues, GFP_KERNEL); mask = bitmap_zalloc(dev->num_rx_queues, GFP_KERNEL);
if (!mask) if (!mask) {
return -ENOMEM; ret = -ENOMEM;
goto err_rtnl_unlock;
}
rcu_read_lock(); rcu_read_lock();
dev_maps = rcu_dereference(dev->xps_rxqs_map); dev_maps = rcu_dereference(dev->xps_rxqs_map);
...@@ -1449,10 +1477,16 @@ static ssize_t xps_rxqs_show(struct netdev_queue *queue, char *buf) ...@@ -1449,10 +1477,16 @@ static ssize_t xps_rxqs_show(struct netdev_queue *queue, char *buf)
out_no_maps: out_no_maps:
rcu_read_unlock(); rcu_read_unlock();
rtnl_unlock();
len = bitmap_print_to_pagebuf(false, buf, mask, dev->num_rx_queues); len = bitmap_print_to_pagebuf(false, buf, mask, dev->num_rx_queues);
bitmap_free(mask); bitmap_free(mask);
return len < PAGE_SIZE ? len : -EINVAL; return len < PAGE_SIZE ? len : -EINVAL;
err_rtnl_unlock:
rtnl_unlock();
return ret;
} }
static ssize_t xps_rxqs_store(struct netdev_queue *queue, const char *buf, static ssize_t xps_rxqs_store(struct netdev_queue *queue, const char *buf,
...@@ -1478,10 +1512,17 @@ static ssize_t xps_rxqs_store(struct netdev_queue *queue, const char *buf, ...@@ -1478,10 +1512,17 @@ static ssize_t xps_rxqs_store(struct netdev_queue *queue, const char *buf,
return err; return err;
} }
if (!rtnl_trylock()) {
bitmap_free(mask);
return restart_syscall();
}
cpus_read_lock(); cpus_read_lock();
err = __netif_set_xps_queue(dev, mask, index, true); err = __netif_set_xps_queue(dev, mask, index, true);
cpus_read_unlock(); cpus_read_unlock();
rtnl_unlock();
bitmap_free(mask); bitmap_free(mask);
return err ? : len; return err ? : len;
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment