]>
Commit | Line | Data |
---|---|---|
fb13cba2 GKH |
1 | From 3895dbf8985f656675b5bde610723a29cbce3fa7 Mon Sep 17 00:00:00 2001 |
2 | From: "Eric W. Biederman" <ebiederm@xmission.com> | |
3 | Date: Tue, 3 Jan 2017 14:18:43 +1300 | |
4 | Subject: mnt: Protect the mountpoint hashtable with mount_lock | |
5 | ||
6 | From: Eric W. Biederman <ebiederm@xmission.com> | |
7 | ||
8 | commit 3895dbf8985f656675b5bde610723a29cbce3fa7 upstream. | |
9 | ||
10 | Protecting the mountpoint hashtable with namespace_sem was sufficient | |
11 | until a call to umount_mnt was added to mntput_no_expire. At which | |
12 | point it became possible for multiple calls of put_mountpoint on | |
13 | the same hash chain to happen on the same time. | |
14 | ||
15 | Kristen Johansen <kjlx@templeofstupid.com> reported: | |
16 | > This can cause a panic when simultaneous callers of put_mountpoint | |
17 | > attempt to free the same mountpoint. This occurs because some callers | |
18 | > hold the mount_hash_lock, while others hold the namespace lock. Some | |
19 | > even hold both. | |
20 | > | |
21 | > In this submitter's case, the panic manifested itself as a GP fault in | |
22 | > put_mountpoint() when it called hlist_del() and attempted to dereference | |
23 | > a m_hash.pprev that had been poisioned by another thread. | |
24 | ||
25 | Al Viro observed that the simple fix is to switch from using the namespace_sem | |
26 | to the mount_lock to protect the mountpoint hash table. | |
27 | ||
28 | I have taken Al's suggested patch moved put_mountpoint in pivot_root | |
29 | (instead of taking mount_lock an additional time), and have replaced | |
30 | new_mountpoint with get_mountpoint a function that does the hash table | |
31 | lookup and addition under the mount_lock. The introduction of get_mounptoint | |
32 | ensures that only the mount_lock is needed to manipulate the mountpoint | |
33 | hashtable. | |
34 | ||
35 | d_set_mounted is modified to only set DCACHE_MOUNTED if it is not | |
36 | already set. This allows get_mountpoint to use the setting of | |
37 | DCACHE_MOUNTED to ensure adding a struct mountpoint for a dentry | |
38 | happens exactly once. | |
39 | ||
40 | Fixes: ce07d891a089 ("mnt: Honor MNT_LOCKED when detaching mounts") | |
41 | Reported-by: Krister Johansen <kjlx@templeofstupid.com> | |
42 | Suggested-by: Al Viro <viro@ZenIV.linux.org.uk> | |
43 | Acked-by: Al Viro <viro@ZenIV.linux.org.uk> | |
44 | Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com> | |
45 | Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> | |
46 | ||
47 | --- | |
48 | fs/dcache.c | 7 ++++-- | |
49 | fs/namespace.c | 64 ++++++++++++++++++++++++++++++++++++++++----------------- | |
50 | 2 files changed, 50 insertions(+), 21 deletions(-) | |
51 | ||
52 | --- a/fs/dcache.c | |
53 | +++ b/fs/dcache.c | |
54 | @@ -1322,8 +1322,11 @@ int d_set_mounted(struct dentry *dentry) | |
55 | } | |
56 | spin_lock(&dentry->d_lock); | |
57 | if (!d_unlinked(dentry)) { | |
58 | - dentry->d_flags |= DCACHE_MOUNTED; | |
59 | - ret = 0; | |
60 | + ret = -EBUSY; | |
61 | + if (!d_mountpoint(dentry)) { | |
62 | + dentry->d_flags |= DCACHE_MOUNTED; | |
63 | + ret = 0; | |
64 | + } | |
65 | } | |
66 | spin_unlock(&dentry->d_lock); | |
67 | out: | |
68 | --- a/fs/namespace.c | |
69 | +++ b/fs/namespace.c | |
70 | @@ -743,26 +743,50 @@ static struct mountpoint *lookup_mountpo | |
71 | return NULL; | |
72 | } | |
73 | ||
74 | -static struct mountpoint *new_mountpoint(struct dentry *dentry) | |
75 | +static struct mountpoint *get_mountpoint(struct dentry *dentry) | |
76 | { | |
77 | - struct hlist_head *chain = mp_hash(dentry); | |
78 | - struct mountpoint *mp; | |
79 | + struct mountpoint *mp, *new = NULL; | |
80 | int ret; | |
81 | ||
82 | - mp = kmalloc(sizeof(struct mountpoint), GFP_KERNEL); | |
83 | - if (!mp) | |
84 | + if (d_mountpoint(dentry)) { | |
85 | +mountpoint: | |
86 | + read_seqlock_excl(&mount_lock); | |
87 | + mp = lookup_mountpoint(dentry); | |
88 | + read_sequnlock_excl(&mount_lock); | |
89 | + if (mp) | |
90 | + goto done; | |
91 | + } | |
92 | + | |
93 | + if (!new) | |
94 | + new = kmalloc(sizeof(struct mountpoint), GFP_KERNEL); | |
95 | + if (!new) | |
96 | return ERR_PTR(-ENOMEM); | |
97 | ||
98 | + | |
99 | + /* Exactly one processes may set d_mounted */ | |
100 | ret = d_set_mounted(dentry); | |
101 | - if (ret) { | |
102 | - kfree(mp); | |
103 | - return ERR_PTR(ret); | |
104 | - } | |
105 | ||
106 | - mp->m_dentry = dentry; | |
107 | - mp->m_count = 1; | |
108 | - hlist_add_head(&mp->m_hash, chain); | |
109 | - INIT_HLIST_HEAD(&mp->m_list); | |
110 | + /* Someone else set d_mounted? */ | |
111 | + if (ret == -EBUSY) | |
112 | + goto mountpoint; | |
113 | + | |
114 | + /* The dentry is not available as a mountpoint? */ | |
115 | + mp = ERR_PTR(ret); | |
116 | + if (ret) | |
117 | + goto done; | |
118 | + | |
119 | + /* Add the new mountpoint to the hash table */ | |
120 | + read_seqlock_excl(&mount_lock); | |
121 | + new->m_dentry = dentry; | |
122 | + new->m_count = 1; | |
123 | + hlist_add_head(&new->m_hash, mp_hash(dentry)); | |
124 | + INIT_HLIST_HEAD(&new->m_list); | |
125 | + read_sequnlock_excl(&mount_lock); | |
126 | + | |
127 | + mp = new; | |
128 | + new = NULL; | |
129 | +done: | |
130 | + kfree(new); | |
131 | return mp; | |
132 | } | |
133 | ||
134 | @@ -1557,11 +1581,11 @@ void __detach_mounts(struct dentry *dent | |
135 | struct mount *mnt; | |
136 | ||
137 | namespace_lock(); | |
138 | + lock_mount_hash(); | |
139 | mp = lookup_mountpoint(dentry); | |
140 | if (IS_ERR_OR_NULL(mp)) | |
141 | goto out_unlock; | |
142 | ||
143 | - lock_mount_hash(); | |
144 | event++; | |
145 | while (!hlist_empty(&mp->m_list)) { | |
146 | mnt = hlist_entry(mp->m_list.first, struct mount, mnt_mp_list); | |
147 | @@ -1571,9 +1595,9 @@ void __detach_mounts(struct dentry *dent | |
148 | } | |
149 | else umount_tree(mnt, UMOUNT_CONNECTED); | |
150 | } | |
151 | - unlock_mount_hash(); | |
152 | put_mountpoint(mp); | |
153 | out_unlock: | |
154 | + unlock_mount_hash(); | |
155 | namespace_unlock(); | |
156 | } | |
157 | ||
158 | @@ -1962,9 +1986,7 @@ retry: | |
159 | namespace_lock(); | |
160 | mnt = lookup_mnt(path); | |
161 | if (likely(!mnt)) { | |
162 | - struct mountpoint *mp = lookup_mountpoint(dentry); | |
163 | - if (!mp) | |
164 | - mp = new_mountpoint(dentry); | |
165 | + struct mountpoint *mp = get_mountpoint(dentry); | |
166 | if (IS_ERR(mp)) { | |
167 | namespace_unlock(); | |
168 | mutex_unlock(&dentry->d_inode->i_mutex); | |
169 | @@ -1983,7 +2005,11 @@ retry: | |
170 | static void unlock_mount(struct mountpoint *where) | |
171 | { | |
172 | struct dentry *dentry = where->m_dentry; | |
173 | + | |
174 | + read_seqlock_excl(&mount_lock); | |
175 | put_mountpoint(where); | |
176 | + read_sequnlock_excl(&mount_lock); | |
177 | + | |
178 | namespace_unlock(); | |
179 | mutex_unlock(&dentry->d_inode->i_mutex); | |
180 | } | |
181 | @@ -3055,9 +3081,9 @@ SYSCALL_DEFINE2(pivot_root, const char _ | |
182 | touch_mnt_namespace(current->nsproxy->mnt_ns); | |
183 | /* A moved mount should not expire automatically */ | |
184 | list_del_init(&new_mnt->mnt_expire); | |
185 | + put_mountpoint(root_mp); | |
186 | unlock_mount_hash(); | |
187 | chroot_fs_refs(&root, &new); | |
188 | - put_mountpoint(root_mp); | |
189 | error = 0; | |
190 | out4: | |
191 | unlock_mount(old_mp); |