• Suzuki K P's avatar
    [PATCH] fix reiserfs bad path release panic · 87b4126f
    Suzuki K P authored
    One of our test team hit a reiserfs_panic while running fsstress tests on
    2.6.19-rc1.  The message looks like :
    
      REISERFS: panic(device Null superblock):
      reiserfs[5676]: assertion !(p->path_length != 1 ) failed at
      fs/reiserfs/stree.c:397:reiserfs_check_path: path not properly relsed.
    
    The backtrace looked :
    
      kernel BUG in reiserfs_panic at fs/reiserfs/prints.c:361!
    	.reiserfs_check_path+0x58/0x74
    	.reiserfs_get_block+0x1444/0x1508
    	.__block_prepare_write+0x1c8/0x558
    	.block_prepare_write+0x34/0x64
    	.reiserfs_prepare_write+0x118/0x1d0
    	.generic_file_buffered_write+0x314/0x82c
    	.__generic_file_aio_write_nolock+0x350/0x3e0
    	.__generic_file_write_nolock+0x78/0xb0
    	.generic_file_write+0x60/0xf0
    	.reiserfs_file_write+0x198/0x2038
    	.vfs_write+0xd0/0x1b4
    	.sys_write+0x4c/0x8c
    	syscall_exit+0x0/0x4
    
    Upon debugging I found that the restart_transaction was not releasing
    the path if the th->refcount was > 1.
    
    /*static*/
    int restart_transaction(struct reiserfs_transaction_handle *th,
                               			struct inode *inode, struct path *path)
    {
    	[...]
    
             /* we cannot restart while nested */
             if (th->t_refcount > 1) { <<- Path is not released in this case!
                     return 0;
             }
    
             pathrelse(path); <<- Path released here.
    	[...]
    
    This could happen in such a situation :
    
    In reiserfs/inode.c: reiserfs_get_block() ::
    
          if (repeat == NO_DISK_SPACE || repeat == QUOTA_EXCEEDED) {
              /* restart the transaction to give the journal a chance to free
               ** some blocks.  releases the path, so we have to go back to
               ** research if we succeed on the second try
               */
              SB_JOURNAL(inode->i_sb)->j_next_async_flush = 1;
    
            -->>  retval = restart_transaction(th, inode, &path); <<--
    
      We are supposed to release the path, no matter we succeed or fail. But
    if the th->refcount is > 1, the path is still valid. And,
    
              if (retval)
                       goto failure;
              repeat =
                  _allocate_block(th, block, inode,
                                 &allocated_block_nr, NULL, create);
    
    If the above allocate_block fails with NO_DISK_SPACE or QUOTA_EXCEEDED,
    we would have path which is not released.
    
             if (repeat != NO_DISK_SPACE && repeat != QUOTA_EXCEEDED) {
                       goto research;
             }
             if (repeat == QUOTA_EXCEEDED)
                       retval = -EDQUOT;
             else
                       retval = -ENOSPC;
             goto failure;
    	[...]
    
           failure:
    	[...]
             reiserfs_check_path(&path); << Panics here !
    
    Attached here is a patch which could fix the issue.
    
    fix reiserfs/inode.c : restart_transaction() to release the path in all
    cases.
    
    The restart_transaction() doesn't release the path when the the journal
    handle has a refcount > 1.  This would trigger a reiserfs_panic() if we
    encounter an -ENOSPC / -EDQUOT in reiserfs_get_block().
    Signed-off-by: default avatarSuzuki K P <suzuki@in.ibm.com>
    Cc: "Vladimir V. Saveliev" <vs@namesys.com>
    Cc: <reiserfs-dev@namesys.com>
    Cc: Jeff Mahoney <jeffm@suse.com>
    Acked-by: default avatarJan Kara <jack@suse.cz>
    Signed-off-by: default avatarAndrew Morton <akpm@osdl.org>
    Signed-off-by: default avatarLinus Torvalds <torvalds@osdl.org>
    87b4126f
inode.c 87 KB