[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <6483105.q6B1KMqZtl@blindfold>
Date: Thu, 01 Nov 2018 10:13:29 +0100
From: Richard Weinberger <richard@....at>
To: Rafał Miłecki <zajec5@...il.com>
Cc: linux-mtd@...ts.infradead.org,
Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
Russell Senior <russell@...sonaltelco.net>,
Stable <stable@...r.kernel.org>
Subject: Re: [PATCH] ubifs: Handle re-linking of inodes correctly while recovery
Am Donnerstag, 1. November 2018, 09:55:53 CET schrieb Rafał Miłecki:
> On Sun, 28 Oct 2018 at 22:44, Richard Weinberger <richard@....at> wrote:
> > UBIFS's recovery code strictly assumes that a deleted inode will never
> > come back, therefore it removes all data which belongs to that inode
> > as soon it faces an inode with link count 0 in the replay list.
> > Before O_TMPFILE this assumption was perfectly fine. With O_TMPFILE
> > it can lead to data loss upon a power-cut.
> >
> > Consider a journal with entries like:
> > 0: inode X (nlink = 0) /* O_TMPFILE was created */
> > 1: data for inode X /* Someone writes to the temp file */
> > 2: inode X (nlink = 0) /* inode was changed, xattr, chmod, … */
> > 3: inode X (nlink = 1) /* inode was re-linked via linkat() */
> >
> > Upon replay of entry #2 UBIFS will drop all data that belongs to inode X,
> > this will lead to an empty file after mounting.
> >
> > As solution for this problem, scan the replay list for a re-link entry
> > before dropping data.
> >
> > Fixes: 474b93704f32 ("ubifs: Implement O_TMPFILE")
> > Cc: stable@...r.kernel.org
> > Reported-by: Russell Senior <russell@...sonaltelco.net>
> > Reported-by: Rafał Miłecki <zajec5@...il.com>
> > Signed-off-by: Richard Weinberger <richard@....at>
>
> Thank you Richard!!!
>
> Tested-by: Rafał Miłecki <rafal@...ecki.pl>
Thanks for testing and providing the reproducer!
I'll send a v2 of the patch soon where I've optimized the list scanning more.
In fact, the correct and fasted approach is walking the replay list backwards
to find the final link state of an inode.
Thanks,
//richard
Powered by blists - more mailing lists