Merged revisions 3937-3940 via svnmerge from

https://vlnb@scst.svn.sourceforge.net/svnroot/scst/trunk ........ r3937 | vlnb | 2011-11-22 21:34:09 -0500 (Tue, 22 Nov 2011) | 6 lines Instead of repeating the same name lookup several times, use vdev_find() where appropriate. Signed-off-by: Bart Van Assche <bvanassche@acm.org> ........ r3938 | vlnb | 2011-11-22 21:35:44 -0500 (Tue, 22 Nov 2011) | 37 lines Avoid that a warning like the one below is triggered when adding a nullio, fileio or blockio device with a name that already exists: WARNING: at fs/sysfs/dir.c:481 sysfs_add_one+0x95/0xd0() sysfs: cannot create duplicate filename '/kernel/scst_tgt/devices/disk09' Call Trace: warn_slowpath_common+0x72/0xa0 ? sysfs_add_one+0x95/0xd0 ? sysfs_add_one+0x95/0xd0 warn_slowpath_fmt+0x33/0x40 sysfs_add_one+0x95/0xd0 create_dir+0x61/0xa0 sysfs_create_dir+0x71/0xb0 kobject_add_internal+0x9b/0x250 ? kvasprintf+0x41/0x50 kobject_init_and_add+0x39/0x60 scst_dev_sysfs_create+0x6e/0x250 [scst] scst_register_virtual_device+0x1f4/0x3e0 [scst] vdev_nullio_add_device+0x14f/0x1c0 [scst_vdisk] ? vdisk_add_nullio_device+0x5a/0x110 [scst_vdisk] vdisk_add_nullio_device+0xf1/0x110 [scst_vdisk] ? scst_check_grab_devt_ptr.clone.0+0xa8/0x170 [scst] scst_process_devt_mgmt_store+0xa9/0x2a0 [scst] ? spin_unlock.clone.11+0x12/0x20 [scst] scst_devt_mgmt_store_work_fn+0x13/0x20 [scst] scst_process_sysfs_works+0x7f/0x170 [scst] ? spin_lock.clone.10+0x12/0x20 [scst] sysfs_work_thread_fn+0x103/0x270 [scst] ? wake_up_bit+0x30/0x30 ? prepare_to_wait_exclusive_head.clone.16+0x60/0x60 [scst] kthread+0x84/0x90 ? __init_kthread_worker+0x60/0x60 kernel_thread_helper+0x6/0x10 Signed-off-by: Bart Van Assche <bvanassche@acm.org> ........ r3939 | vlnb | 2011-11-22 21:53:09 -0500 (Tue, 22 Nov 2011) | 3 lines Check error code of path_lookup() on pre-39 kernels. Suggested by Lev Vainblat <lev@zadarastorage.com> ........ r3940 | vlnb | 2011-11-29 17:00:23 -0500 (Tue, 29 Nov 2011) | 66 lines Fixes problem reported by Lev Vainblat <lev@zadarastorage.com>: The access to the session sysfs during logout may lead to the kernel oops. In iscsi_unreg_sess_done() the iscsi_session is freed, so scst_sess->tgt_priv (that previously pointed to the iscsi_session) becomes stale. The session is still accessible in sysfs until scst_sess_sysfs_del() call, but accessing to the session via sysfs may return invalid values or crash the kernel: $ cat /sys/kernel/scst_tgt/targets/iscsi/T1/sessions/iqn.2011-04.com.zadarastorage:316:vc-0/sid 10000123d0200 [ 178.769460] iscsi-scst[895] logout_exec[2718]: Logout received from initiator iqn.2011-04.com.zadarastorage:316:vc-0 [ 178.769482] iscsi-scst[897] cmnd_tx_end[2900]: Closing connection at initiator's iqn.2011-04.com.zadarastorage:316:vc-0 request [ 178.769546] scst: TM fn 6 [ 178.769623] scst: TM fn 6 finished, status 0 [ 178.770075] scst[1645] scst_cmd_thread[4379]: Processing thread D10_0 (PID 1645) finished [ 178.770111] scst[1646] scst_cmd_thread[4379]: Processing thread D10_1 (PID 1646) finished [ 178.770131] scst[1647] scst_cmd_thread[4379]: Processing thread D10_2 (PID 1647) finished [ 178.770150] scst[1648] scst_cmd_thread[4379]: Processing thread D10_3 (PID 1648) finished [ 178.770168] scst[1649] scst_cmd_thread[4379]: Processing thread D10_4 (PID 1649) finished [ 178.770186] scst[1650] scst_cmd_thread[4379]: Processing thread D10_5 (PID 1650) finished [ 178.770204] scst[1651] scst_cmd_thread[4379]: Processing thread D10_6 (PID 1651) finished [ 178.770223] scst[1652] scst_cmd_thread[4379]: Processing thread D10_7 (PID 1652) finished $ cat /sys/kernel/scst_tgt/targets/iscsi/T1/sessions/iqn.2011-04.com.zadarastorage:316:vc-0/sid 0 $ echo 1> /sys/kernel/scst_tgt/targets/iscsi/T1/sessions/iqn.2011-04.com.zadarastorage:316:vc-0/force_close [ 193.150420] iscsi-scst[1555] iscsi_sess_force_close[389]: Deleting session 0 with initiator (null) (ffff880078316000) [ 193.150461] BUG: unable to handle kernel NULL pointer dereference at (null) [ 193.150545] IP: [<ffffffffa034d034>] iscsi_sess_force_close+0xc4/0x170 [iscsi_scst] [ 193.150628] PGD 78218067 PUD 79666067 PMD 0 [ 193.150678] Oops: 0000 [#1] SMP [ 193.150716] last sysfs file: /sys/kernel/scst_tgt/targets/iscsi/T1/sessions/iqn.2011-04.com.zadarastorage:316:vc-0/force_close [ 193.150824] CPU 0 [ 193.150844] Modules linked in: ib_iser rdma_cm ib_cm iw_cm ib_sa ib_mad ib_core ib_addr iscsi_tcp libiscsi_tcp libiscsi scsi_transport_iscsi dm_iostat scst_vdisk iscsi_scst scst libcrc32c nfsd psmouse exportfs serio_raw virtio_ball oon nfs lockd fscache nfs_acl auth_rpcgss i2c_piix4 sunrpc lp parport floppy raid10 raid456 async_pq async_xor xor async_memcpy async_raid6_recov ixgbevf raid6_pq async_tx raid1 raid0 multipath linear [last unloaded: scsi_transport_is csi] [ 193.151359] [ 193.151377] Pid: 1555, comm: bash Not tainted 2.6.38-8-server #42-Ubuntu Bochs Bochs [ 193.151456] RIP: 0010:[<ffffffffa034d034>] [<ffffffffa034d034>] iscsi_sess_force_close+0xc4/0x170 [iscsi_scst] [ 193.151556] RSP: 0018:ffff88007a247e28 EFLAGS: 00010287 [ 193.151604] RAX: 000000000000007f RBX: fffffffffffffe30 RCX: 000000000003ffff [ 193.153372] RDX: 0000000000000000 RSI: 0000000000000086 RDI: 0000000000000246 [ 193.155115] RBP: ffff88007a247e68 R08: 0000000000000036 R09: 000000000000b874 [ 193.156878] R10: 0000000000000000 R11: 0000000000000001 R12: ffff8800783170a0 [ 193.158640] R13: ffff88007a247fd8 R14: ffff88007828c4a0 R15: ffff880078573600 [ 193.160112] FS: 00007f9d23092720(0000) GS:ffff88007fc00000(0000) knlGS:0000000000000000 [ 193.160112] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 [ 193.160112] CR2: 0000000000000000 CR3: 00000000787e6000 CR4: 00000000000006f0 [ 193.160112] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000 [ 193.160112] DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400 [ 193.160112] Process bash (pid: 1555, threadinfo ffff88007a246000, task ffff88007828c4a0) [ 193.160112] Stack: [ 193.160112] 0000000000000000 ffff880078316000 0000000000000000 0000000000000000 [ 193.160112] ffff880078316000 0000000000000001 ffff8800370ea5a0 ffffffffa02de9c0 [ 193.160112] ffff88007a247e88 ffffffffa034d138 ffff880078573620 ffff88007a247f48 [ 193.160112] Call Trace: [ 193.160112] [<ffffffffa034d138>] iscsi_sess_force_close_store+0x58/0xd0 [iscsi_scst] [ 193.160112] [<ffffffffa02bf31c>] scst_store+0x1c/0x20 [scst] [ 193.160112] [<ffffffff811d35f1>] sysfs_write_file+0xd1/0x160 [ 193.160112] [<ffffffff811652e6>] vfs_write+0xc6/0x180 [ 193.160112] [<ffffffff81165601>] sys_write+0x51/0x90 [ 193.160112] [<ffffffff8100bfc2>] system_call_fastpath+0x16/0x1b [ 193.160112] Code: 1f 84 00 00 00 00 00 f6 05 ea ef 00 00 08 75 68 48 89 df be 03 00 00 00 e8 ca d3 ff ff 48 8b 9b d0 01 00 00 48 81 eb d0 01 00 00 <48> 8b 83 d0 01 00 00 0f 18 08 48 8d 83 d0 01 00 00 4c 39 e0 75 [ 193.160112] RIP [<ffffffffa034d034>] iscsi_sess_force_close+0xc4/0x170 [iscsi_scst] [ 193.160112] RSP <ffff88007a247e28> [ 193.160112] CR2: 0000000000000000 [ 193.200380] ---[ end trace 1988466f0a8da036 ]--- ........ git-svn-id: http://svn.code.sf.net/p/scst/svn/branches/2.1.0.x@3973 d57e44dd-8a1f-0410-8b47-8ef2f437770f
2026-05-17 10:41:26 +00:00 · 2011-12-10 04:26:46 +00:00
parent dadceece11
commit cc5159a340
3 changed files with 45 additions and 100 deletions
--- a/scst/src/dev_handlers/scst_vdisk.c
+++ b/scst/src/dev_handlers/scst_vdisk.c
@@ -707,11 +707,30 @@ out:
 	return res;
 }

+/* scst_vdisk_mutex supposed to be held */
+static struct scst_vdisk_dev *vdev_find(const char *name)
+{
+	struct scst_vdisk_dev *res, *vv;
+
+	TRACE_ENTRY();
+
+	res = NULL;
+	list_for_each_entry(vv, &vdev_list, vdev_list_entry) {
+		if (strcmp(vv->name, name) == 0) {
+			res = vv;
+			break;
+		}
+	}
+
+	TRACE_EXIT_HRES((unsigned long)res);
+	return res;
+}
+
 static int vdisk_attach(struct scst_device *dev)
 {
 	int res = 0;
 	loff_t err;
-	struct scst_vdisk_dev *virt_dev = NULL, *vv;
+	struct scst_vdisk_dev *virt_dev;

 	TRACE_ENTRY();

@@ -727,13 +746,7 @@ static int vdisk_attach(struct scst_device *dev)
 	 * scst_vdisk_mutex must be already taken before
 	 * scst_register_virtual_device()
 	 */
-	list_for_each_entry(vv, &vdev_list, vdev_list_entry) {
-		if (strcmp(vv->name, dev->virt_name) == 0) {
-			virt_dev = vv;
-			break;
-		}
-	}
-
+	virt_dev = vdev_find(dev->virt_name);
 	if (virt_dev == NULL) {
 		PRINT_ERROR("Device %s not found", dev->virt_name);
 		res = -EINVAL;
@@ -3366,10 +3379,14 @@ out:
 static int vdev_create(struct scst_dev_type *devt,
 	const char *name, struct scst_vdisk_dev **res_virt_dev)
 {
-	int res = 0;
+	int res;
 	struct scst_vdisk_dev *virt_dev;
 	uint64_t dev_id_num;

+	res = -EEXIST;
+	if (vdev_find(name))
+		goto out;
+
 	virt_dev = kzalloc(sizeof(*virt_dev), GFP_KERNEL);
 	if (virt_dev == NULL) {
 		PRINT_ERROR("Allocation of virtual device %s failed",
@@ -3406,6 +3423,7 @@ static int vdev_create(struct scst_dev_type *devt,
 	TRACE_DBG("usn %s", virt_dev->usn);

 	*res_virt_dev = virt_dev;
+	res = 0;

 out:
 	return res;
@@ -3422,25 +3440,6 @@ static void vdev_destroy(struct scst_vdisk_dev *virt_dev)
 	return;
 }

-/* scst_vdisk_mutex supposed to be held */
-static struct scst_vdisk_dev *vdev_find(const char *name)
-{
-	struct scst_vdisk_dev *res, *vv;
-
-	TRACE_ENTRY();
-
-	res = NULL;
-	list_for_each_entry(vv, &vdev_list, vdev_list_entry) {
-		if (strcmp(vv->name, name) == 0) {
-			res = vv;
-			break;
-		}
-	}
-
-	TRACE_EXIT_HRES((unsigned long)res);
-	return res;
-}
-
 #ifndef CONFIG_SCST_PROC

 static int vdev_parse_add_dev_params(struct scst_vdisk_dev *virt_dev,
@@ -4661,7 +4660,7 @@ static int vdisk_write_proc(char *buffer, char **start, off_t offset,
 {
 	int res = 0, action;
 	char *p, *name, *filename, *i_buf, *t10_dev_id;
-	struct scst_vdisk_dev *virt_dev, *vv;
+	struct scst_vdisk_dev *virt_dev;
 	uint32_t block_size = DEF_DISK_BLOCKSIZE;
 	int block_shift = DEF_DISK_BLOCKSIZE_SHIFT;
 	size_t slen;
@@ -4724,21 +4723,6 @@ static int vdisk_write_proc(char *buffer, char **start, off_t offset,

 	if (action == 1) {
 		/* open */
-		virt_dev = NULL;
-		list_for_each_entry(vv, &vdev_list,
-					vdev_list_entry) {
-			if (strcmp(vv->name, name) == 0) {
-				virt_dev = vv;
-				break;
-			}
-		}
-		if (virt_dev) {
-			PRINT_ERROR("Virtual device with name "
-				   "%s already exist", name);
-			res = -EINVAL;
-			goto out_up;
-		}
-
 		while (isspace(*p) && *p != '\0')
 			p++;
 		filename = p;
@@ -4862,14 +4846,7 @@ static int vdisk_write_proc(char *buffer, char **start, off_t offset,
 			vdev_get_filename(virt_dev), virt_dev->virt_id,
 			virt_dev->block_size);
 	} else if (action == 0) {	/* close */
-		virt_dev = NULL;
-		list_for_each_entry(vv, &vdev_list,
-					vdev_list_entry) {
-			if (strcmp(vv->name, name) == 0) {
-				virt_dev = vv;
-				break;
-			}
-		}
+		virt_dev = vdev_find(name);
 		if (virt_dev == NULL) {
 			PRINT_ERROR("Device %s not found", name);
 			res = -EINVAL;
@@ -4877,14 +4854,7 @@ static int vdisk_write_proc(char *buffer, char **start, off_t offset,
 		}
 		vdev_del_device(virt_dev);
 	} else if (action == 2) {	/* resync_size */
-		virt_dev = NULL;
-		list_for_each_entry(vv, &vdev_list,
-					vdev_list_entry) {
-			if (strcmp(vv->name, name) == 0) {
-				virt_dev = vv;
-				break;
-			}
-		}
+		virt_dev = vdev_find(name);
 		if (virt_dev == NULL) {
 			PRINT_ERROR("Device %s not found", name);
 			res = -EINVAL;
@@ -4895,14 +4865,7 @@ static int vdisk_write_proc(char *buffer, char **start, off_t offset,
 		if (res != 0)
 			goto out_up;
 	} else if (action == 3) {	/* set T10 device id */
-		virt_dev = NULL;
-		list_for_each_entry(vv, &vdev_list,
-					vdev_list_entry) {
-			if (strcmp(vv->name, name) == 0) {
-				virt_dev = vv;
-				break;
-			}
-		}
+		virt_dev = vdev_find(name);
 		if (virt_dev == NULL) {
 			PRINT_ERROR("Device %s not found", name);
 			res = -EINVAL;
@@ -4993,25 +4956,11 @@ out:
 /* scst_vdisk_mutex supposed to be held */
 static int vcdrom_open(char *p, char *name)
 {
-	struct scst_vdisk_dev *virt_dev, *vv;
+	struct scst_vdisk_dev *virt_dev;
 	char *filename;
 	int res = 0;
 	int cdrom_empty;

-	virt_dev = NULL;
-	list_for_each_entry(vv, &vdev_list, vdev_list_entry) {
-		if (strcmp(vv->name, name) == 0) {
-			virt_dev = vv;
-			break;
-		}
-	}
-	if (virt_dev) {
-		PRINT_ERROR("Virtual device with name "
-		       "%s already exist", name);
-		res = -EINVAL;
-		goto out;
-	}
-
 	while (isspace(*p) && *p != '\0')
 		p++;
 	filename = p;
@@ -5076,17 +5025,10 @@ out_free_vdev:
 /* scst_vdisk_mutex supposed to be held */
 static int vcdrom_close(char *name)
 {
-	struct scst_vdisk_dev *virt_dev, *vv;
+	struct scst_vdisk_dev *virt_dev;
 	int res = 0;

-	virt_dev = NULL;
-	list_for_each_entry(vv, &vdev_list, vdev_list_entry) {
-		if (strcmp(vv->name, name) == 0) {
-			virt_dev = vv;
-			break;
-		}
-	}
-
+	virt_dev = vdev_find(name);
 	if (virt_dev == NULL) {
 		PRINT_ERROR("Virtual device with name "
 		       "%s not found", name);
--- a/scst/src/scst_lib.c
+++ b/scst/src/scst_lib.c
@@ -4009,11 +4009,18 @@ void scst_free_session(struct scst_session *sess)

 	scst_sess_free_tgt_devs(sess);

-#ifndef CONFIG_SCST_PROC
 	mutex_unlock(&scst_mutex);
+
+#ifndef CONFIG_SCST_PROC
 	scst_sess_sysfs_del(sess);
-	mutex_lock(&scst_mutex);
 #endif
+	if (sess->unreg_done_fn) {
+		TRACE_DBG("Calling unreg_done_fn(%p)", sess);
+		sess->unreg_done_fn(sess);
+		TRACE_DBG("%s", "unreg_done_fn() returned");
+	}
+
+	mutex_lock(&scst_mutex);

 	/*
 	 * The lists delete must be after sysfs del. Otherwise it would break
@@ -4064,11 +4071,6 @@ void scst_free_session_callback(struct scst_session *sess)
 	sess->shut_phase = SCST_SESS_SPH_UNREG_DONE_CALLING;
 	mutex_unlock(&scst_mutex);

-	if (sess->unreg_done_fn) {
-		TRACE_DBG("Calling unreg_done_fn(%p)", sess);
-		sess->unreg_done_fn(sess);
-		TRACE_DBG("%s", "unreg_done_fn() returned");
-	}
 	scst_free_session(sess);

 	if (c)
--- a/scst/src/scst_pres.c
+++ b/scst/src/scst_pres.c
@@ -1183,7 +1183,8 @@ static int scst_pr_check_pr_path(void)

 #if LINUX_VERSION_CODE < KERNEL_VERSION(2, 6, 39)
 	res = path_lookup(SCST_PR_DIR, 0, &nd);
-	scst_pr_path_put(&nd);
+	if (res == 0)
+		scst_pr_path_put(&nd);
 #else
 	res = kern_path(SCST_PR_DIR, 0, &path);
 	if (res == 0)