Re: [PATCH net-next] net/ncsi: Refactor MAC, VLAN filters

2018-04-08 Thread David Miller 

The net-next tree is closed at this time, please resend this when the
merge window is over and the net-next tree opens back up.

Thank you.

Re: [PATCH net-next] net/ncsi: Refactor MAC, VLAN filters

2018-04-08 Thread David Miller 

The net-next tree is closed at this time, please resend this when the
merge window is over and the net-next tree opens back up.

Thank you.

[PATCH net-next] net/ncsi: Refactor MAC, VLAN filters

2018-04-08 Thread Samuel Mendoza-Jonas 
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 #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: 0100 94888000 94888fc1 003f 0001 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 
Signed-off-by: Samuel Mendoza-Jonas 
---
 net/ncsi/internal.h |  34 +++---
 net/ncsi/ncsi-manage.c  | 226 +---
 net/ncsi/ncsi-netlink.c |  20 ++--
 net/ncsi/ncsi-rsp.c | 178 +--
 4 files changed, 147 insertions(+), 311 deletions(-)

diff --git a/net/ncsi/internal.h b/net/ncsi/internal.h
index 8da84312cd3b..8055e3965cef 100644
--- a/net/ncsi/internal.h
+++ b/net/ncsi/internal.h
@@ -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 */
@@ -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 {
@@ -186,7 +184,9 @@ struct ncsi_channel {
struct ncsi_channel_version version;
struct ncsi_channel_cap caps[NCSI_CAP_MAX];
struct ncsi_channel_modemodes[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;
@@ -320,10 +320,6 @@ extern spinlock_t ncsi_dev_lock;
list_for_each_entry_rcu(nc, >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

[PATCH net-next] net/ncsi: Refactor MAC, VLAN filters

2018-04-08 Thread Samuel Mendoza-Jonas 
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 #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: 0100 94888000 94888fc1 003f 0001 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 
Signed-off-by: Samuel Mendoza-Jonas 
---
 net/ncsi/internal.h |  34 +++---
 net/ncsi/ncsi-manage.c  | 226 +---
 net/ncsi/ncsi-netlink.c |  20 ++--
 net/ncsi/ncsi-rsp.c | 178 +--
 4 files changed, 147 insertions(+), 311 deletions(-)

diff --git a/net/ncsi/internal.h b/net/ncsi/internal.h
index 8da84312cd3b..8055e3965cef 100644
--- a/net/ncsi/internal.h
+++ b/net/ncsi/internal.h
@@ -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 */
@@ -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 {
@@ -186,7 +184,9 @@ struct ncsi_channel {
struct ncsi_channel_version version;
struct ncsi_channel_cap caps[NCSI_CAP_MAX];
struct ncsi_channel_modemodes[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;
@@ -320,10 +320,6 @@ extern spinlock_t ncsi_dev_lock;
list_for_each_entry_rcu(nc, >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