Commit e3bb00e3 authored by Thomas Betker's avatar Thomas Betker Committed by Ben Hutchings

jffs2: Fix lock acquisition order bug in jffs2_write_begin

commit 5ffd3412 upstream.

jffs2_write_begin() first acquires the page lock, then f->sem. This
causes an AB-BA deadlock with jffs2_garbage_collect_live(), which first
acquires f->sem, then the page lock:

jffs2_garbage_collect_live
    mutex_lock(&f->sem)                         (A)
    jffs2_garbage_collect_dnode
        jffs2_gc_fetch_page
            read_cache_page_async
                do_read_cache_page
                    lock_page(page)             (B)

jffs2_write_begin
    grab_cache_page_write_begin
        find_lock_page
            lock_page(page)                     (B)
    mutex_lock(&f->sem)                         (A)

We fix this by restructuring jffs2_write_begin() to take f->sem before
the page lock. However, we make sure that f->sem is not held when
calling jffs2_reserve_space(), as this is not permitted by the locking
rules.

The deadlock above was observed multiple times on an SoC with a dual
ARMv7 (Cortex-A9), running the long-term 3.4.11 kernel; it occurred
when using scp to copy files from a host system to the ARM target
system. The fix was heavily tested on the same target system.
Signed-off-by: default avatarThomas Betker <thomas.betker@rohde-schwarz.com>
Acked-by: default avatarJoakim Tjernlund <Joakim.Tjernlund@transmode.se>
Signed-off-by: default avatarArtem Bityutskiy <artem.bityutskiy@linux.intel.com>
[bwh: Backported to 3.2:
 - Adjust context
 - Use D1(printk(KERN_DEBUG ...)) instead of jffs2_dbg(1, ...)]
Signed-off-by: default avatarBen Hutchings <ben@decadent.org.uk>
parent c714d5eb
...@@ -135,33 +135,39 @@ static int jffs2_write_begin(struct file *filp, struct address_space *mapping, ...@@ -135,33 +135,39 @@ static int jffs2_write_begin(struct file *filp, struct address_space *mapping,
struct page *pg; struct page *pg;
struct inode *inode = mapping->host; struct inode *inode = mapping->host;
struct jffs2_inode_info *f = JFFS2_INODE_INFO(inode); struct jffs2_inode_info *f = JFFS2_INODE_INFO(inode);
struct jffs2_sb_info *c = JFFS2_SB_INFO(inode->i_sb);
struct jffs2_raw_inode ri;
uint32_t alloc_len = 0;
pgoff_t index = pos >> PAGE_CACHE_SHIFT; pgoff_t index = pos >> PAGE_CACHE_SHIFT;
uint32_t pageofs = index << PAGE_CACHE_SHIFT; uint32_t pageofs = index << PAGE_CACHE_SHIFT;
int ret = 0; int ret = 0;
D1(printk(KERN_DEBUG "%s()\n", __func__));
if (pageofs > inode->i_size) {
ret = jffs2_reserve_space(c, sizeof(ri), &alloc_len,
ALLOC_NORMAL, JFFS2_SUMMARY_INODE_SIZE);
if (ret)
return ret;
}
mutex_lock(&f->sem);
pg = grab_cache_page_write_begin(mapping, index, flags); pg = grab_cache_page_write_begin(mapping, index, flags);
if (!pg) if (!pg) {
if (alloc_len)
jffs2_complete_reservation(c);
mutex_unlock(&f->sem);
return -ENOMEM; return -ENOMEM;
}
*pagep = pg; *pagep = pg;
D1(printk(KERN_DEBUG "jffs2_write_begin()\n")); if (alloc_len) {
if (pageofs > inode->i_size) {
/* Make new hole frag from old EOF to new page */ /* Make new hole frag from old EOF to new page */
struct jffs2_sb_info *c = JFFS2_SB_INFO(inode->i_sb);
struct jffs2_raw_inode ri;
struct jffs2_full_dnode *fn; struct jffs2_full_dnode *fn;
uint32_t alloc_len;
D1(printk(KERN_DEBUG "Writing new hole frag 0x%x-0x%x between current EOF and new page\n", D1(printk(KERN_DEBUG "Writing new hole frag 0x%x-0x%x between current EOF and new page\n",
(unsigned int)inode->i_size, pageofs)); (unsigned int)inode->i_size, pageofs));
ret = jffs2_reserve_space(c, sizeof(ri), &alloc_len,
ALLOC_NORMAL, JFFS2_SUMMARY_INODE_SIZE);
if (ret)
goto out_page;
mutex_lock(&f->sem);
memset(&ri, 0, sizeof(ri)); memset(&ri, 0, sizeof(ri));
ri.magic = cpu_to_je16(JFFS2_MAGIC_BITMASK); ri.magic = cpu_to_je16(JFFS2_MAGIC_BITMASK);
...@@ -188,7 +194,6 @@ static int jffs2_write_begin(struct file *filp, struct address_space *mapping, ...@@ -188,7 +194,6 @@ static int jffs2_write_begin(struct file *filp, struct address_space *mapping,
if (IS_ERR(fn)) { if (IS_ERR(fn)) {
ret = PTR_ERR(fn); ret = PTR_ERR(fn);
jffs2_complete_reservation(c); jffs2_complete_reservation(c);
mutex_unlock(&f->sem);
goto out_page; goto out_page;
} }
ret = jffs2_add_full_dnode_to_inode(c, f, fn); ret = jffs2_add_full_dnode_to_inode(c, f, fn);
...@@ -202,12 +207,10 @@ static int jffs2_write_begin(struct file *filp, struct address_space *mapping, ...@@ -202,12 +207,10 @@ static int jffs2_write_begin(struct file *filp, struct address_space *mapping,
jffs2_mark_node_obsolete(c, fn->raw); jffs2_mark_node_obsolete(c, fn->raw);
jffs2_free_full_dnode(fn); jffs2_free_full_dnode(fn);
jffs2_complete_reservation(c); jffs2_complete_reservation(c);
mutex_unlock(&f->sem);
goto out_page; goto out_page;
} }
jffs2_complete_reservation(c); jffs2_complete_reservation(c);
inode->i_size = pageofs; inode->i_size = pageofs;
mutex_unlock(&f->sem);
} }
/* /*
...@@ -216,18 +219,18 @@ static int jffs2_write_begin(struct file *filp, struct address_space *mapping, ...@@ -216,18 +219,18 @@ static int jffs2_write_begin(struct file *filp, struct address_space *mapping,
* case of a short-copy. * case of a short-copy.
*/ */
if (!PageUptodate(pg)) { if (!PageUptodate(pg)) {
mutex_lock(&f->sem);
ret = jffs2_do_readpage_nolock(inode, pg); ret = jffs2_do_readpage_nolock(inode, pg);
mutex_unlock(&f->sem);
if (ret) if (ret)
goto out_page; goto out_page;
} }
mutex_unlock(&f->sem);
D1(printk(KERN_DEBUG "end write_begin(). pg->flags %lx\n", pg->flags)); D1(printk(KERN_DEBUG "end write_begin(). pg->flags %lx\n", pg->flags));
return ret; return ret;
out_page: out_page:
unlock_page(pg); unlock_page(pg);
page_cache_release(pg); page_cache_release(pg);
mutex_unlock(&f->sem);
return ret; return ret;
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment