Commit ccf5ae83a6cf3d9cfe9a7038bfe7cd38ab03d5e1

Authored by Joern Engel
Committed by Nicholas Bellinger
1 parent a1321ddd27

target: close target_put_sess_cmd() vs. core_tmr_abort_task() race

It is possible for one thread to to take se_sess->sess_cmd_lock in
core_tmr_abort_task() before taking a reference count on
se_cmd->cmd_kref, while another thread in target_put_sess_cmd() drops
se_cmd->cmd_kref before taking se_sess->sess_cmd_lock.

This introduces kref_put_spinlock_irqsave() and uses it in
target_put_sess_cmd() to close the race window.

Signed-off-by: Joern Engel <joern@logfs.org>
Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: <stable@vger.kernel.org>
Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>

Showing 2 changed files with 38 additions and 6 deletions Side-by-side Diff

drivers/target/target_core_transport.c
... ... @@ -2211,21 +2211,19 @@
2211 2211 {
2212 2212 struct se_cmd *se_cmd = container_of(kref, struct se_cmd, cmd_kref);
2213 2213 struct se_session *se_sess = se_cmd->se_sess;
2214   - unsigned long flags;
2215 2214  
2216   - spin_lock_irqsave(&se_sess->sess_cmd_lock, flags);
2217 2215 if (list_empty(&se_cmd->se_cmd_list)) {
2218   - spin_unlock_irqrestore(&se_sess->sess_cmd_lock, flags);
  2216 + spin_unlock(&se_sess->sess_cmd_lock);
2219 2217 se_cmd->se_tfo->release_cmd(se_cmd);
2220 2218 return;
2221 2219 }
2222 2220 if (se_sess->sess_tearing_down && se_cmd->cmd_wait_set) {
2223   - spin_unlock_irqrestore(&se_sess->sess_cmd_lock, flags);
  2221 + spin_unlock(&se_sess->sess_cmd_lock);
2224 2222 complete(&se_cmd->cmd_wait_comp);
2225 2223 return;
2226 2224 }
2227 2225 list_del(&se_cmd->se_cmd_list);
2228   - spin_unlock_irqrestore(&se_sess->sess_cmd_lock, flags);
  2226 + spin_unlock(&se_sess->sess_cmd_lock);
2229 2227  
2230 2228 se_cmd->se_tfo->release_cmd(se_cmd);
2231 2229 }
... ... @@ -2236,7 +2234,8 @@
2236 2234 */
2237 2235 int target_put_sess_cmd(struct se_session *se_sess, struct se_cmd *se_cmd)
2238 2236 {
2239   - return kref_put(&se_cmd->cmd_kref, target_release_cmd_kref);
  2237 + return kref_put_spinlock_irqsave(&se_cmd->cmd_kref, target_release_cmd_kref,
  2238 + &se_sess->sess_cmd_lock);
2240 2239 }
2241 2240 EXPORT_SYMBOL(target_put_sess_cmd);
2242 2241  
include/linux/kref.h
... ... @@ -19,6 +19,7 @@
19 19 #include <linux/atomic.h>
20 20 #include <linux/kernel.h>
21 21 #include <linux/mutex.h>
  22 +#include <linux/spinlock.h>
22 23  
23 24 struct kref {
24 25 atomic_t refcount;
... ... @@ -93,6 +94,38 @@
93 94 static inline int kref_put(struct kref *kref, void (*release)(struct kref *kref))
94 95 {
95 96 return kref_sub(kref, 1, release);
  97 +}
  98 +
  99 +/**
  100 + * kref_put_spinlock_irqsave - decrement refcount for object.
  101 + * @kref: object.
  102 + * @release: pointer to the function that will clean up the object when the
  103 + * last reference to the object is released.
  104 + * This pointer is required, and it is not acceptable to pass kfree
  105 + * in as this function.
  106 + * @lock: lock to take in release case
  107 + *
  108 + * Behaves identical to kref_put with one exception. If the reference count
  109 + * drops to zero, the lock will be taken atomically wrt dropping the reference
  110 + * count. The release function has to call spin_unlock() without _irqrestore.
  111 + */
  112 +static inline int kref_put_spinlock_irqsave(struct kref *kref,
  113 + void (*release)(struct kref *kref),
  114 + spinlock_t *lock)
  115 +{
  116 + unsigned long flags;
  117 +
  118 + WARN_ON(release == NULL);
  119 + if (atomic_add_unless(&kref->refcount, -1, 1))
  120 + return 0;
  121 + spin_lock_irqsave(lock, flags);
  122 + if (atomic_dec_and_test(&kref->refcount)) {
  123 + release(kref);
  124 + local_irq_restore(flags);
  125 + return 1;
  126 + }
  127 + spin_unlock_irqrestore(lock, flags);
  128 + return 0;
96 129 }
97 130  
98 131 static inline int kref_put_mutex(struct kref *kref,