UBI: recover_peb and power cut safety

UBI: recover_peb and power cut safety

[Jörg Pfähler]

Hi,

I would greatly appreciate some clarification with respect to power cut safety 
during writing of an erase block in UBI, specifically power cut safety of 
recover_peb.

During a normal write operation (ubi_eba_write_leb in mtd/ubi/eba.c) UBI tries 
to move the contents of the block (and the new contents) to a new location via 
recover_peb, if the write fails. However, recover_peb does not seem to use the 
capability to exchange the (logical) block atomically (as 
ubi_eba_atomic_leb_change in mtd/ubi/eba.c does). Specifically, it does not 
seem to write the amount of data and its checksum to the VID header. Thus, if 
the system crashes in the middle of recover_peb before the old/broken block 
could be erased, we are left with a newer version of the block (the sequence 
number in the header is increased by recover_peb), but without having moved 
all the contents of the old block. This would obviously lead to data loss. 
Thus, It seems to me that recover_peb (and therefore ubi_eba_write_leb) is not 
power cut safe or is there some other mechanism distinct from the one used by 
ubi_eba_atomic_leb_change to achieve this? If not I would suggest using 
ubi_eba_atomic_leb_change in ubi_eba_write_leb instead of recover_peb.

Thanks in advance for any clarifications!
Regards,
Jörg

--------------------------------------------------------------------------------------
Jörg Pfähler
Lehrstuhl für Softwaretechnik
Institut für Software and Systems Engineering
Universität Augsburg

Universitätsstr. 6a, Raum 3014
tel:      (+49) 821/598-2229
e-mail:   joerg.pfaehler@informatik.uni-
augsburg.de<mailto:joerg.pfaehler@informatik.uni-augsburg.de>
--------------------------------------------------------------------------------------

Re: UBI: recover_peb and power cut safety

[Richard Weinberger]

Jörg,

On Thu, Jun 16, 2016 at 10:37 AM, Jörg Pfähler <pfaehler@isse.de> wrote:
> Hi,
>
> I would greatly appreciate some clarification with respect to power cut safety
> during writing of an erase block in UBI, specifically power cut safety of
> recover_peb.
>
> During a normal write operation (ubi_eba_write_leb in mtd/ubi/eba.c) UBI tries
> to move the contents of the block (and the new contents) to a new location via
> recover_peb, if the write fails. However, recover_peb does not seem to use the
> capability to exchange the (logical) block atomically (as
> ubi_eba_atomic_leb_change in mtd/ubi/eba.c does). Specifically, it does not
> seem to write the amount of data and its checksum to the VID header. Thus, if
> the system crashes in the middle of recover_peb before the old/broken block
> could be erased, we are left with a newer version of the block (the sequence
> number in the header is increased by recover_peb), but without having moved
> all the contents of the old block. This would obviously lead to data loss.
> Thus, It seems to me that recover_peb (and therefore ubi_eba_write_leb) is not
> power cut safe or is there some other mechanism distinct from the one used by
> ubi_eba_atomic_leb_change to achieve this? If not I would suggest using
> ubi_eba_atomic_leb_change in ubi_eba_write_leb instead of recover_peb.

Hmm, you are right, if ubi_eba_write() is facing -EIO from the MTD driver we can
lose the whole erase block upon power cut.
So you found a bug. :-)

Artem, can you tell more on this?
I'd guess that recover_peb() is older than ubi_eba_atomic_leb_change() and
therefore it was not used.
And nobody noticed so far since the condition is hard to hit.

That said, switching to ubi_eba_atomic_leb_change() seems like a good
plan to me.
Jörg, please send a patch and explain how you tested it.

-- 
Thanks,
//richard

Re: UBI: recover_peb and power cut safety

[Richard Weinberger]

Forgot to CC Artem.

On Thu, Jun 16, 2016 at 11:46 AM, Richard Weinberger
<richard.weinberger@gmail.com> wrote:
> Jörg,
>
> On Thu, Jun 16, 2016 at 10:37 AM, Jörg Pfähler <pfaehler@isse.de> wrote:
>> Hi,
>>
>> I would greatly appreciate some clarification with respect to power cut safety
>> during writing of an erase block in UBI, specifically power cut safety of
>> recover_peb.
>>
>> During a normal write operation (ubi_eba_write_leb in mtd/ubi/eba.c) UBI tries
>> to move the contents of the block (and the new contents) to a new location via
>> recover_peb, if the write fails. However, recover_peb does not seem to use the
>> capability to exchange the (logical) block atomically (as
>> ubi_eba_atomic_leb_change in mtd/ubi/eba.c does). Specifically, it does not
>> seem to write the amount of data and its checksum to the VID header. Thus, if
>> the system crashes in the middle of recover_peb before the old/broken block
>> could be erased, we are left with a newer version of the block (the sequence
>> number in the header is increased by recover_peb), but without having moved
>> all the contents of the old block. This would obviously lead to data loss.
>> Thus, It seems to me that recover_peb (and therefore ubi_eba_write_leb) is not
>> power cut safe or is there some other mechanism distinct from the one used by
>> ubi_eba_atomic_leb_change to achieve this? If not I would suggest using
>> ubi_eba_atomic_leb_change in ubi_eba_write_leb instead of recover_peb.
>
> Hmm, you are right, if ubi_eba_write() is facing -EIO from the MTD driver we can
> lose the whole erase block upon power cut.
> So you found a bug. :-)
>
> Artem, can you tell more on this?
> I'd guess that recover_peb() is older than ubi_eba_atomic_leb_change() and
> therefore it was not used.
> And nobody noticed so far since the condition is hard to hit.
>
> That said, switching to ubi_eba_atomic_leb_change() seems like a good
> plan to me.
> Jörg, please send a patch and explain how you tested it.
>
> --
> Thanks,
> //richard



-- 
Thanks,
//richard

Re: UBI: recover_peb and power cut safety

[Artem Bityutskiy]

On Thu, 2016-06-16 at 12:03 +0200, Richard Weinberger wrote:
> Forgot to CC Artem.
> 
> On Thu, Jun 16, 2016 at 11:46 AM, Richard Weinberger
> <richard.weinberger@gmail.com> wrote:
> > 
> > Jörg,
> > 
> > On Thu, Jun 16, 2016 at 10:37 AM, Jörg Pfähler <pfaehler@isse.de>
> > wrote:
> > > 
> > > Hi,
> > > 
> > > I would greatly appreciate some clarification with respect to
> > > power cut safety
> > > during writing of an erase block in UBI, specifically power cut
> > > safety of
> > > recover_peb.
> > > 
> > > During a normal write operation (ubi_eba_write_leb in
> > > mtd/ubi/eba.c) UBI tries
> > > to move the contents of the block (and the new contents) to a new
> > > location via
> > > recover_peb, if the write fails. However, recover_peb does not
> > > seem to use the
> > > capability to exchange the (logical) block atomically (as
> > > ubi_eba_atomic_leb_change in mtd/ubi/eba.c does). Specifically,
> > > it does not
> > > seem to write the amount of data and its checksum to the VID
> > > header. Thus, if
> > > the system crashes in the middle of recover_peb before the
> > > old/broken block
> > > could be erased, we are left with a newer version of the block
> > > (the sequence
> > > number in the header is increased by recover_peb), but without
> > > having moved
> > > all the contents of the old block. This would obviously lead to
> > > data loss.
> > > Thus, It seems to me that recover_peb (and therefore
> > > ubi_eba_write_leb) is not
> > > power cut safe or is there some other mechanism distinct from the
> > > one used by
> > > ubi_eba_atomic_leb_change to achieve this? If not I would suggest
> > > using
> > > ubi_eba_atomic_leb_change in ubi_eba_write_leb instead of
> > > recover_peb.
> > Hmm, you are right, if ubi_eba_write() is facing -EIO from the MTD
> > driver we can
> > lose the whole erase block upon power cut.
> > So you found a bug. :-)
> > 
> > Artem, can you tell more on this?
> > I'd guess that recover_peb() is older than
> > ubi_eba_atomic_leb_change() and
> > therefore it was not used.
> > And nobody noticed so far since the condition is hard to hit.
> > 
> > That said, switching to ubi_eba_atomic_leb_change() seems like a
> > good
> > plan to me.
> > Jörg, please send a patch and explain how you tested it.

Yes indeed, very bad bug, good catch.

Re: UBI: recover_peb and power cut safety

[Jörg Pfähler]

Hi Richard,

> Hmm, you are right, if ubi_eba_write() is facing -EIO from the MTD driver we
> can lose the whole erase block upon power cut.
> So you found a bug. :-)
>
> Artem, can you tell more on this?
> I'd guess that recover_peb() is older than ubi_eba_atomic_leb_change() and
> therefore it was not used.
> And nobody noticed so far since the condition is hard to hit.
> 
> That said, switching to ubi_eba_atomic_leb_change() seems like a good
> plan to me.
> Jörg, please send a patch and explain how you tested it.

First of all, thanks for confirming this bug so quickly.

However, we would like to refrain from providing a patch. A little inspection 
of the code revealed that the locking is different for both methods, i.e., one 
cannot just call ubi_eba_atomic_leb_change. So it would be a larger change to 
the code than we thought. Furthermore, we have no experience in writing or 
debugging code for the linux kernel and we own only one flash chip for 
testing, so we would not feel very confident in any patch we could provide.

We found the bug rather in the formal verification of a model of UBI/UBIFS 
(see http://www.isse.de/flashix for more details on our project), where it 
turned out that we modeled the feature differently.

MfG,
Jörg

--------------------------------------------------------------------------------------
Jörg Pfähler
Lehrstuhl für Softwaretechnik
Institut für Software and Systems Engineering
Universität Augsburg

Universitätsstr. 6a, Raum 3014
tel:      (+49) 821/598-2229
e-mail:   pfaehler@isse.de<mailto:pfaehler@isse.de>
--------------------------------------------------------------------------------------