[GFS2] Fix lock ordering bug in page fault path
Mmapped files were able to trigger a lock ordering bug. Private
maps do not need to take the glock so early on. Shared maps do
unfortunately, however we can get around that by adding a flag
into the flags for the struct gfs2_file. This only works because
we are taking an exclusive lock at this point, so we know that
nobody else can be racing with us.
Fixes Red Hat bugzilla: #201196
Signed-off-by: Steven Whitehouse <swhiteho@redhat.com>
diff --git a/fs/gfs2/incore.h b/fs/gfs2/incore.h
index 90e0624..e98c14f 100644
--- a/fs/gfs2/incore.h
+++ b/fs/gfs2/incore.h
@@ -279,6 +279,7 @@
enum {
GFF_DID_DIRECT_ALLOC = 0,
+ GFF_EXLOCK = 1,
};
struct gfs2_file {
diff --git a/fs/gfs2/log.c b/fs/gfs2/log.c
index 60fdc94..a591fb8 100644
--- a/fs/gfs2/log.c
+++ b/fs/gfs2/log.c
@@ -256,8 +256,8 @@
if (list_empty(&sdp->sd_ail1_list))
tail = sdp->sd_log_head;
else {
- ai = list_entry(sdp->sd_ail1_list.prev,
- struct gfs2_ail, ai_list);
+ ai = list_entry(sdp->sd_ail1_list.prev, struct gfs2_ail,
+ ai_list);
tail = ai->ai_first;
}
diff --git a/fs/gfs2/ops_address.c b/fs/gfs2/ops_address.c
index fca69f1..bdd4d6b4 100644
--- a/fs/gfs2/ops_address.c
+++ b/fs/gfs2/ops_address.c
@@ -237,14 +237,22 @@
struct gfs2_sbd *sdp = GFS2_SB(page->mapping->host);
struct gfs2_holder gh;
int error;
+ int do_unlock = 0;
if (likely(file != &gfs2_internal_file_sentinal)) {
+ if (file) {
+ struct gfs2_file *gf = file->private_data;
+ if (test_bit(GFF_EXLOCK, &gf->f_flags))
+ goto skip_lock;
+ }
gfs2_holder_init(ip->i_gl, LM_ST_SHARED, GL_ATIME|GL_AOP, &gh);
+ do_unlock = 1;
error = gfs2_glock_nq_m_atime(1, &gh);
if (unlikely(error))
goto out_unlock;
}
+skip_lock:
if (gfs2_is_stuffed(ip)) {
error = stuffed_readpage(ip, page);
unlock_page(page);
@@ -262,7 +270,7 @@
return error;
out_unlock:
unlock_page(page);
- if (file != &gfs2_internal_file_sentinal)
+ if (do_unlock)
gfs2_holder_uninit(&gh);
goto out;
}
@@ -291,17 +299,24 @@
struct gfs2_holder gh;
unsigned page_idx;
int ret;
+ int do_unlock = 0;
if (likely(file != &gfs2_internal_file_sentinal)) {
+ if (file) {
+ struct gfs2_file *gf = file->private_data;
+ if (test_bit(GFF_EXLOCK, &gf->f_flags))
+ goto skip_lock;
+ }
gfs2_holder_init(ip->i_gl, LM_ST_SHARED,
LM_FLAG_TRY_1CB|GL_ATIME|GL_AOP, &gh);
+ do_unlock = 1;
ret = gfs2_glock_nq_m_atime(1, &gh);
if (ret == GLR_TRYFAILED)
goto out_noerror;
if (unlikely(ret))
goto out_unlock;
}
-
+skip_lock:
if (gfs2_is_stuffed(ip)) {
struct pagevec lru_pvec;
pagevec_init(&lru_pvec, 0);
@@ -326,7 +341,7 @@
ret = mpage_readpages(mapping, pages, nr_pages, gfs2_get_block);
}
- if (likely(file != &gfs2_internal_file_sentinal)) {
+ if (do_unlock) {
gfs2_glock_dq_m(1, &gh);
gfs2_holder_uninit(&gh);
}
@@ -344,7 +359,7 @@
unlock_page(page);
page_cache_release(page);
}
- if (likely(file != &gfs2_internal_file_sentinal))
+ if (do_unlock)
gfs2_holder_uninit(&gh);
goto out;
}
diff --git a/fs/gfs2/ops_vm.c b/fs/gfs2/ops_vm.c
index aff6637..875a769 100644
--- a/fs/gfs2/ops_vm.c
+++ b/fs/gfs2/ops_vm.c
@@ -46,13 +46,7 @@
unsigned long address, int *type)
{
struct gfs2_inode *ip = GFS2_I(area->vm_file->f_mapping->host);
- struct gfs2_holder i_gh;
struct page *result;
- int error;
-
- error = gfs2_glock_nq_init(ip->i_gl, LM_ST_SHARED, 0, &i_gh);
- if (error)
- return NULL;
set_bit(GIF_PAGED, &ip->i_flags);
@@ -61,8 +55,6 @@
if (result && result != NOPAGE_OOM)
pfault_be_greedy(ip);
- gfs2_glock_dq_uninit(&i_gh);
-
return result;
}
@@ -141,7 +133,9 @@
static struct page *gfs2_sharewrite_nopage(struct vm_area_struct *area,
unsigned long address, int *type)
{
- struct gfs2_inode *ip = GFS2_I(area->vm_file->f_mapping->host);
+ struct file *file = area->vm_file;
+ struct gfs2_file *gf = file->private_data;
+ struct gfs2_inode *ip = GFS2_I(file->f_mapping->host);
struct gfs2_holder i_gh;
struct page *result = NULL;
unsigned long index = ((address - area->vm_start) >> PAGE_CACHE_SHIFT) +
@@ -156,13 +150,14 @@
set_bit(GIF_PAGED, &ip->i_flags);
set_bit(GIF_SW_PAGED, &ip->i_flags);
- error = gfs2_write_alloc_required(ip,
- (uint64_t)index << PAGE_CACHE_SHIFT,
+ error = gfs2_write_alloc_required(ip, (u64)index << PAGE_CACHE_SHIFT,
PAGE_CACHE_SIZE, &alloc_required);
if (error)
goto out;
+ set_bit(GFF_EXLOCK, &gf->f_flags);
result = filemap_nopage(area, address, type);
+ clear_bit(GFF_EXLOCK, &gf->f_flags);
if (!result || result == NOPAGE_OOM)
goto out;
@@ -177,8 +172,7 @@
}
pfault_be_greedy(ip);
-
- out:
+out:
gfs2_glock_dq_uninit(&i_gh);
return result;
diff --git a/fs/gfs2/recovery.c b/fs/gfs2/recovery.c
index 7aabc03..bbd44a4 100644
--- a/fs/gfs2/recovery.c
+++ b/fs/gfs2/recovery.c
@@ -153,8 +153,7 @@
if (lh.lh_header.mh_magic != GFS2_MAGIC ||
lh.lh_header.mh_type != GFS2_METATYPE_LH ||
- lh.lh_blkno != blk ||
- lh.lh_hash != hash)
+ lh.lh_blkno != blk || lh.lh_hash != hash)
return 1;
*head = lh;
@@ -482,11 +481,9 @@
/* Acquire a shared hold on the transaction lock */
- error = gfs2_glock_nq_init(sdp->sd_trans_gl,
- LM_ST_SHARED,
+ error = gfs2_glock_nq_init(sdp->sd_trans_gl, LM_ST_SHARED,
LM_FLAG_NOEXP | LM_FLAG_PRIORITY |
- GL_NOCANCEL | GL_NOCACHE,
- &t_gh);
+ GL_NOCANCEL | GL_NOCACHE, &t_gh);
if (error)
goto fail_gunlock_ji;
