linux-ext4 - Re: [PATCH 1/2] resize2fs: Add support for lazy itable initialization

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: <5B2E3DA2-B789-41BE-8534-BB19D1B9FA44@dilger.ca>
Date:	Tue, 1 Feb 2011 12:15:17 -0700
From:	Andreas Dilger <adilger@...ger.ca>
To:	Lukas Czerner <lczerner@...hat.com>
Cc:	linux-ext4@...r.kernel.org, tytso@....EDU
Subject: Re: [PATCH 1/2] resize2fs: Add support for lazy itable initialization

On 2011-02-01, at 10:14, Lukas Czerner wrote:
> This commit adds extended options '-E' to the resize2fs code along with
> the first extended option lazy_itable_init=n.

Here it says the option is "=n", is it also possible to force this with "=y"?

> +static void parse_extended_opts(int *flags, const char *opts)
> +{
> +		if (!strcmp(token, "lazy_itable_init")) {
> +			int lazy;
> +			if (arg)
> +				lazy = strtoul(arg, &p, 0);
> +			else
> +				lazy = 1;
> +			if (lazy)
> +				*flags |= RESIZE_LAZY_ITABLE_INIT;

Here it parses the option as "=0" or "=1", not "=n" or "=y".  It looks like "=n" will accidentally return 0 from strtoul(), so that works as expected, but the "=y" option would fail (it would also return 0).

> +	if (r_usage) {
> +		fprintf(stderr, _("\nBad option(s) specified: %s\n\n"
> +			"\tlazy_itable_init=<0 to disable, 1 to enable>\n\n"),

It looks a bit confusing "=<0 to disable", I thought initially that meant any value <= 0 would disable it, though I later see that there is a closing '>'.  I think it is more standard to use "{}" braces for "one of these options must be given".

> @@ -232,6 +290,12 @@ int main (int argc, char ** argv)
> +	if (access("/sys/fs/ext4/features/lazy_itable_init", R_OK) == 0)
> +		flags |= RESIZE_LAZY_ITABLE_INIT;
> +
> +	if (extended_opts)
> +		parse_extended_opts(&flags, extended_opts);

Good that the command-line options override the default value.

> +.B lazy_itable_init\fR[\fB= \fI<0 to disable, 1 to enable>\fR]

Similarly, this should use "{}" around the options instead of "<>".

> +If enabled and the uninit_bg feature is enabled, the inode table will
> +not be fully initialized by

I would write "not be zeroed out on disk", since it is otherwise unclear
if "not fully initialized" means that there will be less inodes available
or some other issues if the inode table is not "fully" initialized.

Cheers, Andreas

--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html