Commit aea9058801c0acfa2831af1714da412dfb0018c2

Authored by Jun'ichi Nomura
Committed by Alasdair G Kergon
1 parent 5642b8a61a

dm: path selector use module refcount directly

Fix refcount corruption in dm-path-selector

Refcounting with non-atomic ops under shared lock will corrupt the counter
in multi-processor system and may trigger BUG_ON().
Use module refcount.
# same approach as dm-target-use-module-refcount-directly.patch here
# https://www.redhat.com/archives/dm-devel/2008-December/msg00075.html

Typical oops:
  kernel BUG at linux-2.6.29-rc3/drivers/md/dm-path-selector.c:90!
  Pid: 11148, comm: dmsetup Not tainted 2.6.29-rc3-nm #1
  dm_put_path_selector+0x4d/0x61 [dm_multipath]
  Call Trace:
   [<ffffffffa031d3f9>] free_priority_group+0x33/0xb3 [dm_multipath]
   [<ffffffffa031d4aa>] free_multipath+0x31/0x67 [dm_multipath]
   [<ffffffffa031d50d>] multipath_dtr+0x2d/0x32 [dm_multipath]
   [<ffffffffa015d6c2>] dm_table_destroy+0x64/0xd8 [dm_mod]
   [<ffffffffa015b73a>] __unbind+0x46/0x4b [dm_mod]
   [<ffffffffa015b79f>] dm_swap_table+0x60/0x14d [dm_mod]
   [<ffffffffa015f963>] dev_suspend+0xfd/0x177 [dm_mod]
   [<ffffffffa0160250>] dm_ctl_ioctl+0x24c/0x29c [dm_mod]
   [<ffffffff80288cd3>] ? get_page_from_freelist+0x49c/0x61d
   [<ffffffffa015f866>] ? dev_suspend+0x0/0x177 [dm_mod]
   [<ffffffff802bf05c>] vfs_ioctl+0x2a/0x77
   [<ffffffff802bf4f1>] do_vfs_ioctl+0x448/0x4a0
   [<ffffffff802bf5a0>] sys_ioctl+0x57/0x7a
   [<ffffffff8020c05b>] system_call_fastpath+0x16/0x1b

Cc: stable@kernel.org
Signed-off-by: Jun'ichi Nomura <j-nomura@ce.jp.nec.com>
Signed-off-by: Alasdair G Kergon <agk@redhat.com>

Showing 1 changed file with 3 additions and 18 deletions Inline Diff

drivers/md/dm-path-selector.c
1 /* 1 /*
2 * Copyright (C) 2003 Sistina Software. 2 * Copyright (C) 2003 Sistina Software.
3 * Copyright (C) 2004 Red Hat, Inc. All rights reserved. 3 * Copyright (C) 2004 Red Hat, Inc. All rights reserved.
4 * 4 *
5 * Module Author: Heinz Mauelshagen 5 * Module Author: Heinz Mauelshagen
6 * 6 *
7 * This file is released under the GPL. 7 * This file is released under the GPL.
8 * 8 *
9 * Path selector registration. 9 * Path selector registration.
10 */ 10 */
11 11
12 #include <linux/device-mapper.h> 12 #include <linux/device-mapper.h>
13 13
14 #include "dm-path-selector.h" 14 #include "dm-path-selector.h"
15 15
16 #include <linux/slab.h> 16 #include <linux/slab.h>
17 17
18 struct ps_internal { 18 struct ps_internal {
19 struct path_selector_type pst; 19 struct path_selector_type pst;
20
21 struct list_head list; 20 struct list_head list;
22 long use;
23 }; 21 };
24 22
25 #define pst_to_psi(__pst) container_of((__pst), struct ps_internal, pst) 23 #define pst_to_psi(__pst) container_of((__pst), struct ps_internal, pst)
26 24
27 static LIST_HEAD(_path_selectors); 25 static LIST_HEAD(_path_selectors);
28 static DECLARE_RWSEM(_ps_lock); 26 static DECLARE_RWSEM(_ps_lock);
29 27
30 static struct ps_internal *__find_path_selector_type(const char *name) 28 static struct ps_internal *__find_path_selector_type(const char *name)
31 { 29 {
32 struct ps_internal *psi; 30 struct ps_internal *psi;
33 31
34 list_for_each_entry(psi, &_path_selectors, list) { 32 list_for_each_entry(psi, &_path_selectors, list) {
35 if (!strcmp(name, psi->pst.name)) 33 if (!strcmp(name, psi->pst.name))
36 return psi; 34 return psi;
37 } 35 }
38 36
39 return NULL; 37 return NULL;
40 } 38 }
41 39
42 static struct ps_internal *get_path_selector(const char *name) 40 static struct ps_internal *get_path_selector(const char *name)
43 { 41 {
44 struct ps_internal *psi; 42 struct ps_internal *psi;
45 43
46 down_read(&_ps_lock); 44 down_read(&_ps_lock);
47 psi = __find_path_selector_type(name); 45 psi = __find_path_selector_type(name);
48 if (psi) { 46 if (psi && !try_module_get(psi->pst.module))
49 if ((psi->use == 0) && !try_module_get(psi->pst.module)) 47 psi = NULL;
50 psi = NULL;
51 else
52 psi->use++;
53 }
54 up_read(&_ps_lock); 48 up_read(&_ps_lock);
55 49
56 return psi; 50 return psi;
57 } 51 }
58 52
59 struct path_selector_type *dm_get_path_selector(const char *name) 53 struct path_selector_type *dm_get_path_selector(const char *name)
60 { 54 {
61 struct ps_internal *psi; 55 struct ps_internal *psi;
62 56
63 if (!name) 57 if (!name)
64 return NULL; 58 return NULL;
65 59
66 psi = get_path_selector(name); 60 psi = get_path_selector(name);
67 if (!psi) { 61 if (!psi) {
68 request_module("dm-%s", name); 62 request_module("dm-%s", name);
69 psi = get_path_selector(name); 63 psi = get_path_selector(name);
70 } 64 }
71 65
72 return psi ? &psi->pst : NULL; 66 return psi ? &psi->pst : NULL;
73 } 67 }
74 68
75 void dm_put_path_selector(struct path_selector_type *pst) 69 void dm_put_path_selector(struct path_selector_type *pst)
76 { 70 {
77 struct ps_internal *psi; 71 struct ps_internal *psi;
78 72
79 if (!pst) 73 if (!pst)
80 return; 74 return;
81 75
82 down_read(&_ps_lock); 76 down_read(&_ps_lock);
83 psi = __find_path_selector_type(pst->name); 77 psi = __find_path_selector_type(pst->name);
84 if (!psi) 78 if (!psi)
85 goto out; 79 goto out;
86 80
87 if (--psi->use == 0) 81 module_put(psi->pst.module);
88 module_put(psi->pst.module);
89
90 BUG_ON(psi->use < 0);
91
92 out: 82 out:
93 up_read(&_ps_lock); 83 up_read(&_ps_lock);
94 } 84 }
95 85
96 static struct ps_internal *_alloc_path_selector(struct path_selector_type *pst) 86 static struct ps_internal *_alloc_path_selector(struct path_selector_type *pst)
97 { 87 {
98 struct ps_internal *psi = kzalloc(sizeof(*psi), GFP_KERNEL); 88 struct ps_internal *psi = kzalloc(sizeof(*psi), GFP_KERNEL);
99 89
100 if (psi) 90 if (psi)
101 psi->pst = *pst; 91 psi->pst = *pst;
102 92
103 return psi; 93 return psi;
104 } 94 }
105 95
106 int dm_register_path_selector(struct path_selector_type *pst) 96 int dm_register_path_selector(struct path_selector_type *pst)
107 { 97 {
108 int r = 0; 98 int r = 0;
109 struct ps_internal *psi = _alloc_path_selector(pst); 99 struct ps_internal *psi = _alloc_path_selector(pst);
110 100
111 if (!psi) 101 if (!psi)
112 return -ENOMEM; 102 return -ENOMEM;
113 103
114 down_write(&_ps_lock); 104 down_write(&_ps_lock);
115 105
116 if (__find_path_selector_type(pst->name)) { 106 if (__find_path_selector_type(pst->name)) {
117 kfree(psi); 107 kfree(psi);
118 r = -EEXIST; 108 r = -EEXIST;
119 } else 109 } else
120 list_add(&psi->list, &_path_selectors); 110 list_add(&psi->list, &_path_selectors);
121 111
122 up_write(&_ps_lock); 112 up_write(&_ps_lock);
123 113
124 return r; 114 return r;
125 } 115 }
126 116
127 int dm_unregister_path_selector(struct path_selector_type *pst) 117 int dm_unregister_path_selector(struct path_selector_type *pst)
128 { 118 {
129 struct ps_internal *psi; 119 struct ps_internal *psi;
130 120
131 down_write(&_ps_lock); 121 down_write(&_ps_lock);
132 122
133 psi = __find_path_selector_type(pst->name); 123 psi = __find_path_selector_type(pst->name);
134 if (!psi) { 124 if (!psi) {
135 up_write(&_ps_lock); 125 up_write(&_ps_lock);
136 return -EINVAL; 126 return -EINVAL;
137 }
138
139 if (psi->use) {
140 up_write(&_ps_lock);
141 return -ETXTBSY;
142 } 127 }
143 128
144 list_del(&psi->list); 129 list_del(&psi->list);
145 130
146 up_write(&_ps_lock); 131 up_write(&_ps_lock);
147 132
148 kfree(psi); 133 kfree(psi);
149 134
150 return 0; 135 return 0;
151 } 136 }
152 137
153 EXPORT_SYMBOL_GPL(dm_register_path_selector); 138 EXPORT_SYMBOL_GPL(dm_register_path_selector);
154 EXPORT_SYMBOL_GPL(dm_unregister_path_selector); 139 EXPORT_SYMBOL_GPL(dm_unregister_path_selector);
155 140