All of lore.kernel.org
 help / color / mirror / Atom feed
From: Grzegorz Uriasz <gorbak25@gmail.com>
To: "Martin K. Petersen" <martin.petersen@oracle.com>,
	linux-scsi@vger.kernel.org, target-devel@vger.kernel.org,
	linux-kernel@vger.kernel.org
Cc: dutkahugo@gmail.com
Subject: [PATCH] scsi: target: Fix data corruption under concurrent target configuration
Date: Sat, 20 May 2023 02:26:14 +0200	[thread overview]
Message-ID: <5f637569-36af-a8d0-e378-b27a63f08501@gmail.com> (raw)

[-- Attachment #1: Type: text/plain, Size: 2946 bytes --]

This fixes data corruptions arising from concurrent enabling of a target
devices. When multiple enable calls are made concurrently then it is
possible for the target device to be set up twice which results in a
kernel BUG.
Introduces a per target device mutex for serializing enable requests.

Signed-off-by: Grzegorz Uriasz <gorbak25@gmail.com>
---
  drivers/target/target_core_device.c | 17 +++++++++++++----
  include/target/target_core_base.h   |  1 +
  2 files changed, 14 insertions(+), 4 deletions(-)

diff --git a/drivers/target/target_core_device.c 
b/drivers/target/target_core_device.c
index 90f3f4926172..6d8fb962c780 100644
--- a/drivers/target/target_core_device.c
+++ b/drivers/target/target_core_device.c
@@ -742,6 +742,7 @@ struct se_device *target_alloc_device(struct se_hba 
*hba, const char *name)

      INIT_WORK(&dev->delayed_cmd_work, target_do_delayed_work);
      mutex_init(&dev->lun_reset_mutex);
+    mutex_init(&dev->configure_mutex);

      dev->t10_wwn.t10_dev = dev;
      /*
@@ -904,10 +905,15 @@ int target_configure_device(struct se_device *dev)
      struct se_hba *hba = dev->se_hba;
      int ret, id;

+    ret = mutex_lock_interruptible(&dev->configure_mutex);
+    if (ret)
+        return ret;
+
      if (target_dev_configured(dev)) {
          pr_err("se_dev->se_dev_ptr already set for storage"
                  " object\n");
-        return -EEXIST;
+        ret = -EEXIST;
+        goto out_release_mutex;
      }

      /*
@@ -923,7 +929,7 @@ int target_configure_device(struct se_device *dev)
      mutex_unlock(&device_mutex);
      if (id < 0) {
          ret = -ENOMEM;
-        goto out;
+        goto out_release_vpd;
      }
      dev->dev_index = id;

@@ -969,7 +975,8 @@ int target_configure_device(struct se_device *dev)

      dev->dev_flags |= DF_CONFIGURED;

-    return 0;
+    ret = 0;
+    goto out_release_mutex;

  out_destroy_device:
      dev->transport->destroy_device(dev);
@@ -977,8 +984,10 @@ int target_configure_device(struct se_device *dev)
      mutex_lock(&device_mutex);
      idr_remove(&devices_idr, dev->dev_index);
      mutex_unlock(&device_mutex);
-out:
+out_release_vpd:
      se_release_vpd_for_dev(dev);
+out_release_mutex:
+    mutex_unlock(&dev->configure_mutex);
      return ret;
  }

diff --git a/include/target/target_core_base.h 
b/include/target/target_core_base.h
index 5f8e96f1516f..b3f9bd641688 100644
--- a/include/target/target_core_base.h
+++ b/include/target/target_core_base.h
@@ -869,6 +869,7 @@ struct se_device {
      int            queue_cnt;
      struct se_device_queue    *queues;
      struct mutex        lun_reset_mutex;
+    struct mutex        configure_mutex;
  };

  struct target_opcode_descriptor {
-- 
2.40.0


[-- Attachment #2: DMESG_LOGS --]
[-- Type: text/plain, Size: 5606 bytes --]

[  710.831838] se_dev->se_dev_ptr already set for storage object
[  710.831841] se_dev->se_dev_ptr already set for storage object
[  710.831842] se_dev->se_dev_ptr already set for storage object
[  710.831848] se_dev->se_dev_ptr already set for storage object
[  710.831855] se_dev->se_dev_ptr already set for storage object
[  710.831856] se_dev->se_dev_ptr already set for storage object
[  710.831857] se_dev->se_dev_ptr already set for storage object
[  710.831858] se_dev->se_dev_ptr already set for storage object
[  710.831859] se_dev->se_dev_ptr already set for storage object
[  710.831860] se_dev->se_dev_ptr already set for storage object
[  710.831861] se_dev->se_dev_ptr already set for storage object
[  710.831862] se_dev->se_dev_ptr already set for storage object
[  710.831863] se_dev->se_dev_ptr already set for storage object
[  710.831864] se_dev->se_dev_ptr already set for storage object
[  710.831865] se_dev->se_dev_ptr already set for storage object
[  710.831866] se_dev->se_dev_ptr already set for storage object
[  710.832103] tcmu nl cmd 1/-2 completion could not find device with dev id 163.
[  710.832405] list_add double add: new=ffff8882dfd72000, prev=ffffffff82b8f5e0, next=ffff8882dfd72000.
[  710.832494] tcmu nl cmd 1/-2 completion could not find device with dev id 163.
[  710.832842] ------------[ cut here ]------------
[  710.833022] tcmu nl cmd 1/-2 completion could not find device with dev id 164.
[  710.833306] Kernel BUG at __list_add_valid.cold+0x23/0x5b [verbose debug info unavailable]
[  710.833503] tcmu nl cmd 1/-2 completion could not find device with dev id 164.
[  710.833789] invalid opcode: 0000 [#1] PREEMPT SMP NOPTI
[  710.833987] tcmu nl cmd 1/0 completion could not find device with dev id 163.
[  710.834268] CPU: 1 PID: 368871 Comm: node Not tainted 6.2.0hocus #1
[  710.834484] tcmu nl cmd 1/-2 completion could not find device with dev id 164.
[  710.834758] RIP: 0010:__list_add_valid.cold+0x23/0x5b
[  710.834957] tcmu nl cmd 1/-2 completion could not find device with dev id 164.
[  710.835239] Code: ff ff e9 26 5a b2 ff 48 c7 c7 90 1d 1b 82 e8 f8 c4 fe ff 0f 0b 48 89 f2 4c 89 c1 48 89 fe 48 c7 c7 90 1e 1b 82 e8 e1 c4 fe ff <0f> 0b 48 89 d1 4c 89 c6 4c 89 ca 48 c7 c7 38 1e 1b 82 e8 ca c4 fe
[  710.835451] tcmu nl cmd 1/-2 completion could not find device with dev id 164.
[  710.835715] RSP: 0018:ffffc90001e3fd68 EFLAGS: 00010246
[  710.835974] tcmu nl cmd 1/-2 completion could not find device with dev id 164.
[  710.836669] tcmu nl cmd 1/0 completion could not find device with dev id 164.
[  710.836994] RAX: 0000000000000058 RBX: ffff8882dfd72018 RCX: 0000000000000000
[  710.837220] tcmu nl cmd 1/-2 completion could not find device with dev id 164.
[  710.837487] RDX: 0000000000000000 RSI: ffffffff8214e624 RDI: 00000000ffffffff
[  710.837786] tcmu daemon: command reply support 1.
[  710.838082] RBP: ffffc90001e3fd68 R08: 74705f7665645f65 R09: 733e2d7665645f65
[  710.842114] R10: 705f7665645f6573 R11: 3e2d7665645f6573 R12: 0000000000000000
[  710.842347] R13: ffff8882dfd72000 R14: ffff8882dfd72000 R15: ffff8882dfd72010
[  710.842581] FS:  00007fb5e4f7f6c0(0000) GS:ffff88841fc40000(0000) knlGS:0000000000000000
[  710.842847] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[  710.843035] CR2: 00007fb5e4f7ed28 CR3: 000000027b2ba000 CR4: 00000000003506a0
[  710.843269] Call Trace:
[  710.843361]  <TASK>
[  710.843435]  tcmu_configure_device+0x29b/0x390
[  710.843594]  target_configure_device+0x7c/0x2b0
[  710.843749]  target_dev_enable_store+0x36/0x50
[  710.843897]  configfs_write_iter+0xc5/0x130
[  710.844045]  vfs_write+0x2c3/0x3f0
[  710.844163]  ksys_write+0x66/0xf0
[  710.844275]  __x64_sys_write+0x18/0x20
[  710.844400]  do_syscall_64+0x3f/0x90
[  710.844524]  entry_SYSCALL_64_after_hwframe+0x72/0xdc
[  710.844704] RIP: 0033:0x7fb5e848711f
[  710.844824] Code: 89 54 24 18 48 89 74 24 10 89 7c 24 08 e8 39 d5 f8 ff 48 8b 54 24 18 48 8b 74 24 10 41 89 c0 8b 7c 24 08 b8 01 00 00 00 0f 05 <48> 3d 00 f0 ff ff 77 31 44 89 c7 48 89 44 24 08 e8 8c d5 f8 ff 48
[  710.845420] RSP: 002b:00007fb5e4f7ecf0 EFLAGS: 00000293 ORIG_RAX: 0000000000000001
[  710.845666] RAX: ffffffffffffffda RBX: 0000000000000000 RCX: 00007fb5e848711f
[  710.845897] RDX: 0000000000000001 RSI: 00007fb5e5a4e578 RDI: 000000000000001f
[  710.846135] RBP: 00007fb5e4f7ee20 R08: 0000000000000000 R09: 00000000ffffffff
[  710.846368] R10: 0000000000000000 R11: 0000000000000293 R12: 00007fb5e4f7f5c0
[  710.846599] R13: 00007fb5e5880128 R14: 0000000000000001 R15: 0000000000000400
[  710.846830]  </TASK>
[  710.846947] ---[ end trace 0000000000000000 ]---
[  710.847108] RIP: 0010:__list_add_valid.cold+0x23/0x5b
[  710.847279] Code: ff ff e9 26 5a b2 ff 48 c7 c7 90 1d 1b 82 e8 f8 c4 fe ff 0f 0b 48 89 f2 4c 89 c1 48 89 fe 48 c7 c7 90 1e 1b 82 e8 e1 c4 fe ff <0f> 0b 48 89 d1 4c 89 c6 4c 89 ca 48 c7 c7 38 1e 1b 82 e8 ca c4 fe
[  710.847895] RSP: 0018:ffffc90001e3fd68 EFLAGS: 00010246
[  710.848069] RAX: 0000000000000058 RBX: ffff8882dfd72018 RCX: 0000000000000000
[  710.848301] RDX: 0000000000000000 RSI: ffffffff8214e624 RDI: 00000000ffffffff
[  710.848534] RBP: ffffc90001e3fd68 R08: 74705f7665645f65 R09: 733e2d7665645f65
[  710.848766] R10: 705f7665645f6573 R11: 3e2d7665645f6573 R12: 0000000000000000
[  710.848998] R13: ffff8882dfd72000 R14: ffff8882dfd72000 R15: ffff8882dfd72010
[  710.849230] FS:  00007fb5e4f7f6c0(0000) GS:ffff88841fc40000(0000) knlGS:0000000000000000
[  710.849498] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[  710.849686] CR2: 00007fb5e4f7ed28 CR3: 000000027b2ba000 CR4: 00000000003506a0


             reply	other threads:[~2023-05-20  0:28 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-05-20  0:26 Grzegorz Uriasz [this message]
2023-05-20  8:46 ` [PATCH] scsi: target: Fix data corruption under concurrent target configuration Dmitry Bogdanov
2023-05-21 11:28   ` Grzegorz Uriasz
2023-05-22 11:27     ` Grzegorz Uriasz
2023-06-12 12:19       ` Grzegorz Uriasz

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=5f637569-36af-a8d0-e378-b27a63f08501@gmail.com \
    --to=gorbak25@gmail.com \
    --cc=dutkahugo@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-scsi@vger.kernel.org \
    --cc=martin.petersen@oracle.com \
    --cc=target-devel@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.