diff options
author | Bill O'Donnell <billodo@redhat.com> | 2017-02-07 15:59:33 -0500 |
---|---|---|
committer | Darrick J. Wong <darrick.wong@oracle.com> | 2017-02-09 13:50:25 -0500 |
commit | b20fe4730ea5c037c16631fb0df659c7b6d4b3b1 (patch) | |
tree | e397e683169623414400ea7ea900109710105d12 | |
parent | c5ecb42342852892f978572ddc6dca703460f25a (diff) |
xfs: correct null checks and error processing in xfs_initialize_perag
If pag cannot be allocated, the current error exit path will trip
a null pointer deference error when calling xfs_buf_hash_destroy
with a null pag. Fix this by adding a new error exit labels and
jumping to those accordingly, avoiding the hash destroy and
unnecessary kmem_free on pag.
Up to three things need to be properly unwound:
1) pag memory allocation
2) xfs_buf_hash_init
3) radix_tree_insert
For any given iteration through the loop, any of the above which
succeed must be unwound for /this/ pag, and then all prior
initialized pags must be unwound.
Addresses-Coverity-Id: 1397628 ("Dereference after null check")
Reported-by: Colin Ian King <colin.king@canonical.com>
Signed-off-by: Bill O'Donnell <billodo@redhat.com>
Reviewed-by: Eric Sandeen <sandeen@redhat.com>
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
-rw-r--r-- | fs/xfs/xfs_mount.c | 24 |
1 files changed, 15 insertions, 9 deletions
diff --git a/fs/xfs/xfs_mount.c b/fs/xfs/xfs_mount.c index 9b9540db17a6..1f1e4ae44150 100644 --- a/fs/xfs/xfs_mount.c +++ b/fs/xfs/xfs_mount.c | |||
@@ -187,7 +187,7 @@ xfs_initialize_perag( | |||
187 | xfs_agnumber_t *maxagi) | 187 | xfs_agnumber_t *maxagi) |
188 | { | 188 | { |
189 | xfs_agnumber_t index; | 189 | xfs_agnumber_t index; |
190 | xfs_agnumber_t first_initialised = 0; | 190 | xfs_agnumber_t first_initialised = NULLAGNUMBER; |
191 | xfs_perag_t *pag; | 191 | xfs_perag_t *pag; |
192 | int error = -ENOMEM; | 192 | int error = -ENOMEM; |
193 | 193 | ||
@@ -202,22 +202,20 @@ xfs_initialize_perag( | |||
202 | xfs_perag_put(pag); | 202 | xfs_perag_put(pag); |
203 | continue; | 203 | continue; |
204 | } | 204 | } |
205 | if (!first_initialised) | ||
206 | first_initialised = index; | ||
207 | 205 | ||
208 | pag = kmem_zalloc(sizeof(*pag), KM_MAYFAIL); | 206 | pag = kmem_zalloc(sizeof(*pag), KM_MAYFAIL); |
209 | if (!pag) | 207 | if (!pag) |
210 | goto out_unwind; | 208 | goto out_unwind_new_pags; |
211 | pag->pag_agno = index; | 209 | pag->pag_agno = index; |
212 | pag->pag_mount = mp; | 210 | pag->pag_mount = mp; |
213 | spin_lock_init(&pag->pag_ici_lock); | 211 | spin_lock_init(&pag->pag_ici_lock); |
214 | mutex_init(&pag->pag_ici_reclaim_lock); | 212 | mutex_init(&pag->pag_ici_reclaim_lock); |
215 | INIT_RADIX_TREE(&pag->pag_ici_root, GFP_ATOMIC); | 213 | INIT_RADIX_TREE(&pag->pag_ici_root, GFP_ATOMIC); |
216 | if (xfs_buf_hash_init(pag)) | 214 | if (xfs_buf_hash_init(pag)) |
217 | goto out_unwind; | 215 | goto out_free_pag; |
218 | 216 | ||
219 | if (radix_tree_preload(GFP_NOFS)) | 217 | if (radix_tree_preload(GFP_NOFS)) |
220 | goto out_unwind; | 218 | goto out_hash_destroy; |
221 | 219 | ||
222 | spin_lock(&mp->m_perag_lock); | 220 | spin_lock(&mp->m_perag_lock); |
223 | if (radix_tree_insert(&mp->m_perag_tree, index, pag)) { | 221 | if (radix_tree_insert(&mp->m_perag_tree, index, pag)) { |
@@ -225,10 +223,13 @@ xfs_initialize_perag( | |||
225 | spin_unlock(&mp->m_perag_lock); | 223 | spin_unlock(&mp->m_perag_lock); |
226 | radix_tree_preload_end(); | 224 | radix_tree_preload_end(); |
227 | error = -EEXIST; | 225 | error = -EEXIST; |
228 | goto out_unwind; | 226 | goto out_hash_destroy; |
229 | } | 227 | } |
230 | spin_unlock(&mp->m_perag_lock); | 228 | spin_unlock(&mp->m_perag_lock); |
231 | radix_tree_preload_end(); | 229 | radix_tree_preload_end(); |
230 | /* first new pag is fully initialized */ | ||
231 | if (first_initialised == NULLAGNUMBER) | ||
232 | first_initialised = index; | ||
232 | } | 233 | } |
233 | 234 | ||
234 | index = xfs_set_inode_alloc(mp, agcount); | 235 | index = xfs_set_inode_alloc(mp, agcount); |
@@ -239,11 +240,16 @@ xfs_initialize_perag( | |||
239 | mp->m_ag_prealloc_blocks = xfs_prealloc_blocks(mp); | 240 | mp->m_ag_prealloc_blocks = xfs_prealloc_blocks(mp); |
240 | return 0; | 241 | return 0; |
241 | 242 | ||
242 | out_unwind: | 243 | out_hash_destroy: |
243 | xfs_buf_hash_destroy(pag); | 244 | xfs_buf_hash_destroy(pag); |
245 | out_free_pag: | ||
244 | kmem_free(pag); | 246 | kmem_free(pag); |
245 | for (; index > first_initialised; index--) { | 247 | out_unwind_new_pags: |
248 | /* unwind any prior newly initialized pags */ | ||
249 | for (index = first_initialised; index < agcount; index++) { | ||
246 | pag = radix_tree_delete(&mp->m_perag_tree, index); | 250 | pag = radix_tree_delete(&mp->m_perag_tree, index); |
251 | if (!pag) | ||
252 | break; | ||
247 | xfs_buf_hash_destroy(pag); | 253 | xfs_buf_hash_destroy(pag); |
248 | kmem_free(pag); | 254 | kmem_free(pag); |
249 | } | 255 | } |