Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1361459 > unrolled thread

[GIT PULL] f2fs updates for v4.6

Back to article view | Back to linux.kernel

Contents

  [GIT PULL] f2fs updates for v4.6 Jaegeuk Kim <jaegeuk@kernel.org> - 2016-03-21 00:00 +0100
    Re: [GIT PULL] f2fs updates for v4.6 Linus Torvalds <torvalds@linux-foundation.org> - 2016-03-21 19:10 +0100
      Re: [GIT PULL] f2fs updates for v4.6 Jaegeuk Kim <jaegeuk@kernel.org> - 2016-03-21 21:10 +0100
        Re: [GIT PULL] f2fs updates for v4.6 Eric Biggers <ebiggers3@gmail.com> - 2016-03-26 07:20 +0100
          Re: [GIT PULL] f2fs updates for v4.6 Jaegeuk Kim <jaegeuk@kernel.org> - 2016-03-26 10:00 +0100
            Re: [GIT PULL] f2fs updates for v4.6 Eric Biggers <ebiggers3@gmail.com> - 2016-03-26 14:50 +0100
              Re: [GIT PULL] f2fs updates for v4.6 Linus Torvalds <torvalds@linux-foundation.org> - 2016-03-26 19:00 +0100
                Re: [GIT PULL] f2fs updates for v4.6 Linus Torvalds <torvalds@linux-foundation.org> - 2016-03-26 21:10 +0100
                  Re: [GIT PULL] f2fs updates for v4.6 Jaegeuk Kim <jaegeuk@kernel.org> - 2016-03-26 21:30 +0100
                Re: [GIT PULL] f2fs updates for v4.6 Jaegeuk Kim <jaegeuk@kernel.org> - 2016-03-26 21:10 +0100
                Re: [GIT PULL] f2fs updates for v4.6 Theodore Ts'o <tytso@mit.edu> - 2016-03-26 21:50 +0100
                  Re: [GIT PULL] f2fs updates for v4.6 Linus Torvalds <torvalds@linux-foundation.org> - 2016-03-26 21:50 +0100
                    Re: [GIT PULL] f2fs updates for v4.6 Theodore Ts'o <tytso@mit.edu> - 2016-03-26 22:00 +0100

#1361459 — [GIT PULL] f2fs updates for v4.6

Hi Linus,

I made another pull request which removes the previous wrong commits and adds
a single commit to migrate the f2fs crypto into fs/crypto.

Could you please consider to pull this?

Thanks,

The following changes since commit 4de8ebeff8ddefaceeb7fc6a9b1a514fc9624509:

  Merge tag 'trace-fixes-v4.5-rc5' of git://git.kernel.org/pub/scm/linux/kernel/git/rostedt/linux-trace (2016-02-22 14:09:18 -0800)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/jaegeuk/f2fs.git tags/for-f2fs-4.6

for you to fetch changes up to 12bb0a8fd47e6020a7b52dc283a2d855f03d6ef5:

  f2fs: submit node page write bios when really required (2016-03-17 21:19:47 -0700)

----------------------------------------------------------------
= New Features =
 - uplift filesystem encryption into fs/crypto/
 - give sysfs entries to control memroy consumption

= Enhancement ==
 - aio performance by preallocating blocks in ->write_iter
 - use writepages lock for only WB_SYNC_ALL
 - avoid redundant inline_data conversion
 - enhance forground GC
 - use wait_for_stable_page as possible
 - speed up SEEK_DATA and fiiemap

= Bug Fixes =
 - corner case in terms of -ENOSPC for inline_data
 - hung task caused by long latency in shrinker
 - corruption between atomic write and f2fs_trace_pid
 - avoid garbage lengths in dentries
 - revoke atomicly written pages if an error occurs

In addition, there are various minor bug fixes and clean-ups.

----------------------------------------------------------------
Arnd Bergmann (1):
      f2fs: add missing argument to f2fs_setxattr stub

Chao Yu (33):
      f2fs: relocate is_merged_page
      f2fs: flush dirty nat entries when exceeding threshold
      f2fs: export dirty_nats_ratio in sysfs
      f2fs: correct search area in get_new_segment
      f2fs: enhance foreground GC
      f2fs: simplify f2fs_map_blocks
      f2fs: simplify __allocate_data_blocks
      f2fs: remove unneeded pointer conversion
      f2fs: introduce get_next_page_offset to speed up SEEK_DATA
      f2fs: speed up handling holes in fiemap
      f2fs: introduce f2fs_submit_merged_bio_cond
      f2fs: split drop_inmem_pages from commit_inmem_pages
      f2fs: support revoking atomic written pages
      f2fs crypto: make sure the encryption info is initialized on opendir(2)
      f2fs crypto: handle unexpected lack of encryption keys
      f2fs crypto: avoid unneeded memory allocation when {en/de}crypting symlink
      f2fs: introduce f2fs_journal struct to wrap journal info
      f2fs: enhance IO path with block plug
      f2fs: split journal cache from curseg cache
      f2fs: reorder nat cache lock in cache_nat_entry
      f2fs: detect error of update_dent_inode in ->rename
      f2fs: fix to delete old dirent in converted inline directory in ->rename
      f2fs: fix the wrong stat count of calling gc
      f2fs: show more info about superblock recovery
      f2fs: try to flush inode after merging inline data
      f2fs: trace old block address for CoWed page
      f2fs: fix incorrect upper bound when iterating inode mapping tree
      f2fs crypto: fix incorrect positioning for GCing encrypted data page
      f2fs: introduce f2fs_update_data_blkaddr for cleanup
      f2fs: introduce f2fs_flush_merged_bios for cleanup
      f2fs: fix to avoid deadlock when merging inline data
      f2fs: clean up opened code with f2fs_update_dentry
      f2fs: fix to avoid unneeded unlock_new_inode

Fan Li (2):
      f2fs: avoid unnecessary search while finding victim in gc
      f2fs: modify the readahead method in ra_node_page()

Hou Pengyang (2):
      f2fs: reconstruct the code to free an extent_node
      f2fs: improve shrink performance of extent nodes

Jaegeuk Kim (32):
      f2fs: remove needless condition check
      f2fs: use writepages->lock for WB_SYNC_ALL
      f2fs: fix to overcome inline_data floods
      f2fs: do f2fs_balance_fs when block is allocated
      f2fs: avoid multiple node page writes due to inline_data
      f2fs: don't need to sync node page at every time
      f2fs: avoid needless sync_inode_page when reading inline_data
      f2fs: don't need to call set_page_dirty for io error
      f2fs: use wait_for_stable_page to avoid contention
      f2fs: use wq_has_sleeper for cp_wait wait_queue
      f2fs: move extent_node list operations being coupled with rbtree operation
      f2fs: don't set cached_en if it will be freed
      f2fs: give scheduling point in shrinking path
      f2fs: wait on page's writeback in writepages path
      f2fs: flush bios to handle cp_error in put_super
      f2fs: fix conflict on page->private usage
      f2fs: move dio preallocation into f2fs_file_write_iter
      f2fs: preallocate blocks for buffered aio writes
      f2fs: increase i_size to avoid missing data
      f2fs crypto: replace some BUG_ON()'s with error checks
      f2fs crypto: fix spelling typo in comment
      f2fs crypto: f2fs_page_crypto() doesn't need a encryption context
      f2fs crypto: check for too-short encrypted file names
      f2fs crypto: add missing locking for keyring_key access
      f2fs: use correct errno
      f2fs crypto: sync with ext4's fname padding
      f2fs: avoid garbage lenghs in dentries
      f2fs: remain last victim segment number ascending order
      fs crypto: move per-file encryption from f2fs tree to fs/crypto
      f2fs crypto: sync ext4_lookup and ext4_file_open
      f2fs: declare static functions
      f2fs: submit node page write bios when really required

Keith Mok (1):
      f2fs: use cryptoapi crc32 functions

Liu Xue (1):
      f2fs: recovery missing dot dentries in root directory

Shawn Lin (3):
      f2fs: slightly reorganize read_raw_super_block
      f2fs: move sanity checking of cp into get_valid_checkpoint
      f2fs: reuse read_inline_data for f2fs_convert_inline_page

Sheng Yong (1):
      f2fs: fix endianness of on-disk summary_footer

Shuoran Liu (1):
      f2fs: introduce lifetime write IO statistics

Yang Shi (1):
      f2fs: mutex can't be used by down_write_nest_lock()

Yunlei He (3):
      f2fs: delete unnecessary wait for page writeback
      f2fs: fix missing skip pages info
      f2fs: avoid hungtask problem caused by losing wake_up

 Documentation/ABI/testing/sysfs-fs-f2fs    |  14 +
 fs/Kconfig                                 |   2 +
 fs/Makefile                                |   1 +
 fs/crypto/Kconfig                          |  18 +
 fs/crypto/Makefile                         |   3 +
 fs/crypto/crypto.c                         | 556 +++++++++++++++++++++++++++++
 fs/{f2fs/crypto_fname.c => crypto/fname.c} | 245 ++++++-------
 fs/crypto/keyinfo.c                        | 278 +++++++++++++++
 fs/crypto/policy.c                         | 229 ++++++++++++
 fs/f2fs/Kconfig                            |  12 +-
 fs/f2fs/Makefile                           |   2 -
 fs/f2fs/checkpoint.c                       |  77 ++--
 fs/f2fs/crypto.c                           | 491 -------------------------
 fs/f2fs/crypto_key.c                       | 254 -------------
 fs/f2fs/crypto_policy.c                    | 209 -----------
 fs/f2fs/data.c                             | 428 +++++++++++-----------
 fs/f2fs/dir.c                              |  94 ++---
 fs/f2fs/extent_cache.c                     | 176 ++++-----
 fs/f2fs/f2fs.h                             | 315 +++++++---------
 fs/f2fs/f2fs_crypto.h                      | 151 --------
 fs/f2fs/file.c                             | 114 +++---
 fs/f2fs/gc.c                               | 245 +++++++------
 fs/f2fs/inline.c                           |  43 +--
 fs/f2fs/inode.c                            |  15 +-
 fs/f2fs/namei.c                            | 168 +++++----
 fs/f2fs/node.c                             | 223 ++++++++----
 fs/f2fs/node.h                             |  26 +-
 fs/f2fs/recovery.c                         |  14 +-
 fs/f2fs/segment.c                          | 386 ++++++++++++--------
 fs/f2fs/segment.h                          |   5 +-
 fs/f2fs/super.c                            | 204 +++++++----
 fs/f2fs/trace.c                            |   6 +-
 fs/f2fs/xattr.c                            |   6 +-
 fs/f2fs/xattr.h                            |   3 +-
 include/linux/dcache.h                     |   2 +
 include/linux/f2fs_fs.h                    |  34 +-
 include/linux/fs.h                         |   8 +
 include/linux/fscrypto.h                   | 433 ++++++++++++++++++++++
 include/trace/events/f2fs.h                |  12 +-
 include/uapi/linux/fs.h                    |  18 +
 40 files changed, 3142 insertions(+), 2378 deletions(-)
 create mode 100644 fs/crypto/Kconfig
 create mode 100644 fs/crypto/Makefile
 create mode 100644 fs/crypto/crypto.c
 rename fs/{f2fs/crypto_fname.c => crypto/fname.c} (60%)
 create mode 100644 fs/crypto/keyinfo.c
 create mode 100644 fs/crypto/policy.c
 delete mode 100644 fs/f2fs/crypto.c
 delete mode 100644 fs/f2fs/crypto_key.c
 delete mode 100644 fs/f2fs/crypto_policy.c
 delete mode 100644 fs/f2fs/f2fs_crypto.h
 create mode 100644 include/linux/fscrypto.h

[toc] | [next] | [standalone]

#1362055

On Sun, Mar 20, 2016 at 3:58 PM, Jaegeuk Kim <jaegeuk@kernel.org> wrote:
>
> I made another pull request which removes the previous wrong commits and adds
> a single commit to migrate the f2fs crypto into fs/crypto.

Ok, merged.  Please double-check the end result, but since most of the
skcipher update was literally to just change "ablk" to "sk", the merge
seems to have been pretty trivial.

It all compiles for me, but since I don't use f2fs, I have nothing to test.

(It's not pushed out quite yet - I'm still test-building everything
else, so if you pull my tree and don't see the merge, wait a few
minutes and it should be there).

                Linus

[toc] | [prev] | [next] | [standalone]

#1362126

On Mon, Mar 21, 2016 at 11:09:45AM -0700, Linus Torvalds wrote:
> On Sun, Mar 20, 2016 at 3:58 PM, Jaegeuk Kim <jaegeuk@kernel.org> wrote:
> >
> > I made another pull request which removes the previous wrong commits and adds
> > a single commit to migrate the f2fs crypto into fs/crypto.
> 
> Ok, merged.  Please double-check the end result, but since most of the
> skcipher update was literally to just change "ablk" to "sk", the merge
> seems to have been pretty trivial.
> 
> It all compiles for me, but since I don't use f2fs, I have nothing to test.

Thank you very much for the merge.
I got the merged branch, and started testing f2fs.

Thanks,

> 
> (It's not pushed out quite yet - I'm still test-building everything
> else, so if you pull my tree and don't see the merge, wait a few
> minutes and it should be there).
> 
>                 Linus

[toc] | [prev] | [next] | [standalone]

#1364910

Why was the XTS tweak initialization changed in commit 0b81d0779072 ("fs crypto:
move per-file encryption from f2fs tree to fs/crypto")?

Old code:

       memcpy(xts_tweak, &index, sizeof(index));
       memset(&xts_tweak[sizeof(index)], 0,
                       F2FS_XTS_TWEAK_SIZE - sizeof(index));

New code:
       memcpy(xts_tweak, &inode->i_ino, sizeof(index));
       memset(&xts_tweak[sizeof(index)], 0,
                       FS_XTS_TWEAK_SIZE - sizeof(index));

Now the XTS tweak is the same for all pages of each inode.

[toc] | [prev] | [next] | [standalone]

#1364926

On Sat, Mar 26, 2016 at 01:10:07AM -0500, Eric Biggers wrote:
> Why was the XTS tweak initialization changed in commit 0b81d0779072 ("fs crypto:
> move per-file encryption from f2fs tree to fs/crypto")?
> 
> Old code:
> 
>        memcpy(xts_tweak, &index, sizeof(index));
>        memset(&xts_tweak[sizeof(index)], 0,
>                        F2FS_XTS_TWEAK_SIZE - sizeof(index));
> 
> New code:
>        memcpy(xts_tweak, &inode->i_ino, sizeof(index));
>        memset(&xts_tweak[sizeof(index)], 0,
>                        FS_XTS_TWEAK_SIZE - sizeof(index));
> 
> Now the XTS tweak is the same for all pages of each inode.

Thank you for catching this.
I've checked several times, but turns out I missed something tho. :(
Let me write a patch to fix this.

Thanks,

[toc] | [prev] | [next] | [standalone]

#1364947

It seems strange to me --- a "renaming" commit made IVs start getting reused,
weakening the encryption.  Do you have an explanation for how this change got
introduced?

Another question about the choice of IV.  If the page index in CPU order is
(supposed to be) used as the IV, doesn't make the on-disk format of the
filesystem endianness-dependent?  I thought that's a big no-no.

On Sat, Mar 26, 2016 at 01:56:06AM -0700, Jaegeuk Kim wrote:
> On Sat, Mar 26, 2016 at 01:10:07AM -0500, Eric Biggers wrote:
> > Why was the XTS tweak initialization changed in commit 0b81d0779072 ("fs crypto:
> > move per-file encryption from f2fs tree to fs/crypto")?
> > 
> > Old code:
> > 
> >        memcpy(xts_tweak, &index, sizeof(index));
> >        memset(&xts_tweak[sizeof(index)], 0,
> >                        F2FS_XTS_TWEAK_SIZE - sizeof(index));
> > 
> > New code:
> >        memcpy(xts_tweak, &inode->i_ino, sizeof(index));
> >        memset(&xts_tweak[sizeof(index)], 0,
> >                        FS_XTS_TWEAK_SIZE - sizeof(index));
> > 
> > Now the XTS tweak is the same for all pages of each inode.
> 
> Thank you for catching this.
> I've checked several times, but turns out I missed something tho. :(
> Let me write a patch to fix this.
> 
> Thanks,
> 
> 

[toc] | [prev] | [next] | [standalone]

#1364964

Good catch, Eric.

While waiting for explanations, I've committed the change to move back
to just the page index.

On Sat, Mar 26, 2016 at 6:47 AM, Eric Biggers <ebiggers3@gmail.com> wrote:
>
> Another question about the choice of IV.  If the page index in CPU order is
> (supposed to be) used as the IV, doesn't make the on-disk format of the
> filesystem endianness-dependent?  I thought that's a big no-no.

It is indeed a bad design choice, but at the same time I think it's
time to just admit that big-endian is largely dead, and people don't
necessarily need to worry about it too much. It makes it impossible to
move a disk image from x86 and ARM to Sparc or BE powerpc, but it's
not like anybody is actually going to do that. And PowerPC is moving
towards LE particularly in environments that have any kind of sharing
going on.

The code itself seems largely identical between ext4 and f2fs (which I
assume is the reason for trying to move it to a shared subdirectory),
and is fairly new. It was merged in both back in April of last year,
so I can't tell if one or the other is supposedly the "official" one,
or how much actual use the crypto extensions actually have yet. Adding
Michael Halcrow to the Cc for comments.

I do wonder if it would make sense to just _force_ the use of the
little-endian format, and make that memcpy be a

     *(__le64 *)xts_tweak = cpu_to_le64(pgoff);

instead. That would break any existing BE uses, but considering that
this code is fairly new, those may simply not exist.

I also wonder if the xts_tweak should perhaps have both the page
offset _and_ the inode number in it. Both ext4 and f2fs specify that
xts tweak size to be 16 bytes, and right now fill the last 8 bytes
with zero. Would it make sense to just put the inode number in there?
I didn't look at the actual key stuff - maybe the key is already
per-inode and it doesn't make any sense to add the inode info _again_,
but if the key is fs-wide, maybe it would be a good idea? With a mount
option so that you can keep any legacy filesystem images..

                   Linus

[toc] | [prev] | [next] | [standalone]

#1364999

On Sat, Mar 26, 2016 at 1:00 PM, Jaegeuk Kim <jaegeuk@kernel.org> wrote:
>
> After then, as Dave suggested, we needed to move the crypto libraries into
> common fs/crypto, so that other filesystems can support that in common sense.
>
> Please check the below threads about this history.
> http://www.spinics.net/lists/linux-fsdevel/msg93424.html

That's not the explanation we wanted.

The thing that needs explaining is the "index" change to
"inode->i_ino", which I undid.

How/why did that happen?

                 Linus

[toc] | [prev] | [next] | [standalone]

#1365003

On Sat, Mar 26, 2016 at 01:04:44PM -0700, Linus Torvalds wrote:
> On Sat, Mar 26, 2016 at 1:00 PM, Jaegeuk Kim <jaegeuk@kernel.org> wrote:
> >
> > After then, as Dave suggested, we needed to move the crypto libraries into
> > common fs/crypto, so that other filesystems can support that in common sense.
> >
> > Please check the below threads about this history.
> > http://www.spinics.net/lists/linux-fsdevel/msg93424.html
> 
> That's not the explanation we wanted.
> 
> The thing that needs explaining is the "index" change to
> "inode->i_ino", which I undid.

Oh, I made the original uplift patch a year ago and confused with the below
patch at that time.
I missed this, when writing new patches based on the old ones recently.

https://lkml.org/lkml/2015/5/11/1214

I made that patch to resolve the changed page index by fcollapse.
But, simply I noticed that encryption doesn't support fcollapse or finsert.

Thanks,

> 
> How/why did that happen?
> 
>                  Linus

[toc] | [prev] | [next] | [standalone]

#1365000

On Sat, Mar 26, 2016 at 10:53:52AM -0700, Linus Torvalds wrote:
> Good catch, Eric.
> 
> While waiting for explanations, I've committed the change to move back
> to just the page index.

Thank you.

+Ted

This was originally introduced in EXT4 for AOSP last year.
At that time, Ted suggested that it'd good to support it in F2FS as well for
AOSP. (It is now easy to build AOSP with F2FS anyway.)
So, I copied most of libraries from EXT4 and modified F2FS to activate that.

After then, as Dave suggested, we needed to move the crypto libraries into
common fs/crypto, so that other filesystems can support that in common sense.

Please check the below threads about this history.
http://www.spinics.net/lists/linux-fsdevel/msg93424.html

In my sense currently, I can see that there is still no product shipped with
this by default.
It seems manufacturers have been continuing to run a bunch of stress and
functional tests. It's up to google's decision though.

Thanks,

> 
> On Sat, Mar 26, 2016 at 6:47 AM, Eric Biggers <ebiggers3@gmail.com> wrote:
> >
> > Another question about the choice of IV.  If the page index in CPU order is
> > (supposed to be) used as the IV, doesn't make the on-disk format of the
> > filesystem endianness-dependent?  I thought that's a big no-no.
> 
> It is indeed a bad design choice, but at the same time I think it's
> time to just admit that big-endian is largely dead, and people don't
> necessarily need to worry about it too much. It makes it impossible to
> move a disk image from x86 and ARM to Sparc or BE powerpc, but it's
> not like anybody is actually going to do that. And PowerPC is moving
> towards LE particularly in environments that have any kind of sharing
> going on.
> 
> The code itself seems largely identical between ext4 and f2fs (which I
> assume is the reason for trying to move it to a shared subdirectory),
> and is fairly new. It was merged in both back in April of last year,
> so I can't tell if one or the other is supposedly the "official" one,
> or how much actual use the crypto extensions actually have yet. Adding
> Michael Halcrow to the Cc for comments.
> 
> I do wonder if it would make sense to just _force_ the use of the
> little-endian format, and make that memcpy be a
> 
>      *(__le64 *)xts_tweak = cpu_to_le64(pgoff);
> 
> instead. That would break any existing BE uses, but considering that
> this code is fairly new, those may simply not exist.
> 
> I also wonder if the xts_tweak should perhaps have both the page
> offset _and_ the inode number in it. Both ext4 and f2fs specify that
> xts tweak size to be 16 bytes, and right now fill the last 8 bytes
> with zero. Would it make sense to just put the inode number in there?
> I didn't look at the actual key stuff - maybe the key is already
> per-inode and it doesn't make any sense to add the inode info _again_,
> but if the key is fs-wide, maybe it would be a good idea? With a mount
> option so that you can keep any legacy filesystem images..
> 
>                    Linus

[toc] | [prev] | [next] | [standalone]

#1365006

On Sat, Mar 26, 2016 at 10:53:52AM -0700, Linus Torvalds wrote:
> On Sat, Mar 26, 2016 at 6:47 AM, Eric Biggers <ebiggers3@gmail.com> wrote:
> > Another question about the choice of IV.  If the page index in CPU order is
> > (supposed to be) used as the IV, doesn't make the on-disk format of the
> > filesystem endianness-dependent?  I thought that's a big no-no.
> 
> It is indeed a bad design choice, but at the same time I think it's
> time to just admit that big-endian is largely dead, and people don't
> necessarily need to worry about it too much.

Yes, this is a bug.  We should have used a CPU endianness-independent
IV.  Mea culpa for not noticing this.

> I do wonder if it would make sense to just _force_ the use of the
> little-endian format, and make that memcpy be a
> 
>      *(__le64 *)xts_tweak = cpu_to_le64(pgoff);
> 
> instead. That would break any existing BE uses, but considering that
> this code is fairly new, those may simply not exist.

Yes, that's probably the best way to go.

> The code itself seems largely identical between ext4 and f2fs (which I
> assume is the reason for trying to move it to a shared subdirectory),
> and is fairly new. It was merged in both back in April of last year,
> so I can't tell if one or the other is supposedly the "official" one,
> or how much actual use the crypto extensions actually have yet. Adding
> Michael Halcrow to the Cc for comments.

The ext4 code came first, but we've been working closely with the f2fs
folks to make sure they could reuse userspace and to eventually merge
everything into a shared subdirectory.  The main reason why we didn't
was because we were in a rush to try to beat the Android 'M'
deadlines.  Unfortunately, an ARM-specific bug caused us to have to
pull the feature from the Android Marshmellow release at the last
minute (xfstests hadn't been ported to the Android environment yet,
and so I could only do my testing on x86 and hope that translated to
ARM).  That being said, the AOSP release for M has working userspace
support for the file system crypto feature, and you can make it work
with either ext4 or f2fs encryption.

The pre-release e2fsprogs 1.43 has support for the encryption feature,
and the e4crypt binary should work for f2fs as well --- but that only
got released in Debian unstable last week.  (Single signon support via
integration for PAM is not yet done; there's enough in e4crypt for
testing purposes, but we've been more focused on userspace integration
for Android than for the generic Linux desktop.)

So while it's possible that there might be some super-aggressive early
adopters using Cyanogen or Debian unstable, it wouldn't be very many,
and it's almost certainly on little-endian and not on bigendian
architectures.  So preserving compatibility for LE platforms is the
right way to go.

> I also wonder if the xts_tweak should perhaps have both the page
> offset _and_ the inode number in it. Both ext4 and f2fs specify that
> xts tweak size to be 16 bytes, and right now fill the last 8 bytes
> with zero. Would it make sense to just put the inode number in there?
> I didn't look at the actual key stuff - maybe the key is already
> per-inode and it doesn't make any sense to add the inode info _again_,

Yes, the key is per-file.  The user's key is mixed with a per-inode
nonce to create a per-inode key.

BTW, as far as switching ext4 to use the shared code in fs/crypto ---
I'm hoping to get that done for the next merge window.  There are a 2
or 3 patches to fix some recently discovered bugs that I'll need to
push into the fs/crypto code, but I'll take care of that for the next
development cycle.

						- Ted

[toc] | [prev] | [next] | [standalone]

#1365008

On Sat, Mar 26, 2016 at 1:41 PM, Theodore Ts'o <tytso@mit.edu> wrote:
>
>> I also wonder if the xts_tweak should perhaps have both the page
>> offset _and_ the inode number in it. Both ext4 and f2fs specify that
>> xts tweak size to be 16 bytes, and right now fill the last 8 bytes
>> with zero. Would it make sense to just put the inode number in there?
>> I didn't look at the actual key stuff - maybe the key is already
>> per-inode and it doesn't make any sense to add the inode info _again_,
>
> Yes, the key is per-file.  The user's key is mixed with a per-inode
> nonce to create a per-inode key.

Ok, so the only thing that makes sense is really the page offset. Sounds fine.

> BTW, as far as switching ext4 to use the shared code in fs/crypto ---
> I'm hoping to get that done for the next merge window.  There are a 2
> or 3 patches to fix some recently discovered bugs that I'll need to
> push into the fs/crypto code, but I'll take care of that for the next
> development cycle.

No worries. Considering this mistake (which happily seems to have an
innocent explanation for it) it would be good to have it verified that
the shared code does actually work for you, so that we don't end up in
the unhappy situation that the code got split up in order to be
shared, but some random detail choice then made it not actually work
for ext4 after all..

So even if I wouldn't want to pull any ext4 updates to use the new
shared location this merge windoe (I'm making ready to close the merge
window asap), it would be good to know that what we will have for 4.6
is going to work for you, and we don't end up with somethign that
diverges between f2fs and ext4 after all..

                Linus

[toc] | [prev] | [next] | [standalone]

#1365009

On Sat, Mar 26, 2016 at 01:48:23PM -0700, Linus Torvalds wrote:
> > BTW, as far as switching ext4 to use the shared code in fs/crypto ---
> > I'm hoping to get that done for the next merge window.  There are a 2
> > or 3 patches to fix some recently discovered bugs that I'll need to
> > push into the fs/crypto code, but I'll take care of that for the next
> > development cycle.
> 
> No worries. Considering this mistake (which happily seems to have an
> innocent explanation for it) it would be good to have it verified that
> the shared code does actually work for you, so that we don't end up in
> the unhappy situation that the code got split up in order to be
> shared, but some random detail choice then made it not actually work
> for ext4 after all..

Oh, for sure.  One of the things I plan to do is to make sure that a
file system created using the *current* ext4 encryption code is
properly readable using the new fs/crypto code.  I'll also run a full
regression test run using xfstests as well.

That's one of the reasons why I wasn't going to even try to get it
done for this merge window (even if you weren't going to be closing
the merge window tomorrow).  I just didn't have the time to do a good
job, and it made sense to let the f2fs code go in first, and then
convert ext4 afterwards.

						- Ted

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web