Commit 0b7db7b0 authored by Nicholas Bellinger's avatar Nicholas Bellinger Committed by Greg Kroah-Hartman

target: Fix kref->refcount underflow in transport_cmd_finish_abort

commit 73d4e580 upstream.

This patch fixes a se_cmd->cmd_kref underflow during CMD_T_ABORTED
when a fabric driver drops it's second reference from below the
target_core_tmr.c based callers of transport_cmd_finish_abort().

Recently with the conversion of kref to refcount_t, this bug was
manifesting itself as:

[705519.601034] refcount_t: underflow; use-after-free.
[705519.604034] INFO: NMI handler (kgdb_nmi_handler) took too long to run: 20116.512 msecs
[705539.719111] ------------[ cut here ]------------
[705539.719117] WARNING: CPU: 3 PID: 26510 at lib/refcount.c:184 refcount_sub_and_test+0x33/0x51

Since the original kref atomic_t based kref_put() didn't check for
underflow and only invoked the final callback when zero was reached,
this bug did not manifest in practice since all se_cmd memory is
using preallocated tags.

To address this, go ahead and propigate the existing return from
transport_put_cmd() up via transport_cmd_finish_abort(), and
change transport_cmd_finish_abort() + core_tmr_handle_tas_abort()
callers to only do their local target_put_sess_cmd() if necessary.
Reported-by: default avatarBart Van Assche <bart.vanassche@sandisk.com>
Tested-by: default avatarBart Van Assche <bart.vanassche@sandisk.com>
Cc: Mike Christie <mchristi@redhat.com>
Cc: Hannes Reinecke <hare@suse.de>
Cc: Christoph Hellwig <hch@lst.de>
Cc: Himanshu Madhani <himanshu.madhani@qlogic.com>
Cc: Sagi Grimberg <sagig@mellanox.com>
Tested-by: default avatarGary Guo <ghg@datera.io>
Tested-by: default avatarChu Yuan Lin <cyl@datera.io>
Signed-off-by: default avatarNicholas Bellinger <nab@linux-iscsi.org>
Signed-off-by: default avatarGreg Kroah-Hartman <gregkh@linuxfoundation.org>
parent 90e149d0
...@@ -92,7 +92,7 @@ int init_se_kmem_caches(void); ...@@ -92,7 +92,7 @@ int init_se_kmem_caches(void);
void release_se_kmem_caches(void); void release_se_kmem_caches(void);
u32 scsi_get_new_index(scsi_index_t); u32 scsi_get_new_index(scsi_index_t);
void transport_subsystem_check_init(void); void transport_subsystem_check_init(void);
void transport_cmd_finish_abort(struct se_cmd *, int); int transport_cmd_finish_abort(struct se_cmd *, int);
unsigned char *transport_dump_cmd_direction(struct se_cmd *); unsigned char *transport_dump_cmd_direction(struct se_cmd *);
void transport_dump_dev_state(struct se_device *, char *, int *); void transport_dump_dev_state(struct se_device *, char *, int *);
void transport_dump_dev_info(struct se_device *, struct se_lun *, void transport_dump_dev_info(struct se_device *, struct se_lun *,
......
...@@ -78,7 +78,7 @@ void core_tmr_release_req(struct se_tmr_req *tmr) ...@@ -78,7 +78,7 @@ void core_tmr_release_req(struct se_tmr_req *tmr)
kfree(tmr); kfree(tmr);
} }
static void core_tmr_handle_tas_abort(struct se_cmd *cmd, int tas) static int core_tmr_handle_tas_abort(struct se_cmd *cmd, int tas)
{ {
unsigned long flags; unsigned long flags;
bool remove = true, send_tas; bool remove = true, send_tas;
...@@ -94,7 +94,7 @@ static void core_tmr_handle_tas_abort(struct se_cmd *cmd, int tas) ...@@ -94,7 +94,7 @@ static void core_tmr_handle_tas_abort(struct se_cmd *cmd, int tas)
transport_send_task_abort(cmd); transport_send_task_abort(cmd);
} }
transport_cmd_finish_abort(cmd, remove); return transport_cmd_finish_abort(cmd, remove);
} }
static int target_check_cdb_and_preempt(struct list_head *list, static int target_check_cdb_and_preempt(struct list_head *list,
...@@ -190,7 +190,7 @@ void core_tmr_abort_task( ...@@ -190,7 +190,7 @@ void core_tmr_abort_task(
cancel_work_sync(&se_cmd->work); cancel_work_sync(&se_cmd->work);
transport_wait_for_tasks(se_cmd); transport_wait_for_tasks(se_cmd);
transport_cmd_finish_abort(se_cmd, true); if (!transport_cmd_finish_abort(se_cmd, true))
target_put_sess_cmd(se_cmd); target_put_sess_cmd(se_cmd);
printk("ABORT_TASK: Sending TMR_FUNCTION_COMPLETE for" printk("ABORT_TASK: Sending TMR_FUNCTION_COMPLETE for"
...@@ -291,7 +291,7 @@ static void core_tmr_drain_tmr_list( ...@@ -291,7 +291,7 @@ static void core_tmr_drain_tmr_list(
cancel_work_sync(&cmd->work); cancel_work_sync(&cmd->work);
transport_wait_for_tasks(cmd); transport_wait_for_tasks(cmd);
transport_cmd_finish_abort(cmd, 1); if (!transport_cmd_finish_abort(cmd, 1))
target_put_sess_cmd(cmd); target_put_sess_cmd(cmd);
} }
} }
...@@ -390,7 +390,7 @@ static void core_tmr_drain_state_list( ...@@ -390,7 +390,7 @@ static void core_tmr_drain_state_list(
cancel_work_sync(&cmd->work); cancel_work_sync(&cmd->work);
transport_wait_for_tasks(cmd); transport_wait_for_tasks(cmd);
core_tmr_handle_tas_abort(cmd, tas); if (!core_tmr_handle_tas_abort(cmd, tas))
target_put_sess_cmd(cmd); target_put_sess_cmd(cmd);
} }
} }
......
...@@ -604,9 +604,10 @@ static void transport_lun_remove_cmd(struct se_cmd *cmd) ...@@ -604,9 +604,10 @@ static void transport_lun_remove_cmd(struct se_cmd *cmd)
percpu_ref_put(&lun->lun_ref); percpu_ref_put(&lun->lun_ref);
} }
void transport_cmd_finish_abort(struct se_cmd *cmd, int remove) int transport_cmd_finish_abort(struct se_cmd *cmd, int remove)
{ {
bool ack_kref = (cmd->se_cmd_flags & SCF_ACK_KREF); bool ack_kref = (cmd->se_cmd_flags & SCF_ACK_KREF);
int ret = 0;
if (cmd->se_cmd_flags & SCF_SE_LUN_CMD) if (cmd->se_cmd_flags & SCF_SE_LUN_CMD)
transport_lun_remove_cmd(cmd); transport_lun_remove_cmd(cmd);
...@@ -618,9 +619,11 @@ void transport_cmd_finish_abort(struct se_cmd *cmd, int remove) ...@@ -618,9 +619,11 @@ void transport_cmd_finish_abort(struct se_cmd *cmd, int remove)
cmd->se_tfo->aborted_task(cmd); cmd->se_tfo->aborted_task(cmd);
if (transport_cmd_check_stop_to_fabric(cmd)) if (transport_cmd_check_stop_to_fabric(cmd))
return; return 1;
if (remove && ack_kref) if (remove && ack_kref)
transport_put_cmd(cmd); ret = transport_put_cmd(cmd);
return ret;
} }
static void target_complete_failure_work(struct work_struct *work) static void target_complete_failure_work(struct work_struct *work)
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment