]>
Commit | Line | Data |
---|---|---|
6d9ed630 GKH |
1 | From 73d4e580ccc5c3e05cea002f18111f66c9c07034 Mon Sep 17 00:00:00 2001 |
2 | From: Nicholas Bellinger <nab@linux-iscsi.org> | |
3 | Date: Fri, 2 Jun 2017 20:00:17 -0700 | |
4 | Subject: target: Fix kref->refcount underflow in transport_cmd_finish_abort | |
5 | ||
6 | From: Nicholas Bellinger <nab@linux-iscsi.org> | |
7 | ||
8 | commit 73d4e580ccc5c3e05cea002f18111f66c9c07034 upstream. | |
9 | ||
10 | This patch fixes a se_cmd->cmd_kref underflow during CMD_T_ABORTED | |
11 | when a fabric driver drops it's second reference from below the | |
12 | target_core_tmr.c based callers of transport_cmd_finish_abort(). | |
13 | ||
14 | Recently with the conversion of kref to refcount_t, this bug was | |
15 | manifesting itself as: | |
16 | ||
17 | [705519.601034] refcount_t: underflow; use-after-free. | |
18 | [705519.604034] INFO: NMI handler (kgdb_nmi_handler) took too long to run: 20116.512 msecs | |
19 | [705539.719111] ------------[ cut here ]------------ | |
20 | [705539.719117] WARNING: CPU: 3 PID: 26510 at lib/refcount.c:184 refcount_sub_and_test+0x33/0x51 | |
21 | ||
22 | Since the original kref atomic_t based kref_put() didn't check for | |
23 | underflow and only invoked the final callback when zero was reached, | |
24 | this bug did not manifest in practice since all se_cmd memory is | |
25 | using preallocated tags. | |
26 | ||
27 | To address this, go ahead and propigate the existing return from | |
28 | transport_put_cmd() up via transport_cmd_finish_abort(), and | |
29 | change transport_cmd_finish_abort() + core_tmr_handle_tas_abort() | |
30 | callers to only do their local target_put_sess_cmd() if necessary. | |
31 | ||
32 | Reported-by: Bart Van Assche <bart.vanassche@sandisk.com> | |
33 | Tested-by: Bart Van Assche <bart.vanassche@sandisk.com> | |
34 | Cc: Mike Christie <mchristi@redhat.com> | |
35 | Cc: Hannes Reinecke <hare@suse.de> | |
36 | Cc: Christoph Hellwig <hch@lst.de> | |
37 | Cc: Himanshu Madhani <himanshu.madhani@qlogic.com> | |
38 | Cc: Sagi Grimberg <sagig@mellanox.com> | |
39 | Tested-by: Gary Guo <ghg@datera.io> | |
40 | Tested-by: Chu Yuan Lin <cyl@datera.io> | |
41 | Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org> | |
42 | Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> | |
43 | ||
44 | --- | |
45 | drivers/target/target_core_internal.h | 2 +- | |
46 | drivers/target/target_core_tmr.c | 16 ++++++++-------- | |
47 | drivers/target/target_core_transport.c | 9 ++++++--- | |
48 | 3 files changed, 15 insertions(+), 12 deletions(-) | |
49 | ||
50 | --- a/drivers/target/target_core_internal.h | |
51 | +++ b/drivers/target/target_core_internal.h | |
52 | @@ -132,7 +132,7 @@ int init_se_kmem_caches(void); | |
53 | void release_se_kmem_caches(void); | |
54 | u32 scsi_get_new_index(scsi_index_t); | |
55 | void transport_subsystem_check_init(void); | |
56 | -void transport_cmd_finish_abort(struct se_cmd *, int); | |
57 | +int transport_cmd_finish_abort(struct se_cmd *, int); | |
58 | unsigned char *transport_dump_cmd_direction(struct se_cmd *); | |
59 | void transport_dump_dev_state(struct se_device *, char *, int *); | |
60 | void transport_dump_dev_info(struct se_device *, struct se_lun *, | |
61 | --- a/drivers/target/target_core_tmr.c | |
62 | +++ b/drivers/target/target_core_tmr.c | |
63 | @@ -75,7 +75,7 @@ void core_tmr_release_req(struct se_tmr_ | |
64 | kfree(tmr); | |
65 | } | |
66 | ||
67 | -static void core_tmr_handle_tas_abort(struct se_cmd *cmd, int tas) | |
68 | +static int core_tmr_handle_tas_abort(struct se_cmd *cmd, int tas) | |
69 | { | |
70 | unsigned long flags; | |
71 | bool remove = true, send_tas; | |
72 | @@ -91,7 +91,7 @@ static void core_tmr_handle_tas_abort(st | |
73 | transport_send_task_abort(cmd); | |
74 | } | |
75 | ||
76 | - transport_cmd_finish_abort(cmd, remove); | |
77 | + return transport_cmd_finish_abort(cmd, remove); | |
78 | } | |
79 | ||
80 | static int target_check_cdb_and_preempt(struct list_head *list, | |
81 | @@ -185,8 +185,8 @@ void core_tmr_abort_task( | |
82 | cancel_work_sync(&se_cmd->work); | |
83 | transport_wait_for_tasks(se_cmd); | |
84 | ||
85 | - transport_cmd_finish_abort(se_cmd, true); | |
86 | - target_put_sess_cmd(se_cmd); | |
87 | + if (!transport_cmd_finish_abort(se_cmd, true)) | |
88 | + target_put_sess_cmd(se_cmd); | |
89 | ||
90 | printk("ABORT_TASK: Sending TMR_FUNCTION_COMPLETE for" | |
91 | " ref_tag: %llu\n", ref_tag); | |
92 | @@ -286,8 +286,8 @@ static void core_tmr_drain_tmr_list( | |
93 | cancel_work_sync(&cmd->work); | |
94 | transport_wait_for_tasks(cmd); | |
95 | ||
96 | - transport_cmd_finish_abort(cmd, 1); | |
97 | - target_put_sess_cmd(cmd); | |
98 | + if (!transport_cmd_finish_abort(cmd, 1)) | |
99 | + target_put_sess_cmd(cmd); | |
100 | } | |
101 | } | |
102 | ||
103 | @@ -385,8 +385,8 @@ static void core_tmr_drain_state_list( | |
104 | cancel_work_sync(&cmd->work); | |
105 | transport_wait_for_tasks(cmd); | |
106 | ||
107 | - core_tmr_handle_tas_abort(cmd, tas); | |
108 | - target_put_sess_cmd(cmd); | |
109 | + if (!core_tmr_handle_tas_abort(cmd, tas)) | |
110 | + target_put_sess_cmd(cmd); | |
111 | } | |
112 | } | |
113 | ||
114 | --- a/drivers/target/target_core_transport.c | |
115 | +++ b/drivers/target/target_core_transport.c | |
116 | @@ -639,9 +639,10 @@ static void transport_lun_remove_cmd(str | |
117 | percpu_ref_put(&lun->lun_ref); | |
118 | } | |
119 | ||
120 | -void transport_cmd_finish_abort(struct se_cmd *cmd, int remove) | |
121 | +int transport_cmd_finish_abort(struct se_cmd *cmd, int remove) | |
122 | { | |
123 | bool ack_kref = (cmd->se_cmd_flags & SCF_ACK_KREF); | |
124 | + int ret = 0; | |
125 | ||
126 | if (cmd->se_cmd_flags & SCF_SE_LUN_CMD) | |
127 | transport_lun_remove_cmd(cmd); | |
128 | @@ -653,9 +654,11 @@ void transport_cmd_finish_abort(struct s | |
129 | cmd->se_tfo->aborted_task(cmd); | |
130 | ||
131 | if (transport_cmd_check_stop_to_fabric(cmd)) | |
132 | - return; | |
133 | + return 1; | |
134 | if (remove && ack_kref) | |
135 | - transport_put_cmd(cmd); | |
136 | + ret = transport_put_cmd(cmd); | |
137 | + | |
138 | + return ret; | |
139 | } | |
140 | ||
141 | static void target_complete_failure_work(struct work_struct *work) |