On 06/03/2013 09:15 PM, Jonghwan Choi wrote:
> This patch looks like it should be in the 3.9-stable tree, should we apply
> it?

I'm kind of on the fence on this one. I believe the failure here was
triggered by induced errors to check the error paths. I'm not aware of a
real-world crash due to the problem being fixed here. Of course, it's
possible, but I don't think it's very likely.

Thanks,
Dave

> 
> ------------------
> 
> From: "Vahram Martirosyan <[email protected]>"
> 
> commit e9b376671910d105c5e61103111b96209c729529 upstream
> 
> The mentioned functions do not pay attention to the error codes returned
> by the functions updateSuper(), lmLogInit() and lmLogShutdown(). It brings
> to system crash later when writing to log.
> 
> The patch adds corresponding code to check and return the error codes
> and to print correct error messages in case of errors.
> 
> Found by Linux File System Verification project (linuxtesting.org).
> 
> Signed-off-by: Vahram Martirosyan <[email protected]>
> Reviewed-by: Gu Zheng <[email protected]>
> Signed-off-by: Dave Kleikamp <[email protected]>
> Signed-off-by: Jonghwan Choi <[email protected]>
> ---
>  fs/jfs/super.c |   38 ++++++++++++++++++++++++++++++--------
>  1 file changed, 30 insertions(+), 8 deletions(-)
> 
> diff --git a/fs/jfs/super.c b/fs/jfs/super.c
> index 1a543be..2502d39 100644
> --- a/fs/jfs/super.c
> +++ b/fs/jfs/super.c
> @@ -611,11 +611,28 @@ static int jfs_freeze(struct super_block *sb)
>  {
>       struct jfs_sb_info *sbi = JFS_SBI(sb);
>       struct jfs_log *log = sbi->log;
> +     int rc = 0;
>  
>       if (!(sb->s_flags & MS_RDONLY)) {
>               txQuiesce(sb);
> -             lmLogShutdown(log);
> -             updateSuper(sb, FM_CLEAN);
> +             rc = lmLogShutdown(log);
> +             if (rc) {
> +                     jfs_error(sb, "jfs_freeze: lmLogShutdown failed");
> +
> +                     /* let operations fail rather than hang */
> +                     txResume(sb);
> +
> +                     return rc;
> +             }
> +             rc = updateSuper(sb, FM_CLEAN);
> +             if (rc) {
> +                     jfs_err("jfs_freeze: updateSuper failed\n");
> +                     /*
> +                      * Don't fail here. Everything succeeded except
> +                      * marking the superblock clean, so there's really
> +                      * no harm in leaving it frozen for now.
> +                      */
> +             }
>       }
>       return 0;
>  }
> @@ -627,13 +644,18 @@ static int jfs_unfreeze(struct super_block *sb)
>       int rc = 0;
>  
>       if (!(sb->s_flags & MS_RDONLY)) {
> -             updateSuper(sb, FM_MOUNT);
> -             if ((rc = lmLogInit(log)))
> -                     jfs_err("jfs_unlock failed with return code %d",
> rc);
> -             else
> -                     txResume(sb);
> +             rc = updateSuper(sb, FM_MOUNT);
> +             if (rc) {
> +                     jfs_error(sb, "jfs_unfreeze: updateSuper failed");
> +                     goto out;
> +             }
> +             rc = lmLogInit(log);
> +             if (rc)
> +                     jfs_error(sb, "jfs_unfreeze: lmLogInit failed");
> +out:
> +             txResume(sb);
>       }
> -     return 0;
> +     return rc;
>  }
>  
>  static struct dentry *jfs_do_mount(struct file_system_type *fs_type,
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Reply via email to