linux-kernel - Re: [RFC PATCH 07/11] ext4: have sync_fs op report writeback errors when passed a since pointer

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20180518152250.GC6361@bombadil.infradead.org>
Date:   Fri, 18 May 2018 08:22:50 -0700
From:   Matthew Wilcox <willy@...radead.org>
To:     Jeff Layton <jlayton@...nel.org>
Cc:     linux-fsdevel@...r.kernel.org, linux-kernel@...r.kernel.org,
        viro@...IV.linux.org.uk, andres@...razel.de
Subject: Re: [RFC PATCH 07/11] ext4: have sync_fs op report writeback errors
 when passed a since pointer

On Fri, May 18, 2018 at 08:34:11AM -0400, Jeff Layton wrote:
> From: Jeff Layton <jlayton@...hat.com>
> 
> When ext4_sync_fs gets a non-NULL since pointer, use it to report
> errors vs. the errseq_t in the super_block. This allows us to
> properly report an error to sync_fs when any inode has failed writeback
> since we last checked for it.
> 
> Signed-off-by: Jeff Layton <jlayton@...hat.com>
> ---
>  fs/ext4/super.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/fs/ext4/super.c b/fs/ext4/super.c
> index 896ddf8c3421..a5f41d31611f 100644
> --- a/fs/ext4/super.c
> +++ b/fs/ext4/super.c
> @@ -4898,6 +4898,8 @@ static int ext4_sync_fs(struct super_block *sb, int wait, errseq_t *since)
>  	}
>  out:
>  	ret2 = __sync_blockdev(sb->s_bdev, wait);
> +	if (since)
> +		ret2 = errseq_check_and_advance(&sb->s_wb_err, since);
>  	return ret ? ret : ret2;
>  }

This is inconsistent with the ext2 and xfs implementations ...

I'm worried this is too much complexity to push down to the filesystems.
When should errors get reported through the return value; when should they
be reported through the errseq_t?

Can we hide all of this?  Maybe ext4 could do:

	errseq_set(&sb->s_wb_err, __sync_blockdev(sb->s_bdev, wait));
	return ret;

(we'd need to make calling errseq_set(x, 0) be a no-op instead of an error)

and then the caller is the one who takes care of calling
errseq_check_and_advance() so we don't have to pass 'since' into each
filesystem.