linux-kernel - Re: [PATCH 2/2] dm-integrity: introduce the Inline mode

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-next>] [day] [month] [year] [list]

Message-ID: <512a07c-d41a-27c4-b1f9-8fe19352d1f@linux-m68k.org>
Date: Mon, 26 Aug 2024 10:08:41 +0200 (CEST)
From: Geert Uytterhoeven <geert@...ux-m68k.org>
To: Mikulas Patocka <mpatocka@...hat.com>
cc: Mike Snitzer <snitzer@...nel.org>, dm-devel@...ts.linux.dev, 
    linux-kernel@...r.kernel.org, Arnd Bergmann <arnd@...db.de>
Subject: Re: [PATCH 2/2] dm-integrity: introduce the Inline mode

 	Hi Mikulas,

On Tue, 25 Jun 2024, Mikulas Patocka wrote:
> This commit introduces a new 'I' mode for dm-integrity.
>
> The 'I' mode may be selected if the underlying device has non-power-of-2
> sector size. In this mode, dm-integrity will store integrity data
> directly in device's sectors and it will not use journal.
>
> This mode improves performance and reduces flash wear because there would
> be no journal writes.
>
> Signed-off-by: Mikulas Patocka <mpatocka@...hat.com>

Thanks for your patch, which is now commit fb0987682c629c1d
("dm-integrity: introduce the Inline mode") in v6.11-rc1.

> @@ -4433,9 +4737,12 @@ static int dm_integrity_ctr(struct dm_ta
> 		ti->error = "Block size doesn't match the information in superblock";
> 		goto bad;
> 	}
> -	if (!le32_to_cpu(ic->sb->journal_sections)) {
> +	if (!le32_to_cpu(ic->sb->journal_sections) != (ic->mode == 'I')) {

As reporting before in e.g. [1], this is causing build failures with
gcc-5 and -Werror:

     drivers/md/dm-integrity.c:4718:52: error: logical not is only applied to the left hand side of comparison [-Werror=logical-not-parentheses]
       if (!le32_to_cpu(ic->sb->journal_sections) != (ic->mode == 'I')) {
 							^

I could reproduce the issue with gcc-5(Ubuntu 5.5.0-12ubuntu1) on x86 .

The condition indeed looks hard to read. Perhaps it should be rewritten as
"le32_to_cpu(ic->sb->journal_sections) == (ic->mode == 'I')"?

> 		r = -EINVAL;
> -		ti->error = "Corrupted superblock, journal_sections is 0";
> +		if (ic->mode != 'I')
> +			ti->error = "Corrupted superblock, journal_sections is 0";
> +		else
> +			ti->error = "Corrupted superblock, journal_sections is not 0";
> 		goto bad;
> 	}
> 	/* make sure that ti->max_io_len doesn't overflow */

[1] https://lore.kernel.org/20240729092807.2235937-1-geert@linux-m68k.org

Gr{oetje,eeting}s,

 						Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@...ux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
 							    -- Linus Torvalds