summaryrefslogtreecommitdiffstats
path: root/include/soc
diff options
context:
space:
mode:
authorVladimir Oltean <vladimir.oltean@nxp.com>2022-05-05 02:54:59 +0300
committerJakub Kicinski <kuba@kernel.org>2022-05-05 19:15:14 -0700
commite1846cff2fe614d93a2f89461b5935678fd34bd9 (patch)
treeab2eea0ffaaa787eeafcecd90d2bf9283af10f3c /include/soc
parent4e707344e18525b4edf5c2bc2e3eb60692e8c92e (diff)
downloadlinux-e1846cff2fe614d93a2f89461b5935678fd34bd9.tar.bz2
net: mscc: ocelot: mark traps with a bool instead of keeping them in a list
Since the blamed commit, VCAP filters can appear on more than one list. If their action is "trap", they are chained on ocelot->traps via filter->trap_list. This is in addition to their normal placement on the VCAP block->rules list head. Therefore, when we free a VCAP filter, we must remove it from all lists it is a member of, including ocelot->traps. There are at least 2 bugs which are direct consequences of this design decision. First is the incorrect usage of list_empty(), meant to denote whether "filter" is chained into ocelot->traps via filter->trap_list. This does not do the correct thing, because list_empty() checks whether "head->next == head", but in our case, head->next == head->prev == NULL. So we dereference NULL pointers and die when we call list_del(). Second is the fact that not all places that should remove the filter from ocelot->traps do so. One example is ocelot_vcap_block_remove_filter(), which is where we have the main kfree(filter). By keeping freed filters in ocelot->traps we end up in a use-after-free in felix_update_trapping_destinations(). Attempting to fix all the buggy patterns is a whack-a-mole game which makes the driver unmaintainable. Actually this is what the previous patch version attempted to do: https://patchwork.kernel.org/project/netdevbpf/patch/20220503115728.834457-3-vladimir.oltean@nxp.com/ but it introduced another set of bugs, because there are other places in which create VCAP filters, not just ocelot_vcap_filter_create(): - ocelot_trap_add() - felix_tag_8021q_vlan_add_rx() - felix_tag_8021q_vlan_add_tx() Relying on the convention that all those code paths must call INIT_LIST_HEAD(&filter->trap_list) is not going to scale. So let's do what should have been done in the first place and keep a bool in struct ocelot_vcap_filter which denotes whether we are looking at a trapping rule or not. Iterating now happens over the main VCAP IS2 block->rules. The advantage is that we no longer risk having stale references to a freed filter, since it is only present in that list. Fixes: e42bd4ed09aa ("net: mscc: ocelot: keep traps in a list") Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com> Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Diffstat (limited to 'include/soc')
-rw-r--r--include/soc/mscc/ocelot_vcap.h2
1 files changed, 1 insertions, 1 deletions
diff --git a/include/soc/mscc/ocelot_vcap.h b/include/soc/mscc/ocelot_vcap.h
index 7b2bf9b1fe69..de26c992f821 100644
--- a/include/soc/mscc/ocelot_vcap.h
+++ b/include/soc/mscc/ocelot_vcap.h
@@ -681,7 +681,6 @@ struct ocelot_vcap_id {
struct ocelot_vcap_filter {
struct list_head list;
- struct list_head trap_list;
enum ocelot_vcap_filter_type type;
int block_id;
@@ -695,6 +694,7 @@ struct ocelot_vcap_filter {
struct ocelot_vcap_stats stats;
/* For VCAP IS1 and IS2 */
bool take_ts;
+ bool is_trap;
unsigned long ingress_port_mask;
/* For VCAP ES0 */
struct ocelot_vcap_port ingress_port;