linux-ext4 - Re: [PATCH v2 5/5] ext4: add suppport to make bitmaps corruptions nonfatal

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: <20180730025422.GN21725@thunk.org>
Date:   Sun, 29 Jul 2018 22:54:22 -0400
From:   "Theodore Y. Ts'o" <tytso@....edu>
To:     Wang Shilong <wangshilong1991@...il.com>
Cc:     linux-ext4@...r.kernel.org, andreas.dilger@...el.com,
        sihara@....com, wshilong@....com
Subject: Re: [PATCH v2 5/5] ext4: add suppport to make bitmaps corruptions
 nonfatal

On Tue, May 29, 2018 at 08:45:17PM +0900, Wang Shilong wrote:
> +#define EXT4_MOUNT_BITMAPS_WARN		0x100000000 /* warn on bitmaps corruptions */
>  
>  /*
>   * Mount flags set either automatically (could not be set by mount option)
> @@ -1338,7 +1339,7 @@ struct ext4_sb_info {
>  	struct buffer_head * s_sbh;	/* Buffer containing the super block */
>  	struct ext4_super_block *s_es;	/* Pointer to the super block in the buffer */
>  	struct buffer_head **s_group_desc;
> -	unsigned int s_mount_opt;
> +	unsigned long long s_mount_opt;

I'd really rather *not* make s_mount_opt a long long.  64-bit
operations are slower (especially on 32-bit systems).  It would
probably be better to make it be more covenient to use s_mount_opt2
--- by adding a new mount options flag MOPT_2 which causes MOPT_SET
and MOPT_CLEAR to operate on s_mount_opt2, and then teaching
_ext4_show_options() to support mount options that are set in
s_mount_opt2.   And please do this in a separate commit and explain why
--- because we're running out of bits in s_mount_opt)


> +void save_error_info(struct super_block *sb, const char *func,
> +		     unsigned int line);

Please don't make a formerly static function visible in the global
namespace without adding an ext4_ prefix().

I don't think this is actually the right way to do things, actually.
If you're going to set the error_info, then it should be logged as an
EXT4-fs error.  What I would do instead is add an argument to
__ext4_error() that requests it to skip calling ext4_handle_error().

I think you will find that this will significantly reduce the size of
the patch and the number of lines added.

					- Ted