Skip to content

Commit

Permalink
net: filter: don't release unattached filter through call_rcu()
Browse files Browse the repository at this point in the history
sk_unattached_filter_destroy() does not always need to release the
filter object via rcu. Since this filter is never attached to the
socket, the caller should be responsible for releasing the filter
in a safe way, which may not necessarily imply rcu.

This is a short summary of clients of this function:

1) xt_bpf.c and cls_bpf.c use the bpf matchers from rules, these rules
   are removed from the packet path before the filter is released. Thus,
   the framework makes sure the filter is safely removed.

2) In the ppp driver, the ppp_lock ensures serialization between the
   xmit and filter attachment/detachment path. This doesn't use rcu
   so deferred release via rcu makes no sense.

3) In the isdn/ppp driver, it is called from isdn_ppp_release()
   the isdn_ppp_ioctl(). This driver uses mutex and spinlocks, no rcu.
   Thus, deferred rcu makes no sense to me either, the deferred releases
   may be just masking the effects of wrong locking strategy, which
   should be fixed in the driver itself.

4) In the team driver, this is the only place where the rcu
   synchronization with unattached filter is used. Therefore, this
   patch introduces synchronize_rcu() which is called from the
   genetlink path to make sure the filter doesn't go away while packets
   are still walking over it. I think we can revisit this once struct
   bpf_prog (that only wraps specific bpf code bits) is in place, then
   add some specific struct rcu_head in the scope of the team driver if
   Jiri thinks this is needed.

Deferred rcu release for unattached filters was originally introduced
in 302d663 ("filter: Allow to create sk-unattached filters").

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Signed-off-by: David S. Miller <davem@davemloft.net>
  • Loading branch information
Pablo Neira authored and David S. Miller committed Jul 31, 2014
1 parent e10038a commit 34c5bd6
Show file tree
Hide file tree
Showing 2 changed files with 13 additions and 4 deletions.
6 changes: 5 additions & 1 deletion drivers/net/team/team_mode_loadbalance.c
Original file line number Diff line number Diff line change
Expand Up @@ -293,11 +293,15 @@ static int lb_bpf_func_set(struct team *team, struct team_gsetter_ctx *ctx)
__fprog_destroy(lb_priv->ex->orig_fprog);
orig_fp = rcu_dereference_protected(lb_priv->fp,
lockdep_is_held(&team->lock));
sk_unattached_filter_destroy(orig_fp);
}

rcu_assign_pointer(lb_priv->fp, fp);
lb_priv->ex->orig_fprog = fprog;

if (orig_fp) {
synchronize_rcu();
sk_unattached_filter_destroy(orig_fp);
}
return 0;
}

Expand Down
11 changes: 8 additions & 3 deletions net/core/filter.c
Original file line number Diff line number Diff line change
Expand Up @@ -841,6 +841,12 @@ static void sk_release_orig_filter(struct sk_filter *fp)
}
}

static void __sk_filter_release(struct sk_filter *fp)
{
sk_release_orig_filter(fp);
sk_filter_free(fp);
}

/**
* sk_filter_release_rcu - Release a socket filter by rcu_head
* @rcu: rcu_head that contains the sk_filter to free
Expand All @@ -849,8 +855,7 @@ static void sk_filter_release_rcu(struct rcu_head *rcu)
{
struct sk_filter *fp = container_of(rcu, struct sk_filter, rcu);

sk_release_orig_filter(fp);
sk_filter_free(fp);
__sk_filter_release(fp);
}

/**
Expand Down Expand Up @@ -1050,7 +1055,7 @@ EXPORT_SYMBOL_GPL(sk_unattached_filter_create);

void sk_unattached_filter_destroy(struct sk_filter *fp)
{
sk_filter_release(fp);
__sk_filter_release(fp);
}
EXPORT_SYMBOL_GPL(sk_unattached_filter_destroy);

Expand Down

0 comments on commit 34c5bd6

Please sign in to comment.