]>
Commit | Line | Data |
---|---|---|
00e5a55c BS |
1 | From: "Jun'ichi Nomura" <j-nomura@ce.jp.nec.com> |
2 | Subject: dm-path-selector: fix refcount corruption | |
3 | ||
4 | Hi, | |
5 | ||
6 | Refcounting of path-selector module is not safe in SMP environment. | |
7 | The counter may corrupt and trigger BUG() like this: | |
8 | kernel BUG at linux-2.6.29-rc3/drivers/md/dm-path-selector.c:90! | |
9 | though it's rare under normal usage. | |
10 | ||
11 | The bug is here: | |
12 | void dm_put_path_selector(struct path_selector_type *pst) | |
13 | { | |
14 | ... | |
15 | down_read(&_ps_lock); | |
16 | psi = __find_path_selector_type(pst->name); | |
17 | if (!psi) | |
18 | goto out; | |
19 | ||
20 | if (--psi->use == 0) | |
21 | module_put(psi->pst.module); | |
22 | ||
23 | BUG_ON(psi->use < 0); | |
24 | ||
25 | The code manipulates the counter without exclusive lock or atomic ops. | |
26 | So if 2 processors come in, the counter may corrupt. | |
27 | ||
28 | While it could be fixed using atomic ops for the counter manipulation, | |
29 | we can just drop the 'use' counter like Cheng Renquan did for dm-target: | |
30 | https://www.redhat.com/archives/dm-devel/2008-December/msg00075.html | |
31 | ||
32 | (Actually, without his patch, dm-target.c hits the same problem.) | |
33 | ||
34 | Signed-off-by: Hannes Reinecke <hare@suse.de> | |
35 | ||
36 | -- | |
37 | Jun'ichi Nomura, NEC Corporation | |
38 | ||
39 | ||
40 | Fix refcount corruption in dm-path-selector | |
41 | ||
42 | Refcounting with non-atomic ops under shared lock will corrupt the counter | |
43 | in multi-processor system and may trigger BUG_ON(). | |
44 | Use module refcount. | |
45 | # same approach as dm-target-use-module-refcount-directly.patch here | |
46 | # https://www.redhat.com/archives/dm-devel/2008-December/msg00075.html | |
47 | ||
48 | Typical oops: | |
49 | kernel BUG at linux-2.6.29-rc3/drivers/md/dm-path-selector.c:90! | |
50 | Pid: 11148, comm: dmsetup Not tainted 2.6.29-rc3-nm #1 | |
51 | dm_put_path_selector+0x4d/0x61 [dm_multipath] | |
52 | Call Trace: | |
53 | [<ffffffffa031d3f9>] free_priority_group+0x33/0xb3 [dm_multipath] | |
54 | [<ffffffffa031d4aa>] free_multipath+0x31/0x67 [dm_multipath] | |
55 | [<ffffffffa031d50d>] multipath_dtr+0x2d/0x32 [dm_multipath] | |
56 | [<ffffffffa015d6c2>] dm_table_destroy+0x64/0xd8 [dm_mod] | |
57 | [<ffffffffa015b73a>] __unbind+0x46/0x4b [dm_mod] | |
58 | [<ffffffffa015b79f>] dm_swap_table+0x60/0x14d [dm_mod] | |
59 | [<ffffffffa015f963>] dev_suspend+0xfd/0x177 [dm_mod] | |
60 | [<ffffffffa0160250>] dm_ctl_ioctl+0x24c/0x29c [dm_mod] | |
61 | [<ffffffff80288cd3>] ? get_page_from_freelist+0x49c/0x61d | |
62 | [<ffffffffa015f866>] ? dev_suspend+0x0/0x177 [dm_mod] | |
63 | [<ffffffff802bf05c>] vfs_ioctl+0x2a/0x77 | |
64 | [<ffffffff802bf4f1>] do_vfs_ioctl+0x448/0x4a0 | |
65 | [<ffffffff802bf5a0>] sys_ioctl+0x57/0x7a | |
66 | [<ffffffff8020c05b>] system_call_fastpath+0x16/0x1b | |
67 | ||
68 | Signed-off-by: Jun'ichi Nomura <j-nomura@ce.jp.nec.com> | |
69 | --- | |
70 | drivers/md/dm-path-selector.c | 21 +++------------------ | |
71 | 1 file changed, 3 insertions(+), 18 deletions(-) | |
72 | ||
73 | --- a/drivers/md/dm-path-selector.c | |
74 | +++ b/drivers/md/dm-path-selector.c | |
75 | @@ -16,9 +16,7 @@ | |
76 | ||
77 | struct ps_internal { | |
78 | struct path_selector_type pst; | |
79 | - | |
80 | struct list_head list; | |
81 | - long use; | |
82 | }; | |
83 | ||
84 | #define pst_to_psi(__pst) container_of((__pst), struct ps_internal, pst) | |
85 | @@ -44,12 +42,8 @@ static struct ps_internal *get_path_sele | |
86 | ||
87 | down_read(&_ps_lock); | |
88 | psi = __find_path_selector_type(name); | |
89 | - if (psi) { | |
90 | - if ((psi->use == 0) && !try_module_get(psi->pst.module)) | |
91 | - psi = NULL; | |
92 | - else | |
93 | - psi->use++; | |
94 | - } | |
95 | + if (psi && !try_module_get(psi->pst.module)) | |
96 | + psi = NULL; | |
97 | up_read(&_ps_lock); | |
98 | ||
99 | return psi; | |
100 | @@ -83,11 +77,7 @@ void dm_put_path_selector(struct path_se | |
101 | if (!psi) | |
102 | goto out; | |
103 | ||
104 | - if (--psi->use == 0) | |
105 | - module_put(psi->pst.module); | |
106 | - | |
107 | - BUG_ON(psi->use < 0); | |
108 | - | |
109 | + module_put(psi->pst.module); | |
110 | out: | |
111 | up_read(&_ps_lock); | |
112 | } | |
113 | @@ -135,11 +125,6 @@ int dm_unregister_path_selector(struct p | |
114 | return -EINVAL; | |
115 | } | |
116 | ||
117 | - if (psi->use) { | |
118 | - up_write(&_ps_lock); | |
119 | - return -ETXTBSY; | |
120 | - } | |
121 | - | |
122 | list_del(&psi->list); | |
123 | ||
124 | up_write(&_ps_lock); |