[PATCH] e2fsck: zero-fill shared blocks by default

[PATCH] e2fsck: zero-fill shared blocks by default

[Artem Blagodarenko]

When e2fsck detects multiply-claimed blocks, the default repair
behavior is to clone the duplicate blocks. This is guaranteed to
result in data corruption, and is also a security hole. Typically,
one of the inodes with multiply-claimed blocks is valid, the others
have corrupt extent data referencing some of the same disk blocks
as the valid inode. e2fsck has no way to determine which inode is
the rightful owner of the blocks. When e2fsck is run with the -y
option and duplicate blocks are cloned, those duplicate data blocks
from the valid inode or object are replicated to other objects.

e2fsck has some extended options that provide different ways of
handling duplicate blocks:

clone=dup|zero
shared=preserve|lost+found|delete

The default behavior can be changed with modifications to the
e2fsck.conf file. Let's set clone=zero and replace the shared
blocks with private, zero-filled blocks. Leave shared=preserve
because there are no reasons to move zeroed blocks somethere.

This change doesn't touch e2fsprogs tests because they use
their own enviroment and build their own e2fsck.conf

Signed-off-by: Artem Blagodarenko <artem.blagodarenko@gmail.com>
Reported-by: Peggy Gazzola <peggy.gazzola@hpe.com>
HPE-bug-id: LUS-8408
---
 e2fsck.conf | 3 +++
 1 file changed, 3 insertions(+)
 create mode 100644 e2fsck.conf

diff --git a/e2fsck.conf b/e2fsck.conf
new file mode 100644
index 00000000..751b34cc
--- /dev/null
+++ b/e2fsck.conf
@@ -0,0 +1,3 @@
+[options]
+# Replace the shared blocks with private, zero-filled blocks
+clone = zero
-- 
2.18.4

Re: [PATCH] e2fsck: zero-fill shared blocks by default

[Theodore Ts'o]

On Wed, Apr 07, 2021 at 09:23:23PM -0400, Artem Blagodarenko wrote:
> e2fsck has some extended options that provide different ways of
> handling duplicate blocks:
> 
> clone=dup|zero
> shared=preserve|lost+found|delete
> 

This patch isn't applicable to the upstream e2fsprogs, because we
don't support these extended options.

I'd be open to taking commits to support these options, but I'm not
likely to change the default to be "clone=zero".

> When e2fsck detects multiply-claimed blocks, the default repair
> behavior is to clone the duplicate blocks. This is guaranteed to
> result in data corruption, and is also a security hole.

The data corruption occurred when the file system was corrupted.
Changing the default to zero cloned blocks is guaranteed to make
things worse. 

> Typically,
> one of the inodes with multiply-claimed blocks is valid, the others
> have corrupt extent data referencing some of the same disk blocks
> as the valid inode.

True; but when we clone the shared block, one of the files will
hopefully be made whole.  Zeroing means that *both* files are
guranteed to be corrupted.

Can this potentially be a security problem?  Well, it's up to the
system adminsitrator to take a look at the files that were fixed up
during pass1b handling, and decide which file should be fixed up.  If
the system administrator wants to run e2fsck -fy, and then blindly
bring up the system for sharing... that's on the system adminsitrator.
If they don't care to manually inspect the files with shared blocks
first, then sure, perhaps they should edit /etc/e2fsck.conf and change
the cloned or shared behaviour.

It might be that "shared=lost+found" might be a better choice for
them, since /lost+found is mode 700 (owned by root).  "clone=zero"
leaves both files, now guaranteed to be corrupted, left in place for
the user to trip over the corrupted file.  Moving them to lost+found
means the user still has lost access to both files, but least they are
preserved in /lost+found for the system adminsitrator (or the site
security officer if you are running a system with Mandatory Access
Controls) can look them over and them restore them to the user if that
is appropriate/safe.

I'll note, though, that if you have some directory corruption that
causes some file such as /etc/hosts.deny, /etc/iptable/rules or
/etc/ufw/ufw.conf to end up in /lost+found, blindly bringing up the
system after running the e2fsck -fy hammer isn't necessarily going to
be safe, either.  The whole *point* of e2fsck -p is that is
automatically safe, and if it fails to make some kind of fix, it's
because an intelligent human is supposed to drive, and there may be a
need to make manual adjustments to the file system or perhaps tell
e2fsck *not* to make an obvious fix, in the interests of recovering as
much data as possible.

The real problem is people who think running "e2fsck -fy" is
automatically safe and all will be better...

Cheers,

					- Ted