From cc5159a340fcda23a9c192ee9d8ef6f44ec3ed57 Mon Sep 17 00:00:00 2001 From: Vladislav Bolkhovitin Date: Sat, 10 Dec 2011 04:26:46 +0000 Subject: [PATCH] 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 ........ 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 ........ 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 ........ r3940 | vlnb | 2011-11-29 17:00:23 -0500 (Tue, 29 Nov 2011) | 66 lines Fixes problem reported by Lev Vainblat : 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: [] 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:[] [] 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] [] iscsi_sess_force_close_store+0x58/0xd0 [iscsi_scst] [ 193.160112] [] scst_store+0x1c/0x20 [scst] [ 193.160112] [] sysfs_write_file+0xd1/0x160 [ 193.160112] [] vfs_write+0xc6/0x180 [ 193.160112] [] sys_write+0x51/0x90 [ 193.160112] [] 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 [] iscsi_sess_force_close+0xc4/0x170 [iscsi_scst] [ 193.160112] RSP [ 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 --- scst/src/dev_handlers/scst_vdisk.c | 126 ++++++++--------------------- scst/src/scst_lib.c | 16 ++-- scst/src/scst_pres.c | 3 +- 3 files changed, 45 insertions(+), 100 deletions(-) diff --git a/scst/src/dev_handlers/scst_vdisk.c b/scst/src/dev_handlers/scst_vdisk.c index 4423bd96b..396e168c0 100644 --- 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); diff --git a/scst/src/scst_lib.c b/scst/src/scst_lib.c index 514a07991..37043611f 100644 --- 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) diff --git a/scst/src/scst_pres.c b/scst/src/scst_pres.c index 689ffa691..6a207cfab 100644 --- 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)