From mboxrd@z Thu Jan  1 00:00:00 1970
From: =?iso-8859-1?Q?Br=FCns=2C_Stefan?= <Stefan.Bruens@rwth-aachen.de>
Date: Mon, 5 Sep 2016 14:37:46 +0000
Subject: [U-Boot] ext4 delete file fails when ext4 extents enabled in
 filesystem
In-Reply-To: <1BBC30BFBAE48D49A8C663EE5D39C03F01B7F1A7B5@SDEMUCMB02.kontron.local>
References: <1BBC30BFBAE48D49A8C663EE5D39C03F01B7F1A70D@SDEMUCMB02.kontron.local>
 <8673131.TQAZLALiPZ@sbruens-linux>
 <1BBC30BFBAE48D49A8C663EE5D39C03F01B7F1A7B5@SDEMUCMB02.kontron.local>
Message-ID: <2565760.r7uvUPFgsy@sbruens-linux>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de

On Freitag, 2. September 2016 14:51:59 CEST Thomas Schaefer wrote:
> > -----Urspr?ngliche Nachricht-----
> > Von: Br?ns, Stefan [mailto:Stefan.Bruens at rwth-aachen.de]
> > Gesendet: Freitag, 2. September 2016 13:43
> > An: Thomas Schaefer
> > Cc: u-boot at lists.denx.de; Michael Walle
> > Betreff: Re: ext4 delete file fails when ext4 extents enabled in
> > filesystem
> > 
> > On Donnerstag, 1. September 2016 19:25:30 CEST you wrote:
> > > On Donnerstag, 1. September 2016 16:08:51 CEST you wrote:
> > > > Hi Stefan,
> > > > 
> > > > applying patch [U-Boot,v4,06/13]ext4 and Michael Walles patch
> > > > [U-Boot,v4,3/4]ext4, I'm now able to write into directories on ext4
> > > > fs from u-boot. However, when deleting a given file (i.e. when
> > > > writing to an existing filename), u-boot crashes when ext4 extents
> > > > are enabled.
> > > > 
> > > > Some debugging showd that blknr from 'read_allocated_block' function
> > > > returns negative value. I can only guess, maybe its due to 64 bit
> > > > values calculated from ee_start_hi and ee_start_lo entries in the
> > > > ext4_extent structure.
> > > > 
> > > > When disabling extents in the ext4 fs, deleting a given file is
> > > > working.
[...]
> > Hi Thomas,
> > 
> > short followup:
> > 
> > read_allocated_blocks returns either 0 or -1 in case of an error.
> > Unfortunately, the return value is only checked for 0 equality in
> > most/all?
> > cases, and seemingly my patch series introduced some more occasions.
> > 
> > 
> > Now, what *should* read_allocated_blocks return in case of an error?
> > Either:
> > 
> > - 0: a file block can never be allocated as block 0, as that is always in
> > use by the superblock and/or the bootsector block.
> > 
> > - <0: Extents allow 48 bit block numbers. "Limiting" the return value to
> > the positive half of int64_t for valid block numbers and and reserving
> > negative values for error codes is fine.
> > 
> > I would go for negative error codes, as these are more expressive.
> > Comments/ opinions welcome!

Following up on this, the correct behaviour is <0 on "real" errors, like -
ENOMEM, and 0 on blocks not backed on device (e.g. sparse files).

Followup patch in progress.


[...]
> Hi Stefan,
> 
> the attachment contains an image file that causes u-boot to crash when
> trying to overwrite existing files in ext4 fs.

Could reproduce this. The problem seems to be an out-of-bound access of an in-
memory struct ext2_inode, and mixing up its size with fs->inodesz. I have 
found at least one place, will investigate further.

Kind regards,

Stefan