mirror of
				https://github.com/torvalds/linux.git
				synced 2025-11-04 10:40:15 +02:00 
			
		
		
		
	jffs2: Fix lock acquisition order bug in jffs2_write_begin
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.
Cc: stable@vger.kernel.org
Signed-off-by: Thomas Betker <thomas.betker@rohde-schwarz.com>
Acked-by: Joakim Tjernlund <Joakim.Tjernlund@transmode.se>
Signed-off-by: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>
			
			
This commit is contained in:
		
							parent
							
								
									0131950ebd
								
							
						
					
					
						commit
						5ffd3412ae
					
				
					 1 changed files with 22 additions and 19 deletions
				
			
		| 
						 | 
					@ -138,33 +138,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;
 | 
				
			||||||
 | 
					
 | 
				
			||||||
	pg = grab_cache_page_write_begin(mapping, index, flags);
 | 
					 | 
				
			||||||
	if (!pg)
 | 
					 | 
				
			||||||
		return -ENOMEM;
 | 
					 | 
				
			||||||
	*pagep = pg;
 | 
					 | 
				
			||||||
 | 
					 | 
				
			||||||
	jffs2_dbg(1, "%s()\n", __func__);
 | 
						jffs2_dbg(1, "%s()\n", __func__);
 | 
				
			||||||
 | 
					
 | 
				
			||||||
	if (pageofs > inode->i_size) {
 | 
						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);
 | 
				
			||||||
 | 
						if (!pg) {
 | 
				
			||||||
 | 
							if (alloc_len)
 | 
				
			||||||
 | 
								jffs2_complete_reservation(c);
 | 
				
			||||||
 | 
							mutex_unlock(&f->sem);
 | 
				
			||||||
 | 
							return -ENOMEM;
 | 
				
			||||||
 | 
						}
 | 
				
			||||||
 | 
						*pagep = pg;
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
						if (alloc_len) {
 | 
				
			||||||
		/* 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;
 | 
					 | 
				
			||||||
 | 
					
 | 
				
			||||||
		jffs2_dbg(1, "Writing new hole frag 0x%x-0x%x between current EOF and new page\n",
 | 
							jffs2_dbg(1, "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);
 | 
				
			||||||
| 
						 | 
					@ -191,7 +197,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);
 | 
				
			||||||
| 
						 | 
					@ -206,12 +211,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);
 | 
					 | 
				
			||||||
	}
 | 
						}
 | 
				
			||||||
 | 
					
 | 
				
			||||||
	/*
 | 
						/*
 | 
				
			||||||
| 
						 | 
					@ -220,18 +223,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);
 | 
				
			||||||
	jffs2_dbg(1, "end write_begin(). pg->flags %lx\n", pg->flags);
 | 
						jffs2_dbg(1, "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;
 | 
				
			||||||
}
 | 
					}
 | 
				
			||||||
 | 
					
 | 
				
			||||||
| 
						 | 
					
 | 
				
			||||||
		Loading…
	
		Reference in a new issue