linux-kernel - Re: [PATCH 2/3] ext4: introduce ext4_error_remove

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: <20121029024024.GC9365@thunk.org>
Date:	Sun, 28 Oct 2012 22:40:24 -0400
From:	Theodore Ts'o <tytso@....edu>
To:	Dave Chinner <david@...morbit.com>
Cc:	"Luck, Tony" <tony.luck@...el.com>,
	Naoya Horiguchi <n-horiguchi@...jp.nec.com>,
	"Kleen, Andi" <andi.kleen@...el.com>,
	"Wu, Fengguang" <fengguang.wu@...el.com>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Jan Kara <jack@...e.cz>,
	Jun'ichi Nomura <j-nomura@...jp.nec.com>,
	Akira Fujita <a-fujita@...jp.nec.com>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"linux-mm@...ck.org" <linux-mm@...ck.org>,
	"linux-ext4@...r.kernel.org" <linux-ext4@...r.kernel.org>
Subject: Re: [PATCH 2/3] ext4: introduce ext4_error_remove_page

On Mon, Oct 29, 2012 at 12:16:32PM +1100, Dave Chinner wrote:
> 
> Except that there are filesystems that cannot implement such flags,
> or require on-disk format changes to add more of those flags. This
> is most definitely not a filesystem specific behaviour, so any sort
> of VFS level per-file state needs to be kept in xattrs, not special
> flags. Filesystems are welcome to optimise the storage of such
> special xattrs (e.g. down to a single boolean flag in an inode), but
> using a flag for something that dould, in fact, storage the exactly
> offset and length of the corruption is far better than just storing
> a "something is corrupted in this file" bit....

Agreed, if we're going to add an xattr, then we might as well store
not just a boolean, but some indication of what part of the file was
corrupted.  The only complication is what if there are many memory
corruptions.  Do we store just the last ECC hard error that we
detected?  Or just the first?

It wasn't clear to me it was worth the extra complexity, but if there
are indeed for file systems that don't have or don't want to allocate
a spare bit in their inode structure, that might be a good enough
justification to add an xattr.  (Was this a hypothetical, or does this
constraint apply to XFS or some other file system that you're aware of?)

> > I note that we've already added a new error code:
> > 
> > #define EHWPOISON 133	  /* Memory page has hardware error */
> > 
> > ... although the glibc shipping with Debian testing hasn't been taught
> > what it is, so strerror(EHWPOISON) returns "Unknown error 133".  We
> > could simply allow open(2) and stat(2) return this error, although I
> > wonder if we're just better off defining a new error code.
> 
> If we are going to add special new "file corrupted" errors, we
> should add EFSCORRUPTED (i.e. "filesystem corrupted") at the same
> time....

I would dearly love it if we could allocate a new EFSCORRUPTED errno.
I was about to follow XFS's lead and change ext4 to return EUCLEAN
instead of EIO in the cases of fs corruption, but that really is ugly
and gross...

						- Ted
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/