linux-kernel - Re: [PATCH] [RFQ] dm-integrity: Add a lazy commit mode for journal

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: <3e5a2087667333bb88135a6b6f9620201989605f.camel@elektrobit.com>
Date: Fri, 23 Feb 2024 17:53:57 +0000
From: Weiß, Simone <Simone.Weiss@...ktrobit.com>
To: "mpatocka@...hat.com" <mpatocka@...hat.com>
CC: "song@...nel.org" <song@...nel.org>, "Tomerius, Kai"
	<Kai.Tomerius@...ktrobit.com>, "simone.p.weiss@...teo.net"
	<simone.p.weiss@...teo.net>, "agk@...hat.com" <agk@...hat.com>,
	"linux-raid@...r.kernel.org" <linux-raid@...r.kernel.org>,
	"dm-devel@...ts.linux.dev" <dm-devel@...ts.linux.dev>, "yukuai3@...wei.com"
	<yukuai3@...wei.com>, "linux-kernel@...r.kernel.org"
	<linux-kernel@...r.kernel.org>, "lukas.bulwahn@...il.com"
	<lukas.bulwahn@...il.com>, "snitzer@...nel.org" <snitzer@...nel.org>
Subject: Re: [PATCH] [RFQ] dm-integrity: Add a lazy commit mode for journal

On Tue, 2024-02-20 at 19:52 +0100, Mikulas Patocka wrote:
> CAUTION: This email originated from outside of the Elektrobit organization. Do
> not click links or open attachments unless you recognize the sender and know
> the content is safe.
> 
> 
> On Fri, 9 Feb 2024, Simone Weiß wrote:
> 
> > Extend the dm-integrity driver to omit writing unused journal data sectors.
> > Instead of filling up the whole journal section, mark the last used
> > sector with a special commit ID. The commit ID still uses the same base
> > value,
> > but section number and sector number are inverted. At replay when commit IDs
> > are analyzed this special commit ID is detected as end of valid data for
> > this
> > section. The main goal is to prolong the live times of e.g. eMMCs by
> > avoiding
> > to write the whole journal data sectors.
> > 
> > The change is right now to be seen as experimental and gets applied if
> > CONFIG_DMINT_LAZY_COMMIT is set to y. Note please that this is NOT
> > planned for a final version of the changes. I would make it configurable
> > via flags passed e.g. via dmsetup and stored in the superblock.
> > 
> > Architectural Limitations:
> > - A dm-integrity partition, that was previously used with lazy commit,
> >  can't be replayed with a dm-integrity driver not using lazy commit.
> > - A dm-integrity driver that uses lazy commit is expected
> >  to be able to cope with a partition that was created and used without
> >  lazy commit.
> > - With dm-integrity lazy commit, a partially written journal (e.g. due to a
> >  power cut) can cause a tag mismatch during replay if the journal entry
> > marking
> >  the end of the journal section is missing. Due to lazy commit, older
> > journal
> >  entries are not erased and might be processed if they have the same commit
> > ID
> >  as adjacent newer journal entries.
> 
> Hi
> 
> I was thinking about it and I think that this problem is a showstopper.
> 
> Suppose that a journal section contains these commit IDs:
> 
>         2       2       2       2(EOF)  3       3       3       3
> 
> The IDs "3" are left over from previous iterations. The IDs "2" contain
> the current data. And now, the journal rolls over and we attempt to write
> all 8 pages with the ID "3". However, a power failure happens and we only
> write 4 pages with the ID "3". So, the journal will look like:
> 
>         3(new)  3(new)  3(new)  3(new)  3(old)  3(old)  3(old)  3(old)
> 
> After a reboot, the journal-replay logic will falsely believe that the
> whole journal section is consistent and it will attempt to replay it.
> 
> This could be fixed by having always increasing commit IDs - the commit
> IDs have 8 bytes, so we can assume that they never roll-over and it would
> prevent us from mixing old IDs into the current transaction.
Hi

Thanks for the review of the concept. I was out this week and could only think
about it now. I understood it right, that the proposal is to add an extra value
to the commit ID, that is e.g. incremented when integrity_commit is executed?

If so, I tried this quickly and looks good on first glance. Will check and test
further next.

Simone
> 
> Mikulas
> 
> >  If dm-integrity detects bad sections while
> >  replaying the journal, keep track about those sections and try to at least
> >  replay older, good sections.
> >  This is based on the assumption that most likely the newest
> >  section(s) will be damaged, which might have been only partially written
> >  due to a sudden reset. Previously, the whole journal would be cleared in
> >  such a case.
> > 
> > Signed-off-by: Simone Weiß <simone.weiss@...ktrobit.com>
> > Signed-off-by: Kai Tomerius <kai.tomerius@...ktrobit.com>