Skip to content

Commit

Permalink
net/ncsi: Refactor MAC, VLAN filters
Browse files Browse the repository at this point in the history 
The NCSI driver defines a generic ncsi_channel_filter struct that can be
used to store arbitrarily formatted filters, and several generic methods
of accessing data stored in such a filter.
However in both the driver and as defined in the NCSI specification
there are only two actual filters: VLAN ID filters and MAC address
filters. The splitting of the MAC filter into unicast, multicast, and
mixed is also technically not necessary as these are stored in the same
location in hardware.

To save complexity, particularly in the set up and accessing of these
generic filters, remove them in favour of two specific structs. These
can be acted on directly and do not need several generic helper
functions to use.

This also fixes a memory error found by KASAN on ARM32 (which is not
upstream yet), where response handlers accessing a filter's data field
could write past allocated memory.

[  114.926512] ==================================================================
[  114.933861] BUG: KASAN: slab-out-of-bounds in ncsi_configure_channel+0x4b8/0xc58
[  114.941304] Read of size 2 at addr 94888558 by task kworker/0:2/546
[  114.947593]
[  114.949146] CPU: 0 PID: 546 Comm: kworker/0:2 Not tainted 4.16.0-rc6-00119-ge156398bfcad openbmc#13
...
[  115.170233] The buggy address belongs to the object at 94888540
[  115.170233]  which belongs to the cache kmalloc-32 of size 32
[  115.181917] The buggy address is located 24 bytes inside of
[  115.181917]  32-byte region [94888540, 94888560)
[  115.192115] The buggy address belongs to the page:
[  115.196943] page:9eeac100 count:1 mapcount:0 mapping:94888000 index:0x94888fc1
[  115.204200] flags: 0x100(slab)
[  115.207330] raw: 00000100 94888000 94888fc1 0000003f 00000001 9eea2014 9eecaa74 96c003e0
[  115.215444] page dumped because: kasan: bad access detected
[  115.221036]
[  115.222544] Memory state around the buggy address:
[  115.227384]  94888400: fb fb fb fb fc fc fc fc 04 fc fc fc fc fc fc fc
[  115.233959]  94888480: 00 00 00 fc fc fc fc fc 00 04 fc fc fc fc fc fc
[  115.240529] >94888500: 00 00 04 fc fc fc fc fc 00 00 04 fc fc fc fc fc
[  115.247077]                                             ^
[  115.252523]  94888580: 00 04 fc fc fc fc fc fc 06 fc fc fc fc fc fc fc
[  115.259093]  94888600: 00 00 06 fc fc fc fc fc 00 00 04 fc fc fc fc fc
[  115.265639] ==================================================================

Reported-by: Joel Stanley <joel@jms.id.au>
Signed-off-by: Samuel Mendoza-Jonas <sam@mendozajonas.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
  • Loading branch information
@sammj @davem330
sammj authored and davem330 committed Apr 17, 2018
1 parent c210f7b commit 062b3e1
Show file tree
Hide file tree
Showing 4 changed files with 147 additions and 311 deletions.
34 changes: 15 additions & 19 deletions net/ncsi/internal.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -68,15 +68,6 @@ enum {
NCSI_MODE_MAX
};

enum {
NCSI_FILTER_BASE = 0,
NCSI_FILTER_VLAN = 0,
NCSI_FILTER_UC,
NCSI_FILTER_MC,
NCSI_FILTER_MIXED,
NCSI_FILTER_MAX
};

struct ncsi_channel_version {
u32 version; /* Supported BCD encoded NCSI version */
u32 alpha2; /* Supported BCD encoded NCSI version */
Expand All @@ -98,11 +89,18 @@ struct ncsi_channel_mode {
u32 data[8]; /* Data entries */
};

struct ncsi_channel_filter {
u32 index; /* Index of channel filters */
u32 total; /* Total entries in the filter table */
u64 bitmap; /* Bitmap of valid entries */
u32 data[]; /* Data for the valid entries */
struct ncsi_channel_mac_filter {
u8 n_uc;
u8 n_mc;
u8 n_mixed;
u64 bitmap;
unsigned char *addrs;
};

struct ncsi_channel_vlan_filter {
u8 n_vids;
u64 bitmap;
u16 *vids;
};

struct ncsi_channel_stats {
Expand Down Expand Up @@ -186,7 +184,9 @@ struct ncsi_channel {
struct ncsi_channel_version version;
struct ncsi_channel_cap caps[NCSI_CAP_MAX];
struct ncsi_channel_mode modes[NCSI_MODE_MAX];
struct ncsi_channel_filter *filters[NCSI_FILTER_MAX];
/* Filtering Settings */
struct ncsi_channel_mac_filter mac_filter;
struct ncsi_channel_vlan_filter vlan_filter;
struct ncsi_channel_stats stats;
struct {
struct timer_list timer;
Expand Down Expand Up @@ -320,10 +320,6 @@ extern spinlock_t ncsi_dev_lock;
list_for_each_entry_rcu(nc, &np->channels, node)

/* Resources */
u32 *ncsi_get_filter(struct ncsi_channel *nc, int table, int index);
int ncsi_find_filter(struct ncsi_channel *nc, int table, void *data);
int ncsi_add_filter(struct ncsi_channel *nc, int table, void *data);
int ncsi_remove_filter(struct ncsi_channel *nc, int table, int index);
void ncsi_start_channel_monitor(struct ncsi_channel *nc);
void ncsi_stop_channel_monitor(struct ncsi_channel *nc);
struct ncsi_channel *ncsi_find_channel(struct ncsi_package *np,
Expand Down
226 changes: 52 additions & 174 deletions net/ncsi/ncsi-manage.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,125 +27,6 @@
LIST_HEAD(ncsi_dev_list);
DEFINE_SPINLOCK(ncsi_dev_lock);

static inline int ncsi_filter_size(int table)
{
int sizes[] = { 2, 6, 6, 6 };

BUILD_BUG_ON(ARRAY_SIZE(sizes) != NCSI_FILTER_MAX);
if (table < NCSI_FILTER_BASE || table >= NCSI_FILTER_MAX)
return -EINVAL;

return sizes[table];
}

u32 *ncsi_get_filter(struct ncsi_channel *nc, int table, int index)
{
struct ncsi_channel_filter *ncf;
int size;

ncf = nc->filters[table];
if (!ncf)
return NULL;

size = ncsi_filter_size(table);
if (size < 0)
return NULL;

return ncf->data + size * index;
}

/* Find the first active filter in a filter table that matches the given
* data parameter. If data is NULL, this returns the first active filter.
*/
int ncsi_find_filter(struct ncsi_channel *nc, int table, void *data)
{
struct ncsi_channel_filter *ncf;
void *bitmap;
int index, size;
unsigned long flags;

ncf = nc->filters[table];
if (!ncf)
return -ENXIO;

size = ncsi_filter_size(table);
if (size < 0)
return size;

spin_lock_irqsave(&nc->lock, flags);
bitmap = (void *)&ncf->bitmap;
index = -1;
while ((index = find_next_bit(bitmap, ncf->total, index + 1))
< ncf->total) {
if (!data || !memcmp(ncf->data + size * index, data, size)) {
spin_unlock_irqrestore(&nc->lock, flags);
return index;
}
}
spin_unlock_irqrestore(&nc->lock, flags);

return -ENOENT;
}

int ncsi_add_filter(struct ncsi_channel *nc, int table, void *data)
{
struct ncsi_channel_filter *ncf;
int index, size;
void *bitmap;
unsigned long flags;

size = ncsi_filter_size(table);
if (size < 0)
return size;

index = ncsi_find_filter(nc, table, data);
if (index >= 0)
return index;

ncf = nc->filters[table];
if (!ncf)
return -ENODEV;

spin_lock_irqsave(&nc->lock, flags);
bitmap = (void *)&ncf->bitmap;
do {
index = find_next_zero_bit(bitmap, ncf->total, 0);
if (index >= ncf->total) {
spin_unlock_irqrestore(&nc->lock, flags);
return -ENOSPC;
}
} while (test_and_set_bit(index, bitmap));

memcpy(ncf->data + size * index, data, size);
spin_unlock_irqrestore(&nc->lock, flags);

return index;
}

int ncsi_remove_filter(struct ncsi_channel *nc, int table, int index)
{
struct ncsi_channel_filter *ncf;
int size;
void *bitmap;
unsigned long flags;

size = ncsi_filter_size(table);
if (size < 0)
return size;

ncf = nc->filters[table];
if (!ncf || index >= ncf->total)
return -ENODEV;

spin_lock_irqsave(&nc->lock, flags);
bitmap = (void *)&ncf->bitmap;
if (test_and_clear_bit(index, bitmap))
memset(ncf->data + size * index, 0, size);
spin_unlock_irqrestore(&nc->lock, flags);

return 0;
}

static void ncsi_report_link(struct ncsi_dev_priv *ndp, bool force_down)
{
struct ncsi_dev *nd = &ndp->ndev;
Expand Down Expand Up @@ -339,20 +220,13 @@ struct ncsi_channel *ncsi_add_channel(struct ncsi_package *np, unsigned char id)
static void ncsi_remove_channel(struct ncsi_channel *nc)
{
struct ncsi_package *np = nc->package;
struct ncsi_channel_filter *ncf;
unsigned long flags;
int i;

/* Release filters */
spin_lock_irqsave(&nc->lock, flags);
for (i = 0; i < NCSI_FILTER_MAX; i++) {
ncf = nc->filters[i];
if (!ncf)
continue;

nc->filters[i] = NULL;
kfree(ncf);
}
/* Release filters */
kfree(nc->mac_filter.addrs);
kfree(nc->vlan_filter.vids);

nc->state = NCSI_CHANNEL_INACTIVE;
spin_unlock_irqrestore(&nc->lock, flags);
Expand Down Expand Up @@ -670,32 +544,26 @@ static void ncsi_suspend_channel(struct ncsi_dev_priv *ndp)
static int clear_one_vid(struct ncsi_dev_priv *ndp, struct ncsi_channel *nc,
struct ncsi_cmd_arg *nca)
{
struct ncsi_channel_vlan_filter *ncf;
unsigned long flags;
void *bitmap;
int index;
u32 *data;
u16 vid;

index = ncsi_find_filter(nc, NCSI_FILTER_VLAN, NULL);
if (index < 0) {
/* Filter table empty */
return -1;
}
ncf = &nc->vlan_filter;
bitmap = &ncf->bitmap;

data = ncsi_get_filter(nc, NCSI_FILTER_VLAN, index);
if (!data) {
netdev_err(ndp->ndev.dev,
"NCSI: failed to retrieve filter %d\n", index);
/* Set the VLAN id to 0 - this will still disable the entry in
* the filter table, but we won't know what it was.
*/
vid = 0;
} else {
vid = *(u16 *)data;
spin_lock_irqsave(&nc->lock, flags);
index = find_next_bit(bitmap, ncf->n_vids, 0);
if (index >= ncf->n_vids) {
spin_unlock_irqrestore(&nc->lock, flags);
return -1;
}
vid = ncf->vids[index];

netdev_printk(KERN_DEBUG, ndp->ndev.dev,
"NCSI: removed vlan tag %u at index %d\n",
vid, index + 1);
ncsi_remove_filter(nc, NCSI_FILTER_VLAN, index);
clear_bit(index, bitmap);
ncf->vids[index] = 0;
spin_unlock_irqrestore(&nc->lock, flags);

nca->type = NCSI_PKT_CMD_SVF;
nca->words[1] = vid;
Expand All @@ -711,45 +579,55 @@ static int clear_one_vid(struct ncsi_dev_priv *ndp, struct ncsi_channel *nc,
static int set_one_vid(struct ncsi_dev_priv *ndp, struct ncsi_channel *nc,
struct ncsi_cmd_arg *nca)
{
struct ncsi_channel_vlan_filter *ncf;
struct vlan_vid *vlan = NULL;
int index = 0;
unsigned long flags;
int i, index;
void *bitmap;
u16 vid;

if (list_empty(&ndp->vlan_vids))
return -1;

ncf = &nc->vlan_filter;
bitmap = &ncf->bitmap;

spin_lock_irqsave(&nc->lock, flags);

rcu_read_lock();
list_for_each_entry_rcu(vlan, &ndp->vlan_vids, list) {
index = ncsi_find_filter(nc, NCSI_FILTER_VLAN, &vlan->vid);
if (index < 0) {
/* New tag to add */
netdev_printk(KERN_DEBUG, ndp->ndev.dev,
"NCSI: new vlan id to set: %u\n",
vlan->vid);
vid = vlan->vid;
for (i = 0; i < ncf->n_vids; i++)
if (ncf->vids[i] == vid) {
vid = 0;
break;
}
if (vid)
break;
}
netdev_printk(KERN_DEBUG, ndp->ndev.dev,
"vid %u already at filter pos %d\n",
vlan->vid, index);
}
rcu_read_unlock();

if (!vlan || index >= 0) {
netdev_printk(KERN_DEBUG, ndp->ndev.dev,
"no vlan ids left to set\n");
if (!vid) {
/* No VLAN ID is not set */
spin_unlock_irqrestore(&nc->lock, flags);
return -1;
}

index = ncsi_add_filter(nc, NCSI_FILTER_VLAN, &vlan->vid);
if (index < 0) {
index = find_next_zero_bit(bitmap, ncf->n_vids, 0);
if (index < 0 || index >= ncf->n_vids) {
netdev_err(ndp->ndev.dev,
"Failed to add new VLAN tag, error %d\n", index);
if (index == -ENOSPC)
netdev_err(ndp->ndev.dev,
"Channel %u already has all VLAN filters set\n",
nc->id);
"Channel %u already has all VLAN filters set\n",
nc->id);
spin_unlock_irqrestore(&nc->lock, flags);
return -1;
}

netdev_printk(KERN_DEBUG, ndp->ndev.dev,
"NCSI: set vid %u in packet, index %u\n",
vlan->vid, index + 1);
ncf->vids[index] = vid;
set_bit(index, bitmap);
spin_unlock_irqrestore(&nc->lock, flags);

nca->type = NCSI_PKT_CMD_SVF;
nca->words[1] = vlan->vid;
nca->words[1] = vid;
/* HW filter index starts at 1 */
nca->bytes[6] = index + 1;
nca->bytes[7] = 0x01;
Expand Down
20 changes: 7 additions & 13 deletions net/ncsi/ncsi-netlink.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -58,10 +58,9 @@ static int ncsi_write_channel_info(struct sk_buff *skb,
struct ncsi_dev_priv *ndp,
struct ncsi_channel *nc)
{
struct nlattr *vid_nest;
struct ncsi_channel_filter *ncf;
struct ncsi_channel_vlan_filter *ncf;
struct ncsi_channel_mode *m;
u32 *data;
struct nlattr *vid_nest;
int i;

nla_put_u32(skb, NCSI_CHANNEL_ATTR_ID, nc->id);
Expand All @@ -79,18 +78,13 @@ static int ncsi_write_channel_info(struct sk_buff *skb,
vid_nest = nla_nest_start(skb, NCSI_CHANNEL_ATTR_VLAN_LIST);
if (!vid_nest)
return -ENOMEM;
ncf = nc->filters[NCSI_FILTER_VLAN];
ncf = &nc->vlan_filter;
i = -1;
if (ncf) {
while ((i = find_next_bit((void *)&ncf->bitmap, ncf->total,
i + 1)) < ncf->total) {
data = ncsi_get_filter(nc, NCSI_FILTER_VLAN, i);
/* Uninitialised channels will have 'zero' vlan ids */
if (!data || !*data)
continue;
while ((i = find_next_bit((void *)&ncf->bitmap, ncf->n_vids,
i + 1)) < ncf->n_vids) {
if (ncf->vids[i])
nla_put_u16(skb, NCSI_CHANNEL_ATTR_VLAN_ID,
*(u16 *)data);
}
ncf->vids[i]);
}
nla_nest_end(skb, vid_nest);

Expand Down
Loading

0 comments on commit 062b3e1

Please sign in to comment.