[Ack] Natty SRU: eCryptfs: Clear ECRYPTFS_NEW_FILE flag during truncate

[Leann Ogasawara]

Ack, looks good to me.  Don't forget to add the BugLink to the commits
before you push.

Acked-by: Leann Ogasawara <leann.ogasawara at canonical.com>

On Tue, 2011-10-11 at 15:38 +0100, Tim Gardner wrote:
> On 10/11/2011 01:53 PM, Tim Gardner wrote:
> > On 10/10/2011 04:26 PM, Tyler Hicks wrote:
> >> On 2011-10-10 07:16:59, Leann Ogasawara wrote:
> >>> On Mon, 2011-10-10 at 14:46 +0100, Tim Gardner wrote:
> >>>> On 10/10/2011 02:42 PM, Leann Ogasawara wrote:
> >>>>> On Sun, 2011-10-09 at 05:12 -0600, Tim Gardner wrote:
> >>>>>> From 27ed7cb2b00512e81016419715c1d9b6794b06ae Mon Sep 17 00:00:00
> >>>>>> 2001
> >>>>>> From: Tyler Hicks<tyhicks at canonical.com>
> >>>>>> Date: Fri, 7 Oct 2011 15:54:26 -0500
> >>>>>> Subject: [PATCH] eCryptfs: Clear ECRYPTFS_NEW_FILE flag during
> >>>>>> truncate
> >>>>>>
> >>>>>> BugLink: http://bugs.launchpad.net/bugs/745836
> >>>>>>
> >>>>>> The ECRYPTFS_NEW_FILE crypt_stat flag is set upon creation of a new
> >>>>>> eCryptfs file. When the flag is set, eCryptfs reads directly from the
> >>>>>> lower filesystem when bringing a page up to date. This means that no
> >>>>>> offset translation (for the eCryptfs file metadata in the lower file)
> >>>>>> and no decryption is performed. The flag is cleared just before the
> >>>>>> first write is completed (at the beginning of
> >>>>>> ecryptfs_write_begin()).
> >>>>>>
> >>>>>> It was discovered that if a new file was created and then extended
> >>>>>> with
> >>>>>> truncate, the ECRYPTFS_NEW_FILE flag was not cleared. If pages
> >>>>>> corresponding to this file are ever reclaimed, any subsequent reads
> >>>>>> would result in userspace seeing eCryptfs file metadata and encrypted
> >>>>>> file contents instead of the expected decrypted file contents.
> >>>>>>
> >>>>>> Data corruption is possible if the file is written to before the
> >>>>>> eCryptfs directory is unmounted. The data written will be copied into
> >>>>>> pages which have been read directly from the lower file rather than
> >>>>>> zeroed pages, as would be expected after extending the file with
> >>>>>> truncate.
> >>>>>>
> >>>>>> This flag, and the functionality that used it, was removed in
> >>>>>> upstream
> >>>>>> kernels in 2.6.39 with the following commits:
> >>>>>>
> >>>>>> bd4f0fe8bb7c73c738e1e11bc90d6e2cf9c6e20e
> >>>>>> fed8859b3ab94274c986cbdf7d27130e0545f02c
> >>>>>
> >>>>> Is there a reason we're not just cherry-picking the upstream patches?
> >>>>> And so I would assume this patch should be marked as SAUCE?
> >>>>>
> >>>>> Thanks,
> >>>>> Leann
> >>>>>
> >>>>
> >>>> Yeah, 'UBUNTU: SAUCE:' for sure. Tyler said in the LP report that
> >>>> backporting those 2 commits was getting too involved and complicated.
> >>>> Given the simplicity of his ultimate solution I thought the backport
> >>>> seemed better.
> >>>
> >>> Hrm, those two patches appear to cherry-pick cleanly for me, although I
> >>> could be missing other external factors. Reading the bug report it
> >>> sounds like Tyler originally thought this was fixed with upstream commit
> >>> 3b06b3ebf44170c90c893c6c80916db6e922b9f2 and it was that commit which
> >>> was problematic to backport (see comment #85).
> >>
> >> Yep, I was wrong about 3b06b3eb being the fix. Bad assumption on my
> >> part.
> >>
> >>> It's in the following
> >>> comment #86 that he identifies the actual fix being commits bd4f0fe8 and
> >>> fed8859b.
> >>>
> >>> Regardless, the SAUCE patch looks fine to me. It's straightforward and
> >>> tested. I was just more curious as to why we don't just cherry-pick the
> >>> upstream patches. I've CC'd Tyler to get his reasoning.
> >>
> >> While bd4f0fe8 and fed8859b will cherry-pick cleanly and get rid of the
> >> buggy code, they weren't intended to be bug fixes when I wrote them.
> >> They were just intended to remove some functionality in order to make
> >> the file creation process a bit faster. To me, it just didn't feel like
> >> something that should be backported consider how simple the real fix
> >> was.
> >>
> >> Tyler
> >
> > Tyler - these 2 patches are simple enough that I'd prefer the clean
> > cherry-picks (which we try to use as a matter of policy). I'll retest
> > and send out the results...
> >
> > rtg
> 
> The attached patches are clean cherry-picks and produce the same result 
> without regression.
> 
> rtg