| Message ID | 20220503115728.834457-2-vladimir.oltean@nxp.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | Ocelot VCAP fixes | expand |
| Context | Check | Description |
|---|---|---|
| netdev/tree_selection | success | Clearly marked for net |
| netdev/fixes_present | success | Fixes tag present in non-next series |
| netdev/subject_prefix | success | Link |
| netdev/cover_letter | success | Series has a cover letter |
| netdev/patch_count | success | Link |
| netdev/header_inline | success | No static functions without inline keyword in header files |
| netdev/build_32bit | success | Errors and warnings before: 0 this patch: 0 |
| netdev/cc_maintainers | success | CCed 9 of 9 maintainers |
| netdev/build_clang | success | Errors and warnings before: 0 this patch: 0 |
| netdev/module_param | success | Was 0 now: 0 |
| netdev/verify_signedoff | success | Signed-off-by tag matches author and committer |
| netdev/verify_fixes | success | Fixes tag looks correct |
| netdev/build_allmodconfig_warn | success | Errors and warnings before: 0 this patch: 0 |
| netdev/checkpatch | success | total: 0 errors, 0 warnings, 0 checks, 11 lines checked |
| netdev/kdoc | success | Errors and warnings before: 0 this patch: 0 |
| netdev/source_inline | success | Was 0 now: 0 |
diff --git a/drivers/net/ethernet/mscc/ocelot_flower.c b/drivers/net/ethernet/mscc/ocelot_flower.c index 03b5e59d033e..b8617e940063 100644 --- a/drivers/net/ethernet/mscc/ocelot_flower.c +++ b/drivers/net/ethernet/mscc/ocelot_flower.c @@ -793,6 +793,11 @@ static struct ocelot_vcap_filter filter->egress_port.mask = GENMASK(key_length - 1, 0); } + /* Allow the filter to be removed from ocelot->traps + * without traversing the list + */ + INIT_LIST_HEAD(&filter->trap_list); + return filter; }
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. Consequently, when we free a VCAP filter, we must remove it from all lists it is a member of, including ocelot->traps. Normally, conditionally removing an element from a list (depending on whether it is present or not) involves traversing the list, but we already have a reference to the element, so that isn't really necessary. Moreover, the operation "list_del(&filter->trap_list)" operation is fundamentally the same regardless of whether we've iterated through the list or just happened to have the element. So I thought it would be ok to check whether the element has been added to a list by calling list_empty(). However, this does not do the correct thing. list_empty() checks whether "head->next == head", but in our case, head->next == head->prev == NULL, and head != NULL. This makes us proceed to call list_del(), which modifies the prev pointer of the next element, and the next pointer of the prev element. But the next and prev elements are NULL, so we dereference those pointers and die. It would appear that list_empty() is not the function to use to detect that condition. But if we had previously called INIT_LIST_HEAD() on &filter->trap_list, then we could use list_empty() to denote whether we are members of a list (any list). Although the more "natural" thing seems to be to iterate through ocelot->traps and only remove the filter from the list if it was a member of it, it seems pointless to do that. So fix the bug by calling INIT_LIST_HEAD() on the non-head element. Fixes: e42bd4ed09aa ("net: mscc: ocelot: keep traps in a list") Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com> --- drivers/net/ethernet/mscc/ocelot_flower.c | 5 +++++ 1 file changed, 5 insertions(+)