[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 90e0624d806503b823f86b25bdb701379364260b..e98c14f30daa5ef8c4d8a9880b2c9cc577ad05db 100644 (file)
--- a/fs/gfs2/incore.h
+++ b/fs/gfs2/incore.h
@@ -279,6 +279,7 @@ static inline struct gfs2_sbd *GFS2_SB(struct inode *inode)
 
 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 60fdc94ccc8aabbe053d7cfd743060b0705a9a44..a591fb8fae20ab7bf22cd7e1f76bf60f4e61c99c 100644 (file)
--- a/fs/gfs2/log.c
+++ b/fs/gfs2/log.c
@@ -256,8 +256,8 @@ static unsigned int current_tail(struct gfs2_sbd *sdp)
        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 fca69f12e4de8b8c7ea601eec4dd6c8c861cd23b..bdd4d6b487219e3d6d3855599d9abc879d24a800 100644 (file)
--- a/fs/gfs2/ops_address.c
+++ b/fs/gfs2/ops_address.c
@@ -237,14 +237,22 @@ static int gfs2_readpage(struct file *file, struct page *page)
        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 @@ out:
        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 @@ static int gfs2_readpages(struct file *file, struct address_space *mapping,
        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 @@ static int gfs2_readpages(struct file *file, struct address_space *mapping,
                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 @@ out_unlock:
                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 aff66373b7e1a9eacdf8499fc26752249cc7681d..875a769444a1b613d6fb5f75002a571bab0db1ec 100644 (file)
--- a/fs/gfs2/ops_vm.c
+++ b/fs/gfs2/ops_vm.c
@@ -46,13 +46,7 @@ static struct page *gfs2_private_nopage(struct vm_area_struct *area,
                                        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 @@ static struct page *gfs2_private_nopage(struct vm_area_struct *area,
        if (result && result != NOPAGE_OOM)
                pfault_be_greedy(ip);
 
-       gfs2_glock_dq_uninit(&i_gh);
-
        return result;
 }
 
@@ -141,7 +133,9 @@ static int alloc_page_backing(struct gfs2_inode *ip, struct page *page)
 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 @@ static struct page *gfs2_sharewrite_nopage(struct vm_area_struct *area,
        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 @@ static struct page *gfs2_sharewrite_nopage(struct vm_area_struct *area,
        }
 
        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 7aabc03e4abdfc3da6c37ffe6ad42f6348cc353c..bbd44a4b1a1f15748c857f0f506efcf1f2899663 100644 (file)
--- a/fs/gfs2/recovery.c
+++ b/fs/gfs2/recovery.c
@@ -153,8 +153,7 @@ static int get_log_header(struct gfs2_jdesc *jd, unsigned int blk,
 
        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 @@ int gfs2_recover_journal(struct gfs2_jdesc *jd)
 
                /* 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;