linux-ext4 - Re: [PATCH 1/2] ext4: Remove arbitrary block value in __es_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]

Date:	Fri, 18 Apr 2014 13:24:04 +0200 (CEST)
From:	Lukáš Czerner <lczerner@...hat.com>
To:	"Theodore Ts'o" <tytso@....edu>
cc:	linux-ext4@...r.kernel.org
Subject: Re: [PATCH 1/2] ext4: Remove arbitrary block value in
 __es_remove_extent()

On Fri, 18 Apr 2014, Theodore Ts'o wrote:

> Date: Fri, 18 Apr 2014 07:18:26 -0400
> From: Theodore Ts'o <tytso@....edu>
> To: Lukáš Czerner <lczerner@...hat.com>
> Cc: linux-ext4@...r.kernel.org
> Subject: Re: [PATCH 1/2] ext4: Remove arbitrary block value in
>     __es_remove_extent()
> 
> On Fri, Apr 18, 2014 at 11:22:12AM +0200, Lukáš Czerner wrote:
> > Aside from the fact that this is totally undocumented and there is
> > not even comment on what is that all about in couple of years we
> > might actually get file systems big enough that this would not be an
> > I/O error anymore (that might be a bit of a stretch).
> 
> Well, do you have some suggestions about what might be a good place to
> document something like this?  My assumption is that it's something
> that would be used by developers after a bug had been reported, so
> presumably it would be someplace in the source code.
> 
> And I used "0x7FDEADBEEF" deliberately so that it would be a extremely
> unlikely we would have file systems that big (we're approximately 512
> PB, and honestly, if we had fixed all of the scaling limits such that
> it was sane to think someone would want to be using ext4 with a file
> system that big --- well, that would be a very nice problem to have
> :-)

Indeed :)

> 
> > But mainly this value is only going to be used if it is delayed
> > extent or a hole which implies that it has not been mapped and
> > pblock does not contain anything valid. And if we really screwed it
> > up and tried to use pblock of extent which is a hole or delayed
> > extent, then it would not help us anyway since the only place that
> > we actually set this is when splitting extent on removal.
> > 
> > Now I can see that in ext4_da_map_blocks() we're actually using ~0
> > value for the pblock which is a bit better I think as long as we're
> > using this reliably. So I'll resend the patch which will make sure
> > that we're using ~0 reliably when storin delayed, or hole extents in
> > the extent status tree. Does that make sense ?
> 
> So the technique that we're using in mballoc.c is that we use
> different illegal flag values depending on where the bad value was
> introduced:
> 
> % grep "debug value" fs/ext4/mballoc.c
> 	ex.fe_logical = 0xDEADFA11; /* debug value */
> 		ex.fe_logical = 0xDEADC0DE; /* debug value */
> 				ex.fe_logical = 0xDEADF00D; /* debug value */
> 
> I think it might be useful to do the same for the physical blocks in
> the extent_status tree.
> 
> 	  				- Ted

I agree. I'll send a patch.

Thanks!
-Lukas