Linux-ext4 Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH] e2fsck: zero-fill shared blocks by default
@ 2021-04-08  1:23 Artem Blagodarenko
  2021-05-07 23:22 ` Theodore Ts'o
  0 siblings, 1 reply; 2+ messages in thread
From: Artem Blagodarenko @ 2021-04-08  1:23 UTC (permalink / raw)
  To: linux-ext4; +Cc: adilger.kernel, 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


^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: [PATCH] e2fsck: zero-fill shared blocks by default
  2021-04-08  1:23 [PATCH] e2fsck: zero-fill shared blocks by default Artem Blagodarenko
@ 2021-05-07 23:22 ` Theodore Ts'o
  0 siblings, 0 replies; 2+ messages in thread
From: Theodore Ts'o @ 2021-05-07 23:22 UTC (permalink / raw)
  To: Artem Blagodarenko; +Cc: linux-ext4, adilger.kernel, Peggy Gazzola

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

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, back to index

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-04-08  1:23 [PATCH] e2fsck: zero-fill shared blocks by default Artem Blagodarenko
2021-05-07 23:22 ` Theodore Ts'o

Linux-ext4 Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-ext4/0 linux-ext4/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-ext4 linux-ext4/ https://lore.kernel.org/linux-ext4 \
		linux-ext4@vger.kernel.org
	public-inbox-index linux-ext4

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-ext4


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git