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 <vmartirosyan@xxxxxxxxx>"
>
> 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 <vahram.martirosyan@xxxxxxxxxxxxxxxx>
> Reviewed-by: Gu Zheng <guz.fnst@xxxxxxxxxxxxxx>
> Signed-off-by: Dave Kleikamp <dave.kleikamp@xxxxxxxxxx>
> Signed-off-by: Jonghwan Choi <jhbird.choi@xxxxxxxxxxx>
> ---
> 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 stable" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html