diff options
Diffstat (limited to '0005-BFQ-bugfix.patch')
-rw-r--r-- | 0005-BFQ-bugfix.patch | 341 |
1 files changed, 341 insertions, 0 deletions
diff --git a/0005-BFQ-bugfix.patch b/0005-BFQ-bugfix.patch new file mode 100644 index 000000000000..68be7f5fe705 --- /dev/null +++ b/0005-BFQ-bugfix.patch @@ -0,0 +1,341 @@ +From 2da7edbb8f54b447c0b7dfa029ee4df580a6030e Mon Sep 17 00:00:00 2001 +From: Paolo Valente <paolo.valente@linaro.org> +Date: Thu, 16 Mar 2017 14:36:41 +0100 +Subject: [PATCH] BUGIFX: remove use of bfq queues after free + +bfq queues occasionally happened to be used after being freed, because +they were accessed after some invocations of bfq_put_queue that could +cause them to be freed. This commit refactors code, when needed, to +avoid any occurrence of such a use-after-free of a bfq queue. This +commit also adds comments to make references to bfq queues easier to +follow. + +Signed-off-by: Paolo Valente <paolo.valente@linaro.org> +--- + block/bfq-cgroup.c | 1 - + block/bfq-iosched.c | 36 ++++++++++++++++++--------- + block/bfq-sched.c | 71 ++++++++++++++++++++++++++++++++++++----------------- + 3 files changed, 73 insertions(+), 35 deletions(-) + +diff --git a/block/bfq-cgroup.c b/block/bfq-cgroup.c +index de045cf..a66a723 100644 +--- a/block/bfq-cgroup.c ++++ b/block/bfq-cgroup.c +@@ -771,7 +771,6 @@ static void bfq_pd_offline(struct blkg_policy_data *pd) + + __bfq_deactivate_entity(entity, false); + bfq_put_async_queues(bfqd, bfqg); +- BUG_ON(entity->tree); + + /* + * @blkg is going offline and will be ignored by +diff --git a/block/bfq-iosched.c b/block/bfq-iosched.c +index 21197f6..f00761b 100644 +--- a/block/bfq-iosched.c ++++ b/block/bfq-iosched.c +@@ -1391,7 +1391,6 @@ static void bfq_bfqq_handle_idle_busy_switch(struct bfq_data *bfqd, + + bfq_bfqq_expire(bfqd, bfqd->in_service_queue, + false, BFQ_BFQQ_PREEMPTED); +- BUG_ON(in_serv->entity.budget < 0); + } + } + +@@ -1560,8 +1559,10 @@ static void bfq_remove_request(struct request *rq) + BUG_ON(bfqq->entity.budget < 0); + + if (bfq_bfqq_busy(bfqq) && bfqq != bfqd->in_service_queue) { ++ BUG_ON(bfqq->ref < 2); /* referred by rq and on tree */ + bfq_del_bfqq_busy(bfqd, bfqq, false); +- /* bfqq emptied. In normal operation, when ++ /* ++ * bfqq emptied. In normal operation, when + * bfqq is empty, bfqq->entity.service and + * bfqq->entity.budget must contain, + * respectively, the service received and the +@@ -1570,7 +1571,8 @@ static void bfq_remove_request(struct request *rq) + * this last removal occurred while bfqq is + * not in service. To avoid inconsistencies, + * reset both bfqq->entity.service and +- * bfqq->entity.budget. ++ * bfqq->entity.budget, if bfqq has still a ++ * process that may issue I/O requests to it. + */ + bfqq->entity.budget = bfqq->entity.service = 0; + } +@@ -2062,7 +2064,8 @@ bfq_merge_bfqqs(struct bfq_data *bfqd, struct bfq_io_cq *bic, + new_bfqq->wr_coeff = bfqq->wr_coeff; + new_bfqq->wr_cur_max_time = bfqq->wr_cur_max_time; + new_bfqq->last_wr_start_finish = bfqq->last_wr_start_finish; +- new_bfqq->wr_start_at_switch_to_srt = bfqq->wr_start_at_switch_to_srt; ++ new_bfqq->wr_start_at_switch_to_srt = ++ bfqq->wr_start_at_switch_to_srt; + if (bfq_bfqq_busy(new_bfqq)) + bfqd->wr_busy_queues++; + new_bfqq->entity.prio_changed = 1; +@@ -2105,6 +2108,7 @@ bfq_merge_bfqqs(struct bfq_data *bfqd, struct bfq_io_cq *bic, + */ + new_bfqq->bic = NULL; + bfqq->bic = NULL; ++ /* release process reference to bfqq */ + bfq_put_queue(bfqq); + } + +@@ -3077,6 +3081,7 @@ static void bfq_bfqq_expire(struct bfq_data *bfqd, + bool slow; + unsigned long delta = 0; + struct bfq_entity *entity = &bfqq->entity; ++ int ref; + + BUG_ON(bfqq != bfqd->in_service_queue); + +@@ -3184,12 +3189,15 @@ static void bfq_bfqq_expire(struct bfq_data *bfqd, + __bfq_bfqq_recalc_budget(bfqd, bfqq, reason); + BUG_ON(bfqq->next_rq == NULL && + bfqq->entity.budget < bfqq->entity.service); ++ ref = bfqq->ref; + __bfq_bfqq_expire(bfqd, bfqq); + +- BUG_ON(!bfq_bfqq_busy(bfqq) && reason == BFQ_BFQQ_BUDGET_EXHAUSTED && ++ BUG_ON(ref > 1 && ++ !bfq_bfqq_busy(bfqq) && reason == BFQ_BFQQ_BUDGET_EXHAUSTED && + !bfq_class_idle(bfqq)); + +- if (!bfq_bfqq_busy(bfqq) && ++ /* mark bfqq as waiting a request only if a bic still points to it */ ++ if (ref > 1 && !bfq_bfqq_busy(bfqq) && + reason != BFQ_BFQQ_BUDGET_TIMEOUT && + reason != BFQ_BFQQ_BUDGET_EXHAUSTED) + bfq_mark_bfqq_non_blocking_wait_rq(bfqq); +@@ -3809,7 +3817,8 @@ static int bfq_dispatch_requests(struct request_queue *q, int force) + * Task holds one reference to the queue, dropped when task exits. Each rq + * in-flight on this queue also holds a reference, dropped when rq is freed. + * +- * Queue lock must be held here. ++ * Queue lock must be held here. Recall not to use bfqq after calling ++ * this function on it. + */ + static void bfq_put_queue(struct bfq_queue *bfqq) + { +@@ -3878,7 +3887,7 @@ static void bfq_exit_bfqq(struct bfq_data *bfqd, struct bfq_queue *bfqq) + + bfq_put_cooperator(bfqq); + +- bfq_put_queue(bfqq); ++ bfq_put_queue(bfqq); /* release process reference */ + } + + static void bfq_init_icq(struct io_cq *icq) +@@ -3977,6 +3986,7 @@ static void bfq_check_ioprio_change(struct bfq_io_cq *bic, struct bio *bio) + + bfqq = bic_to_bfqq(bic, false); + if (bfqq) { ++ /* release process reference on this queue */ + bfq_put_queue(bfqq); + bfqq = bfq_get_queue(bfqd, bio, BLK_RW_ASYNC, bic); + bic_set_bfqq(bic, bfqq, false); +@@ -4110,7 +4120,7 @@ static struct bfq_queue *bfq_get_queue(struct bfq_data *bfqd, + } + + out: +- bfqq->ref++; ++ bfqq->ref++; /* get a process reference to this queue */ + bfq_log_bfqq(bfqd, bfqq, "get_queue, at end: %p, %d", bfqq, bfqq->ref); + rcu_read_unlock(); + return bfqq; +@@ -4284,10 +4294,14 @@ static void bfq_insert_request(struct request_queue *q, struct request *rq) + bfqq->allocated[rq_data_dir(rq)]--; + new_bfqq->ref++; + bfq_clear_bfqq_just_created(bfqq); +- bfq_put_queue(bfqq); + if (bic_to_bfqq(RQ_BIC(rq), 1) == bfqq) + bfq_merge_bfqqs(bfqd, RQ_BIC(rq), + bfqq, new_bfqq); ++ /* ++ * rq is about to be enqueued into new_bfqq, ++ * release rq reference on bfqq ++ */ ++ bfq_put_queue(bfqq); + rq->elv.priv[1] = new_bfqq; + bfqq = new_bfqq; + } +@@ -4708,7 +4722,7 @@ static void bfq_shutdown_timer_wq(struct bfq_data *bfqd) + } + + static void __bfq_put_async_bfqq(struct bfq_data *bfqd, +- struct bfq_queue **bfqq_ptr) ++ struct bfq_queue **bfqq_ptr) + { + struct bfq_group *root_group = bfqd->root_group; + struct bfq_queue *bfqq = *bfqq_ptr; +diff --git a/block/bfq-sched.c b/block/bfq-sched.c +index 2e9dc59..70aac56 100644 +--- a/block/bfq-sched.c ++++ b/block/bfq-sched.c +@@ -154,7 +154,13 @@ static bool bfq_update_next_in_service(struct bfq_sched_data *sd, + #define for_each_entity(entity) \ + for (; entity ; entity = entity->parent) + +-#define for_each_entity_safe(entity, parent) \ ++/* ++ * For each iteration, compute parent in advance, so as to be safe if ++ * entity is deallocated during the iteration. Such a deallocation may ++ * happen as a consequence of a bfq_put_queue that frees the bfq_queue ++ * containing entity. ++ */ ++#define for_each_entity_safe(entity, parent) \ + for (; entity && ({ parent = entity->parent; 1; }); entity = parent) + + /* +@@ -691,27 +697,31 @@ static void bfq_idle_insert(struct bfq_service_tree *st, + } + + /** +- * bfq_forget_entity - remove an entity from the wfq trees. ++ * bfq_forget_entity - do not consider entity any longer for scheduling + * @st: the service tree. + * @entity: the entity being removed. ++ * @is_in_service: true if entity is currently the in-service entity. + * +- * Update the device status and forget everything about @entity, putting +- * the device reference to it, if it is a queue. Entities belonging to +- * groups are not refcounted. ++ * Forget everything about @entity. In addition, if entity represents ++ * a queue, and the latter is not in service, then release the service ++ * reference to the queue (the one taken through bfq_get_entity). In ++ * fact, in this case, there is really no more service reference to ++ * the queue, as the latter is also outside any service tree. If, ++ * instead, the queue is in service, then __bfq_bfqd_reset_in_service ++ * will take care of putting the reference when the queue finally ++ * stops being served. + */ + static void bfq_forget_entity(struct bfq_service_tree *st, +- struct bfq_entity *entity) ++ struct bfq_entity *entity, ++ bool is_in_service) + { + struct bfq_queue *bfqq = bfq_entity_to_bfqq(entity); +- struct bfq_sched_data *sd; +- + BUG_ON(!entity->on_st); + + entity->on_st = false; + st->wsum -= entity->weight; +- if (bfqq) { +- sd = entity->sched_data; +- bfq_log_bfqq(bfqq->bfqd, bfqq, "forget_entity: %p %d", ++ if (bfqq && !is_in_service) { ++ bfq_log_bfqq(bfqq->bfqd, bfqq, "forget_entity (before): %p %d", + bfqq, bfqq->ref); + bfq_put_queue(bfqq); + } +@@ -726,7 +736,8 @@ static void bfq_put_idle_entity(struct bfq_service_tree *st, + struct bfq_entity *entity) + { + bfq_idle_extract(st, entity); +- bfq_forget_entity(st, entity); ++ bfq_forget_entity(st, entity, ++ entity == entity->sched_data->in_service_entity); + } + + /** +@@ -1082,6 +1093,12 @@ static void __bfq_activate_entity(struct bfq_entity *entity, + */ + entity->start = min_vstart; + st->wsum += entity->weight; ++ /* ++ * entity is about to be inserted into a service tree, ++ * and then set in service: get a reference to make ++ * sure entity does not disappear until it is no ++ * longer in service or scheduled for service. ++ */ + bfq_get_entity(entity); + + BUG_ON(entity->on_st && bfqq); +@@ -1264,27 +1281,27 @@ static bool __bfq_deactivate_entity(struct bfq_entity *entity, + { + struct bfq_sched_data *sd = entity->sched_data; + struct bfq_service_tree *st = bfq_entity_service_tree(entity); +- bool was_in_service = entity == sd->in_service_entity; ++ bool is_in_service = entity == sd->in_service_entity; + + if (!entity->on_st) { /* entity never activated, or already inactive */ + BUG_ON(entity == entity->sched_data->in_service_entity); + return false; + } + +- BUG_ON(was_in_service && entity->tree && entity->tree != &st->active); ++ BUG_ON(is_in_service && entity->tree && entity->tree != &st->active); + +- if (was_in_service) ++ if (is_in_service) + bfq_calc_finish(entity, entity->service); + + if (entity->tree == &st->active) + bfq_active_extract(st, entity); +- else if (!was_in_service && entity->tree == &st->idle) ++ else if (!is_in_service && entity->tree == &st->idle) + bfq_idle_extract(st, entity); + else if (entity->tree) + BUG(); + + if (!ins_into_idle_tree || !bfq_gt(entity->finish, st->vtime)) +- bfq_forget_entity(st, entity); ++ bfq_forget_entity(st, entity, is_in_service); + else + bfq_idle_insert(st, entity); + +@@ -1320,8 +1337,8 @@ static void bfq_deactivate_entity(struct bfq_entity *entity, + + if (!__bfq_deactivate_entity(entity, ins_into_idle_tree)) { + /* +- * Entity is not any tree any more, so, this +- * deactivation is a no-op, and there is ++ * entity is not in any tree any more, so ++ * this deactivation is a no-op, and there is + * nothing to change for upper-level entities + * (in case of expiration, this can never + * happen). +@@ -1821,14 +1838,16 @@ static struct bfq_queue *bfq_get_next_queue(struct bfq_data *bfqd) + + static void __bfq_bfqd_reset_in_service(struct bfq_data *bfqd) + { +- struct bfq_entity *entity = &bfqd->in_service_queue->entity; ++ struct bfq_queue *in_serv_bfqq = bfqd->in_service_queue; ++ struct bfq_entity *in_serv_entity = &in_serv_bfqq->entity; ++ struct bfq_entity *entity = in_serv_entity; + + if (bfqd->in_service_bic) { + put_io_context(bfqd->in_service_bic->icq.ioc); + bfqd->in_service_bic = NULL; + } + +- bfq_clear_bfqq_wait_request(bfqd->in_service_queue); ++ bfq_clear_bfqq_wait_request(in_serv_bfqq); + hrtimer_try_to_cancel(&bfqd->idle_slice_timer); + bfqd->in_service_queue = NULL; + +@@ -1840,6 +1859,14 @@ static void __bfq_bfqd_reset_in_service(struct bfq_data *bfqd) + */ + for_each_entity(entity) + entity->sched_data->in_service_entity = NULL; ++ ++ /* ++ * in_serv_entity is no longer in service, so, if it is in no ++ * service tree either, then release the service reference to ++ * the queue it represents (taken with bfq_get_entity). ++ */ ++ if (!in_serv_entity->on_st) ++ bfq_put_queue(in_serv_bfqq); + } + + static void bfq_deactivate_bfqq(struct bfq_data *bfqd, struct bfq_queue *bfqq, +@@ -1904,8 +1931,6 @@ static void bfq_del_bfqq_busy(struct bfq_data *bfqd, struct bfq_queue *bfqq, + BUG_ON(bfqq->entity.budget < 0); + + bfq_deactivate_bfqq(bfqd, bfqq, true, expiration); +- +- BUG_ON(bfqq->entity.budget < 0); + } + + /* |