[PATCH] PKT_SCHED: Prevent destroying via RTM_DELTFILTER while classifying

[PATCH] PKT_SCHED: Prevent destroying via RTM_DELTFILTER while classifying

[Thomas Graf]

The classify function of every cls is invoked under BH and 
dev->queue_lock spinlock from dev_xmit. Hence to serialize destroying
the destroy function must be called under the spinlock as well. There
are 2 paths in which a classifier can be destroyed:

1) via the qdisc destroy cb calling tcf_destroy under
   qdisc_tree_lock from __qdisc_destroy (rcu callback)
      
2) via RTM_DELTFILTER in cls_api.c under no locks at all.

The first path seems ok, the initial qdisc destroy attempt is called
under the spinlock and thus serialized with the classify while
the list unlinking takes place and dev_xmit takes care of
the RCU callback, hence classify and all the callbacks needed
from process context cannot be found anymore.

The second path needs the spinlock to avoid destroying while
a classification is in progress. 2.4 probably needs the same fix,
I will cook one up if so.

Signed-off-by: Thomas Graf <tgraf@suug.ch>

--- linux-2.6.10-rc2-bk13.orig/net/sched/cls_api.c	2004-11-30 14:01:12.000000000 +0100
+++ linux-2.6.10-rc2-bk13/net/sched/cls_api.c	2004-12-10 02:16:29.000000000 +0100
@@ -257,7 +257,9 @@
 			qdisc_unlock_tree(dev);
 
 			tfilter_notify(skb, n, tp, fh, RTM_DELTFILTER);
+			qdisc_lock_tree(dev);
 			tcf_destroy(tp);
+			qdisc_unlock_tree(dev);
 			err = 0;
 			goto errout;
 		}

Re: [PATCH] PKT_SCHED: Prevent destroying via RTM_DELTFILTER while classifying

[Patrick McHardy]

Thomas Graf wrote:

>The classify function of every cls is invoked under BH and 
>dev->queue_lock spinlock from dev_xmit. Hence to serialize destroying
>the destroy function must be called under the spinlock as well. There
>are 2 paths in which a classifier can be destroyed:
>
>1) via the qdisc destroy cb calling tcf_destroy under
>   qdisc_tree_lock from __qdisc_destroy (rcu callback)
>      
>2) via RTM_DELTFILTER in cls_api.c under no locks at all.
>
>The first path seems ok, the initial qdisc destroy attempt is called
>under the spinlock and thus serialized with the classify while
>the list unlinking takes place and dev_xmit takes care of
>the RCU callback, hence classify and all the callbacks needed
>from process context cannot be found anymore.
>
>The second path needs the spinlock to avoid destroying while
>a classification is in progress. 2.4 probably needs the same fix,
>I will cook one up if so.
>  
>
This is not correct. The classifier is unlinked in tc_ctl_tfilter
before it is destroyed, so it is not visible for classification anymore.

        if (fh == 0) {
                if (n->nlmsg_type == RTM_DELTFILTER && t->tcm_handle == 0) {
                        qdisc_lock_tree(dev);
                        *back = tp->next;
                        qdisc_unlock_tree(dev);

                        tfilter_notify(skb, n, tp, fh, RTM_DELTFILTER);
                        tcf_destroy(tp);
                        err = 0;
                        goto errout;
                }

Regards
Patrick

Re: [PATCH] PKT_SCHED: Prevent destroying via RTM_DELTFILTER while classifying

[Thomas Graf]

> >The second path needs the spinlock to avoid destroying while
> >a classification is in progress. 2.4 probably needs the same fix,
> >I will cook one up if so.
> > 
> >
> This is not correct. The classifier is unlinked in tc_ctl_tfilter
> before it is destroyed, so it is not visible for classification anymore.

You're right. Thanks.