[f2fs-dev] [PATCH 2/3] f2fs: support checkpoint error injection

2016-09-22 Thread Chao Yu 
This patch adds to support checkpoint error injection in f2fs for testing
fatal error tolerance.

Signed-off-by: Chao Yu 
---
 fs/f2fs/f2fs.h  | 7 +++
 fs/f2fs/super.c | 1 +
 2 files changed, 8 insertions(+)

diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index e216bc0..3c513fe 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -47,6 +47,7 @@ enum {
FAULT_DIR_DEPTH,
FAULT_EVICT_INODE,
FAULT_IO,
+   FAULT_CHECKPOINT,
FAULT_MAX,
 };
 
@@ -80,6 +81,8 @@ static inline bool time_to_inject(int type)
return false;
else if (type == FAULT_IO && !IS_FAULT_SET(type))
return false;
+   else if (type == FAULT_CHECKPOINT && !IS_FAULT_SET(type))
+   return false;
 
atomic_inc(_fault.inject_ops);
if (atomic_read(_fault.inject_ops) >= f2fs_fault.inject_rate) {
@@ -1873,6 +1876,10 @@ static inline int f2fs_readonly(struct super_block *sb)
 
 static inline bool f2fs_cp_error(struct f2fs_sb_info *sbi)
 {
+#ifdef CONFIG_F2FS_FAULT_INJECTION
+   if (time_to_inject(FAULT_CHECKPOINT))
+   return true;
+#endif
return is_set_ckpt_flags(sbi, CP_ERROR_FLAG);
 }
 
diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
index 6426855..3c49419 100644
--- a/fs/f2fs/super.c
+++ b/fs/f2fs/super.c
@@ -51,6 +51,7 @@ char *fault_name[FAULT_MAX] = {
[FAULT_DIR_DEPTH]   = "too big dir depth",
[FAULT_EVICT_INODE] = "evict_inode fail",
[FAULT_IO]  = "IO error",
+   [FAULT_CHECKPOINT]  = "checkpoint error",
 };
 
 static void f2fs_build_fault_attr(unsigned int rate)
-- 
2.8.2.311.gee88674


--
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

[f2fs-dev] [PATCH 3/3] f2fs: fix potential deadlock when hitting checkpoint error

2016-09-22 Thread Chao Yu 
tests/generic/013 of fstest suit complains us with below dmesg when we
trigger checkpoint error injection in f2fs.

F2FS-fs : inject checkpoint error in sync_node_pages+0x69f/0x6f0 [f2fs]
F2FS-fs (zram0): Cannot recover all fsync data errno=-5
INFO: task mount:97685 blocked for more than 120 seconds.
  Tainted: G   OE   4.8.0-rc4 #11
"echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
mount   D 8801c1bf7960 0 97685  97397 0x0008
 8801c1bf7960 8801c1bf7930 88017590 8801c1bf7980
 8801c1bf8000  7fff 88021f7be340
 817c8880 8801c1bf7978 817c80a5 880214f58fc0
Call Trace:
 [] ? bit_wait+0x50/0x50
 [] schedule+0x35/0x80
 [] schedule_timeout+0x292/0x3d0
 [] ? xen_clocksource_get_cycles+0x15/0x20
 [] ? ktime_get+0x3c/0xb0
 [] ? bit_wait+0x50/0x50
 [] io_schedule_timeout+0xa6/0x110
 [] bit_wait_io+0x1b/0x60
 [] __wait_on_bit+0x64/0x90
 [] wait_on_page_bit+0xc4/0xd0
 [] ? autoremove_wake_function+0x40/0x40
 [] truncate_inode_pages_range+0x409/0x840
 [] ? pcpu_free_area+0x13d/0x1a0
 [] ? wake_up_bit+0x25/0x30
 [] truncate_inode_pages_final+0x4c/0x60
 [] f2fs_evict_inode+0x48/0x390 [f2fs]
 [] evict+0xc7/0x1a0
 [] iput+0x197/0x200
 [] f2fs_fill_super+0xab2/0x1130 [f2fs]
 [] mount_bdev+0x184/0x1c0
 [] ? f2fs_commit_super+0x100/0x100 [f2fs]
 [] f2fs_mount+0x15/0x20 [f2fs]
 [] mount_fs+0x39/0x160
 [] vfs_kern_mount+0x67/0x110
 [] do_mount+0x1bb/0xc80
 [] SyS_mount+0x83/0xd0
 [] do_syscall_64+0x6e/0x170
 [] entry_SYSCALL64_slow_path+0x25/0x25

The reason is that after we commit at least one page into f2fs private
bio cache, if there occurs checkpoint error, we will lose the chance to
commit private bio, result in deadlock in f2fs_evict_inode when wait
that page being writebacked. So giving a chance to do committing in
sync_node_pages for fixing.

Signed-off-by: Chao Yu 
---
 fs/f2fs/node.c | 15 +++
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/fs/f2fs/node.c b/fs/f2fs/node.c
index 55c22a9..c2d953e 100644
--- a/fs/f2fs/node.c
+++ b/fs/f2fs/node.c
@@ -1416,6 +1416,7 @@ int sync_node_pages(struct f2fs_sb_info *sbi, struct 
writeback_control *wbc)
struct pagevec pvec;
int step = 0;
int nwritten = 0;
+   int ret = 0;
 
pagevec_init(, 0);
 
@@ -1436,7 +1437,8 @@ next_step:
 
if (unlikely(f2fs_cp_error(sbi))) {
pagevec_release();
-   return -EIO;
+   ret = -EIO;
+   goto out;
}
 
/*
@@ -1485,9 +1487,11 @@ continue_unlock:
set_fsync_mark(page, 0);
set_dentry_mark(page, 0);
 
-   if (NODE_MAPPING(sbi)->a_ops->writepage(page, wbc))
+   if (NODE_MAPPING(sbi)->a_ops->writepage(page, wbc)) {
unlock_page(page);
-
+   } else {
+   nwritten++;
+   }
if (--wbc->nr_to_write == 0)
break;
}
@@ -1504,7 +1508,10 @@ continue_unlock:
step++;
goto next_step;
}
-   return nwritten;
+out:
+   if (ret && nwritten)
+   f2fs_submit_merged_bio(sbi, NODE, WRITE);
+   return ret;
 }
 
 int wait_on_node_pages_writeback(struct f2fs_sb_info *sbi, nid_t ino)
-- 
2.8.2.311.gee88674


--
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

[f2fs-dev] [PATCH 1/3] f2fs: adjust display format of segment bit

2016-09-22 Thread Chao Yu 
Just adjust segment bit info printed in procfs.

Before:
1008  5|0  |0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 
0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0
1009  3|183|0 0 61 20 20 0 0 21 80 c0 2 e4 e 54 0 21 21 17 a 44 d0 28 e4 50 
40 30 8 0 2d 32 0 5 b0 80 1 43 2 8e f8 7b 2 25 93 bf e0 73 8e 9a 19 44 60 ff e4 
cc e6 8e bf f9 ff 5 3d 31 3d 13
1010  3|1  |0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 
0 0 0 0 0 0 0 0 0 0 40 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0

After:
1008  5|0  | 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
1009  4|434| ff 7d ff bf d9 3f ff e7 ff bf d7 bf ff bb be ff fb df f7 fb fa 
bf fb fe bb df dd ff fe ef ff fe ef e2 27 bf ab bf fb df fd bd bf fb db fc ff 
ff 3f ff ff bf ff 5f db 3f fb fb bf fb bf 4f ff ef
1010  4|422| ff bb fe ff ef d7 ee ff ff fc bf ef 7d eb ec fd fb 3f 97 7f ef 
ff af ff db ff ff 69 bf ff f6 e7 ff fb f7 7b fb df be ff ff ef f3 fe ff ff df 
fe f7 fa ff b7 77 be fe fb a9 7f 87 a2 ac c7 ff 75

Signed-off-by: Chao Yu 
---
 fs/f2fs/super.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
index e7bb153..6426855 100644
--- a/fs/f2fs/super.c
+++ b/fs/f2fs/super.c
@@ -954,7 +954,7 @@ static int segment_bits_seq_show(struct seq_file *seq, void 
*offset)
seq_printf(seq, "%d|%-3u|", se->type,
get_valid_blocks(sbi, i, 1));
for (j = 0; j < SIT_VBLOCK_MAP_SIZE; j++)
-   seq_printf(seq, "%x ", se->cur_valid_map[j]);
+   seq_printf(seq, " %.2x", se->cur_valid_map[j]);
seq_putc(seq, '\n');
}
return 0;
-- 
2.8.2.311.gee88674


--
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

Re: [f2fs-dev] ext4, f2fs: fscrypt_has_permitted_context() check in file open

2016-09-22 Thread Richard Weinberger 
Ted,

On 22.09.2016 15:44, Theodore Ts'o wrote:
> On Thu, Sep 22, 2016 at 02:24:35PM +0200, Richard Weinberger wrote:
>> Why do we need this check? AFAIK this situation can never happen unless due 
>> to
>> a bug in the filesystem code.
> 
> Or in the case of a malicious attacker who is trying to achieve an
> off-line attack on your file system  applications aren't going to
> be checking to see if they are writing their file with encryption
> enabled (and with the correct key), because they will largely be
> encryption oblivious.
> 
> So imagine a case where you have a file, say, dissidents.txt.  This
> file is encrypted, and is in a encrypted directory.  The bad guy, in
> an offline attack (e.g., using a tool like debugfs), creates a
> replacement directory which is unencrypted, and creates a link to the
> encrypted dissidents.txt file to that replacement directory.
> 
> You then go back to your hotel room in Beijing, boot your laptop, fire
> up your editor, and then edit the dissidents.txt file.  You have the
> keys, so it is read in just fine into vi or emacs.  But when when you
> write out the file, the editor writes the file into
> dissidents.txt.new, calls fsync on it, and then renames dissidents.txt
> to dissidents.txt~, and renames dissidents.txt.new to dissidents.txt.
> But since it is now in an unencrypted directory, dissidents.txt is now
> unencrypted.

Got it. So, the use case is preventing off-line attacks.
But I fear this is only a drop in the bucket. What we really need is
meta data authentication.

Thanks,
//richard

--
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

Re: [f2fs-dev] [PATCH] mkfs.f2fs: do not need strdup for parse_feature

2016-09-22 Thread Chao Yu 
On 2016/9/21 18:09, Yunlong Song wrote:
> strdup is useless here, with no free op with its return value.
> 
> Signed-off-by: Yunlong Song 

Acked-by: Chao Yu 


--
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

[f2fs-dev] fscrypt: in-place decrypt vs. out-of-place encrypt?

2016-09-22 Thread Richard Weinberger 
Hi!

While reading the fscrypt code I noticed that some functions use the bounce 
pages
and some not.
fscrypt_decrypt_page() and fscrypt_decrypt_bio_pages() work in-place while
fscrypt_encrypt_page() and fscrypt_zeroout_range() use a bounce page.

So, both ext4 and f2fs encrypt data using an extra buffer but decrypt mostly
in-place without the need of an extra buffer.
Why that? I'd assume when decryption can be done in-place also encryption is 
possible
that way.

I'm working on fscrypt for UBIFS and would like to avoid an extra buffer since 
memory
is low on embedded systems.

Thanks,
//richard

--
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

Re: [f2fs-dev] [PATCH 1/4] f2fs: assign return value in f2fs_gc

2016-09-22 Thread Chao Yu 
On 2016/9/22 11:54, Jaegeuk Kim wrote:
> This patch adds a return value of write_checkpoint for f2fs_gc.
> 
> Signed-off-by: Jaegeuk Kim 

Please add this in all patches of this serials.

Reviewed-by: Chao Yu 


--
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

Re: [f2fs-dev] [PATCH] f2fs: No need to wait for data page writeback by force

2016-09-22 Thread Chao Yu 
Hi Yunlei,

On 2016/9/21 18:41, Yunlei He wrote:
> No need to wait for data page writeback, wait or not
> can be decided by device.
> 
> Signed-off-by: Yunlei He 
> ---
>  fs/f2fs/segment.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
> index e78501c..71e4a0f 100644
> --- a/fs/f2fs/segment.c
> +++ b/fs/f2fs/segment.c
> @@ -1658,7 +1658,7 @@ void f2fs_wait_on_encrypted_page_writeback(struct 
> f2fs_sb_info *sbi,
>  
>   cpage = find_lock_page(META_MAPPING(sbi), blkaddr);
>   if (cpage) {
> - f2fs_wait_on_page_writeback(cpage, DATA, true);
> + f2fs_wait_on_page_writeback(cpage, DATA, false);

You can see why we need to wait page writebacked in this commit 08b39fbd5978
("f2fs crypto: fix racing of accessing encrypted page among"), here, I think we
should not let that page slip during writebacking even driver supports accessing
writebacking page.

Thanks,

>   f2fs_put_page(cpage, 1);
>   }
>  }
> 


--
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

Re: [f2fs-dev] [PATCH] f2fs: continue to do fg_gc even if beyond max_victim_search

2016-09-22 Thread Chao Yu 
On 2016/9/22 11:48, Yunlei He wrote:
> For forground gc, if can't find a victim in one max_victim_search,
> try it again.
> 
> Signed-off-by: Yunlei He 
> ---
>  fs/f2fs/gc.c | 23 ---
>  1 file changed, 12 insertions(+), 11 deletions(-)
> 
> diff --git a/fs/f2fs/gc.c b/fs/f2fs/gc.c
> index 24acbbb..19e51a5 100644
> --- a/fs/f2fs/gc.c
> +++ b/fs/f2fs/gc.c
> @@ -916,24 +916,25 @@ gc_more:
>   goto stop;
>   }
>  
> - if (gc_type == BG_GC && has_not_enough_free_secs(sbi, sec_freed, 0)) {
> - gc_type = FG_GC;
> + if (gc_type == BG_GC && has_not_enough_free_secs(sbi, 0, 0)) {
>   /*
>* If there is no victim and no prefree segment but still not
>* enough free sections, we should flush dent/node blocks and do
>* garbage collections.
>*/
> - if (__get_victim(sbi, , gc_type) ||
> - prefree_segments(sbi)) {
> - write_checkpoint(sbi, );
> - segno = NULL_SEGNO;
> - } else if (has_not_enough_free_secs(sbi, 0, 0)) {
> - write_checkpoint(sbi, );
> - }
> + gc_type = FG_GC;
> + write_checkpoint(sbi, );
>   }
>  
> - if (segno == NULL_SEGNO && !__get_victim(sbi, , gc_type))
> - goto stop;
> + if (!__get_victim(sbi, , gc_type)) {
> + if (sync)
> + goto stop;
> + else if (gc_type == FG_GC
> + && has_not_enough_free_secs(sbi, sec_freed, 0))
> + goto gc_more;

Will we deadloop here?

Thanks,

> + else
> + goto stop;
> + }
>   ret = 0;
>  
>   if (do_garbage_collect(sbi, segno, _list, gc_type) &&
> 


--
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

Re: [f2fs-dev] [PATCH] fsck.f2fs: free node_blk to avoid memory leak

2016-09-22 Thread Chao Yu 
On 2016/9/21 21:01, Yunlong Song wrote:
> Signed-off-by: Yunlong Song 

Acked-by: Chao Yu 


--
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

[f2fs-dev] ext4, f2fs: fscrypt_has_permitted_context() check in file open

2016-09-22 Thread Richard Weinberger 
Hi!

Both ext4 and f2fs check in the file open code the context of the parent 
directory too:

ext4:
if (ext4_encrypted_inode(d_inode(dir)) &&
!fscrypt_has_permitted_context(d_inode(dir), inode)) {
ext4_warning(inode->i_sb,
 "Inconsistent encryption contexts: %lu/%lu",
 (unsigned long) d_inode(dir)->i_ino,
 (unsigned long) inode->i_ino);
dput(dir);
return -EPERM;
}

f2fs:
if (f2fs_encrypted_inode(d_inode(dir)) &&
!fscrypt_has_permitted_context(d_inode(dir), inode)) {
dput(dir);
return -EPERM;
}

Why do we need this check? AFAIK this situation can never happen unless due to
a bug in the filesystem code.

Thanks,
//richard

--
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

Re: [f2fs-dev] ext4, f2fs: fscrypt_has_permitted_context() check in file open

2016-09-22 Thread Theodore Ts'o 
On Thu, Sep 22, 2016 at 04:21:30PM +0200, Richard Weinberger wrote:
> 
> Got it. So, the use case is preventing off-line attacks.
> But I fear this is only a drop in the bucket. What we really need is
> meta data authentication.

True security requires a system-wide design, sure.  For example, you
might want a locked bootloader that will only boot signed kernels.
The kernel might then require to use a read-only root file system with
dm-verity to make sure the system software can't be trojan'ed.  And
then you want the system software to enforce that the top-level
directories which contain encrypted information are protected using
the correct keys, perhaps using some trusted hardware store where the
user's keys are stored (and only released when the proper password /
pin is given).

Given all of those induction steps, *then* the file system level
checks that require that all subdirectories and files in an encrypted
directories must be encrypted using the same key as their parent will
provide the security you need.

Cheers,

- Ted

--
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

[f2fs-dev] [PATCH] f2fs: do not unnecessarily null-terminate encrypted symlink data

2016-09-22 Thread Eric Biggers 
Null-terminating the fscrypt_symlink_data on read is unnecessary because
it is not string data --- it contains binary ciphertext.

Signed-off-by: Eric Biggers 
---
 fs/f2fs/namei.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/fs/f2fs/namei.c b/fs/f2fs/namei.c
index afd5633..5b4733e 100644
--- a/fs/f2fs/namei.c
+++ b/fs/f2fs/namei.c
@@ -1010,7 +1010,6 @@ static const char *f2fs_encrypted_get_link(struct dentry 
*dentry,
struct fscrypt_str cstr = FSTR_INIT(NULL, 0);
struct fscrypt_str pstr = FSTR_INIT(NULL, 0);
struct fscrypt_symlink_data *sd;
-   loff_t size = min_t(loff_t, i_size_read(inode), PAGE_SIZE - 1);
u32 max_size = inode->i_sb->s_blocksize;
int res;
 
@@ -1025,7 +1024,6 @@ static const char *f2fs_encrypted_get_link(struct dentry 
*dentry,
if (IS_ERR(cpage))
return ERR_CAST(cpage);
caddr = page_address(cpage);
-   caddr[size] = 0;
 
/* Symlink is encrypted */
sd = (struct fscrypt_symlink_data *)caddr;
-- 
2.8.0.rc3.226.g39d4020


--
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

[f2fs-dev] [PATCH] ext4: do not unnecessarily null-terminate encrypted symlink data

2016-09-22 Thread Eric Biggers 
Null-terminating the fscrypt_symlink_data on read is unnecessary because
it is not string data --- it contains binary ciphertext.

Signed-off-by: Eric Biggers 
---
 fs/ext4/symlink.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/fs/ext4/symlink.c b/fs/ext4/symlink.c
index 0a26cbd..fdf1c61 100644
--- a/fs/ext4/symlink.c
+++ b/fs/ext4/symlink.c
@@ -30,7 +30,6 @@ static const char *ext4_encrypted_get_link(struct dentry 
*dentry,
char *caddr, *paddr = NULL;
struct fscrypt_str cstr, pstr;
struct fscrypt_symlink_data *sd;
-   loff_t size = min_t(loff_t, i_size_read(inode), PAGE_SIZE - 1);
int res;
u32 max_size = inode->i_sb->s_blocksize;
 
@@ -49,7 +48,6 @@ static const char *ext4_encrypted_get_link(struct dentry 
*dentry,
if (IS_ERR(cpage))
return ERR_CAST(cpage);
caddr = page_address(cpage);
-   caddr[size] = 0;
}
 
/* Symlink is encrypted */
-- 
2.8.0.rc3.226.g39d4020


--
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

Re: [f2fs-dev] fscrypt: in-place decrypt vs. out-of-place encrypt?

2016-09-22 Thread Richard Weinberger 
Ted,

On 22.09.2016 15:14, Theodore Ts'o wrote:
> On Thu, Sep 22, 2016 at 10:49:41AM +0200, Richard Weinberger wrote:
>>
>> While reading the fscrypt code I noticed that some functions use the bounce 
>> pages
>> and some not.
>> fscrypt_decrypt_page() and fscrypt_decrypt_bio_pages() work in-place while
>> fscrypt_encrypt_page() and fscrypt_zeroout_range() use a bounce page.
>>
>> So, both ext4 and f2fs encrypt data using an extra buffer but decrypt mostly
>> in-place without the need of an extra buffer.
>> Why that? I'd assume when decryption can be done in-place also encryption is 
>> possible
>> that way.
> 
> The problem is that the page may be mapped into some process's (or
> multiple processes') address space.  The only way we can encrypt in
> place is if we make sure that it is not mapped into any processes'
> page tables, and prohibit it from being mmapped, *and* prevent any
> simultaneous reads from accessing the page for which we are in the
> process of destroying its original contents because we are doing an
> in-plance encrypt.
> 
> So it could be done, but you would have to remove the page from the
> page cache first (meaning making sure it's not in any page tables) so
> it can't be found by subsequent reads and mmaps, and then sure you
> have a lock out which prevents anyone else from trying to read or mmap
> the page(s) in question, since the version on the storage device is
> out-of-date, because it hasn't been written yet.
> 
> So basically, it's a mess, and if you do need the page again after
> it's been written, the fact that you have to read it back in from the
> storage device would also be a performance hit in some cases.

For UBIFS the story is a bit different. UBIFS operates on a kmap()'ed
page and has already a temporary buffer where it prepares and compresses data.
So, I can use in-place encryption. All I have to do is massaging the fscrypt
interface to allow the filesystem to select when a bouncing page (or buffer in
my case) is needed or not.
I've extended the fscrypt API already to work on buffers too.
In MTD world we don't have struct bio and other fancy stuff. :-)

Thanks,
//richard

--
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

Re: [f2fs-dev] fscrypt: in-place decrypt vs. out-of-place encrypt?

2016-09-22 Thread Theodore Ts'o 
On Thu, Sep 22, 2016 at 10:49:41AM +0200, Richard Weinberger wrote:
> 
> While reading the fscrypt code I noticed that some functions use the bounce 
> pages
> and some not.
> fscrypt_decrypt_page() and fscrypt_decrypt_bio_pages() work in-place while
> fscrypt_encrypt_page() and fscrypt_zeroout_range() use a bounce page.
> 
> So, both ext4 and f2fs encrypt data using an extra buffer but decrypt mostly
> in-place without the need of an extra buffer.
> Why that? I'd assume when decryption can be done in-place also encryption is 
> possible
> that way.

The problem is that the page may be mapped into some process's (or
multiple processes') address space.  The only way we can encrypt in
place is if we make sure that it is not mapped into any processes'
page tables, and prohibit it from being mmapped, *and* prevent any
simultaneous reads from accessing the page for which we are in the
process of destroying its original contents because we are doing an
in-plance encrypt.

So it could be done, but you would have to remove the page from the
page cache first (meaning making sure it's not in any page tables) so
it can't be found by subsequent reads and mmaps, and then sure you
have a lock out which prevents anyone else from trying to read or mmap
the page(s) in question, since the version on the storage device is
out-of-date, because it hasn't been written yet.

So basically, it's a mess, and if you do need the page again after
it's been written, the fact that you have to read it back in from the
storage device would also be a performance hit in some cases.

> I'm working on fscrypt for UBIFS and would like to avoid an extra
> buffer since memory is low on embedded systems.

The amount of memory needed for the bounce pages is relative to the
speed of your storage device.  It should be possible throttle how much
memory gets used based on how many I/O's you allow to be in flight at
any one time, and hence, how much memory is needed for bounce buffers.

In the long run, I suspect that the real answer for embedded devices
will be hardware support in the form of in-line encryption engines,
since embedded devices tend to have pathetically slow CPU support for
encryption anyway.  It also solves the problem where you have an
extremely unbalanced system (i.e., expensive PCIe attached flash that
can do thousands of IOPS per second, and a tiny wee bit of memory :-)

 - Ted

--
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

Re: [f2fs-dev] ext4, f2fs: fscrypt_has_permitted_context() check in file open

2016-09-22 Thread Theodore Ts'o 
On Thu, Sep 22, 2016 at 02:24:35PM +0200, Richard Weinberger wrote:
> Hi!
> 
> Both ext4 and f2fs check in the file open code the context of the parent 
> directory too:
> 
> ext4:
> if (ext4_encrypted_inode(d_inode(dir)) &&
> !fscrypt_has_permitted_context(d_inode(dir), inode)) {
> ext4_warning(inode->i_sb,
>  "Inconsistent encryption contexts: %lu/%lu",
>  (unsigned long) d_inode(dir)->i_ino,
>  (unsigned long) inode->i_ino);
> dput(dir);
> return -EPERM;
> }
> 
> f2fs:
> if (f2fs_encrypted_inode(d_inode(dir)) &&
> !fscrypt_has_permitted_context(d_inode(dir), inode)) {
> dput(dir);
> return -EPERM;
> }
> 
> Why do we need this check? AFAIK this situation can never happen unless due to
> a bug in the filesystem code.

Or in the case of a malicious attacker who is trying to achieve an
off-line attack on your file system  applications aren't going to
be checking to see if they are writing their file with encryption
enabled (and with the correct key), because they will largely be
encryption oblivious.

So imagine a case where you have a file, say, dissidents.txt.  This
file is encrypted, and is in a encrypted directory.  The bad guy, in
an offline attack (e.g., using a tool like debugfs), creates a
replacement directory which is unencrypted, and creates a link to the
encrypted dissidents.txt file to that replacement directory.

You then go back to your hotel room in Beijing, boot your laptop, fire
up your editor, and then edit the dissidents.txt file.  You have the
keys, so it is read in just fine into vi or emacs.  But when when you
write out the file, the editor writes the file into
dissidents.txt.new, calls fsync on it, and then renames dissidents.txt
to dissidents.txt~, and renames dissidents.txt.new to dissidents.txt.
But since it is now in an unencrypted directory, dissidents.txt is now
unencrypted.

You then leave the hotel room, and the MSS agent goes back to your
room, and completes the exfiltration of dissidents.txt.

Cheers,

- Ted

P.S.  If you're from China, replace MSS with FBI, and Beijing with
Washington, D.C.  :-)   The principle is the same in either case.

--
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel