Skip to contentSkip to conversation

: Fix scheduler/classifier module unload race

PATCH Applied Patrick McHardy 2 participants 2 messages Nov 7, 2004 – Nov 10, 2004

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

PATCH 1/1 : Fix scheduler/classifier module unload race

This patch fixes an scheduler/classifier module unload race. struct Qdisc_ops which includes the owner field is also part of the module's memory, so ops might already be freed when try_module_get(ops->owner) is called outside of the locked section.

Regards
Patrick

# This is a BitKeeper generated diff -Nru style patch. #
# ChangeSet
# 2004/11/07 05:03:11+01:00 [email protected]
# [PKT_SCHED]: Fix scheduler/classifier module unload race
#
# Signed-off-by: Patrick McHardy <[email protected]>
#

# net/sched/sch_api.c
#   2004/11/07 05:03:04+01:00 [email protected] +4 -4
#   [PKT_SCHED]: Fix scheduler/classifier module unload race
#   
#   Signed-off-by: Patrick McHardy <[email protected]>
# 
# net/sched/cls_api.c
#   2004/11/07 05:03:04+01:00 [email protected] +4 -6
#   [PKT_SCHED]: Fix scheduler/classifier module unload race
#   
#   Signed-off-by: Patrick McHardy <[email protected]>
# 
diff -Nru a/net/sched/cls_api.c b/net/sched/cls_api.c
--- a/net/sched/cls_api.c	2004-11-07 23:32:50 +01:00
+++ b/net/sched/cls_api.c	2004-11-07 23:32:50 +01:00
@@ -60,8 +60,11 @@
 	if (kind) {
 		read_lock(&cls_mod_lock);
 		for (t = tcf_proto_base; t; t = t->next) {
-			if (rtattr_strcmp(kind, t->kind) == 0)
+			if (rtattr_strcmp(kind, t->kind) == 0) {
+				if (!try_module_get(t->owner))
+					t = NULL;
 				break;
+			}
 		}
 		read_unlock(&cls_mod_lock);
 	}
@@ -231,11 +234,6 @@
 		tp->q = q;
 		tp->classify = tp_ops->classify;
 		tp->classid = parent;
-		err = -EBUSY;
-		if (!try_module_get(tp_ops->owner)) {
-			kfree(tp);
-			goto errout;
-		}
 		if ((err = tp_ops->init(tp)) != 0) {
 			module_put(tp_ops->owner);
 			kfree(tp);
diff -Nru a/net/sched/sch_api.c b/net/sched/sch_api.c
--- a/net/sched/sch_api.c	2004-11-07 23:32:50 +01:00
+++ b/net/sched/sch_api.c	2004-11-07 23:32:50 +01:00
@@ -229,8 +229,11 @@
 	if (kind) {
 		read_lock(&qdisc_mod_lock);
 		for (q = qdisc_base; q; q = q->next) {
-			if (rtattr_strcmp(kind, q->id) == 0)
+			if (rtattr_strcmp(kind, q->id) == 0) {
+				if (!try_module_get(q->owner))
+					q = NULL;
 				break;
+			}
 		}
 		read_unlock(&qdisc_mod_lock);
 	}
@@ -408,9 +411,6 @@

err = -EINVAL;
if (ops == NULL)
- goto err_out;
- err = -EBUSY;
- if (!try_module_get(ops->owner))
goto err_out;

/* ensure that the Qdisc and the private data are 32-byte aligned */

maintainer Applied
#1 lore raw
Quoted 5 lines · On Sun, 07 Nov 2004 23:41:27 +0100 Patrick McHardy <[email protected]> wrote:

This patch fixes an scheduler/classifier module unload race. struct Qdisc_ops which includes the owner field is also part of the module's memory, so ops might already be freed when try_module_get(ops->owner) is called outside of the locked section.

Applied, thanks a lot Patrick.