[PATCH 0/2] Secure deletion under JFFS2

[PATCH 0/2] Secure deletion under JFFS2

[Theuns Verwoerd]

Security certifications such as FIPS require the capability to securely 
delete files, which is problematic under JFFS2's log-based model.  We can
ensure that all dirty or obsolete data has been cleared at the conclusion 
of secure deletion by forcibly driving the existing garbage collection 
system however.

To that end, these patches provide two pieces of functionality:
1. -POLL signal handling in jffs2_gc to force complete collection of 
   dirty blocks up to the time it was started.
2. Synchronisation with userspace (via debugfs files) to allow 
   secure deletion to wait until that process has completed.

Theuns Verwoerd (2):
  jffs2: Provide forced dirty node cleanup via POLL signal
  jffs2: Provide jffs2_sync files to track gc POLL progress

 fs/jffs2/Kconfig       |  8 ++++++++
 fs/jffs2/background.c  | 31 ++++++++++++++++++++++++++++++-
 fs/jffs2/build.c       |  1 +
 fs/jffs2/jffs2_fs_sb.h |  2 ++
 fs/jffs2/nodelist.h    |  1 +
 fs/jffs2/nodemgmt.c    |  6 +++++-
 fs/jffs2/super.c       | 40 ++++++++++++++++++++++++++++++++++++++++
 7 files changed, 87 insertions(+), 2 deletions(-)

-- 
2.18.0

[PATCH 1/2] jffs2: Provide forced dirty node cleanup via POLL signal

[Theuns Verwoerd]

Secure deletion under JFFS2 is complicated by the log of dirty nodes
that may contain obsoleted versions of sensitive data.  There is an
existing mechanism to trigger a single gc step via -HUP signal;
extend this to trigger gc collection of all currently-dirty data via
a -POLL signal.

On receipt of -POLL, the gc will retire the current nextblock, store
the last dirty list entry, and keep continuously cycling collection
until that entry has been cleaned.

Signed-off-by: Theuns Verwoerd <theuns.verwoerd@alliedtelesis.co.nz>
---
 fs/jffs2/background.c  | 31 ++++++++++++++++++++++++++++++-
 fs/jffs2/build.c       |  1 +
 fs/jffs2/jffs2_fs_sb.h |  2 ++
 fs/jffs2/nodelist.h    |  1 +
 fs/jffs2/nodemgmt.c    |  6 +++++-
 5 files changed, 39 insertions(+), 2 deletions(-)

diff --git a/fs/jffs2/background.c b/fs/jffs2/background.c
index 453a6a1fff34..4c29e2d323c4 100644
--- a/fs/jffs2/background.c
+++ b/fs/jffs2/background.c
@@ -72,15 +72,27 @@ void jffs2_stop_garbage_collect_thread(struct jffs2_sb_info *c)
 		wait_for_completion(&c->gc_thread_exit);
 }
 
+static int list_contains(struct list_head *list, struct list_head *entry)
+{
+	struct list_head *ptr;
+
+	list_for_each(ptr, list) {
+		if (ptr == entry)
+			return 1;
+	}
+	return 0;
+}
+
 static int jffs2_garbage_collect_thread(void *_c)
 {
 	struct jffs2_sb_info *c = _c;
 	sigset_t hupmask;
 
-	siginitset(&hupmask, sigmask(SIGHUP));
+	siginitset(&hupmask, sigmask(SIGHUP) | sigmask(SIGPOLL));
 	allow_signal(SIGKILL);
 	allow_signal(SIGSTOP);
 	allow_signal(SIGHUP);
+	allow_signal(SIGPOLL);
 
 	c->gc_task = current;
 	complete(&c->gc_thread_start);
@@ -143,6 +155,15 @@ static int jffs2_garbage_collect_thread(void *_c)
 				jffs2_dbg(1, "%s(): SIGHUP received\n",
 					  __func__);
 				break;
+			case SIGPOLL:
+				if (!c->tidemark) {
+					/* Force retire current half-used block */
+					if (c->nextblock)
+						jffs2_close_nextblock(c, c->nextblock);
+					/* Keep going until we hit the last element in the (now) current dirty list */
+					c->tidemark = c->dirty_list.prev;
+				}
+				break;
 			default:
 				jffs2_dbg(1, "%s(): signal %ld received\n",
 					  __func__, signr);
@@ -156,6 +177,14 @@ static int jffs2_garbage_collect_thread(void *_c)
 			pr_notice("No space for garbage collection. Aborting GC thread\n");
 			goto die;
 		}
+		/* If we're working towards a tidemark, keep going until it's clean */
+		if (c->tidemark && (
+			!list_empty(&c->very_dirty_list) ||
+			list_contains(&c->dirty_list, c->tidemark) ||
+			(&c->gcblock->list) == c->tidemark))
+			goto again;
+		else if (c->tidemark)
+			c->tidemark = NULL;
 	}
  die:
 	spin_lock(&c->erase_completion_lock);
diff --git a/fs/jffs2/build.c b/fs/jffs2/build.c
index b288c8ae1236..7d128705648f 100644
--- a/fs/jffs2/build.c
+++ b/fs/jffs2/build.c
@@ -405,6 +405,7 @@ int jffs2_do_mount_fs(struct jffs2_sb_info *c)
 	INIT_LIST_HEAD(&c->bad_used_list);
 	c->highest_ino = 1;
 	c->summary = NULL;
+	c->tidemark = NULL;
 
 	ret = jffs2_sum_init(c);
 	if (ret)
diff --git a/fs/jffs2/jffs2_fs_sb.h b/fs/jffs2/jffs2_fs_sb.h
index 778275f48a87..25960589e3c0 100644
--- a/fs/jffs2/jffs2_fs_sb.h
+++ b/fs/jffs2/jffs2_fs_sb.h
@@ -87,6 +87,8 @@ struct jffs2_sb_info {
 
 	uint32_t nospc_dirty_size;
 
+	struct list_head *tidemark;		/* Last dirty block at the time a full sync started */
+
 	uint32_t nr_blocks;
 	struct jffs2_eraseblock *blocks;	/* The whole array of blocks. Used for getting blocks
 						 * from the offset (blocks[ofs / sector_size]) */
diff --git a/fs/jffs2/nodelist.h b/fs/jffs2/nodelist.h
index 0637271f3770..73348bc7f545 100644
--- a/fs/jffs2/nodelist.h
+++ b/fs/jffs2/nodelist.h
@@ -386,6 +386,7 @@ int jffs2_reserve_space(struct jffs2_sb_info *c, uint32_t minsize,
 			uint32_t *len, int prio, uint32_t sumsize);
 int jffs2_reserve_space_gc(struct jffs2_sb_info *c, uint32_t minsize,
 			uint32_t *len, uint32_t sumsize);
+void jffs2_close_nextblock(struct jffs2_sb_info *c, struct jffs2_eraseblock *jeb);
 struct jffs2_raw_node_ref *jffs2_add_physical_node_ref(struct jffs2_sb_info *c, 
 						       uint32_t ofs, uint32_t len,
 						       struct jffs2_inode_cache *ic);
diff --git a/fs/jffs2/nodemgmt.c b/fs/jffs2/nodemgmt.c
index a7bbe879cfc3..c07c53f69516 100644
--- a/fs/jffs2/nodemgmt.c
+++ b/fs/jffs2/nodemgmt.c
@@ -240,7 +240,7 @@ int jffs2_reserve_space_gc(struct jffs2_sb_info *c, uint32_t minsize,
 
 /* Classify nextblock (clean, dirty of verydirty) and force to select an other one */
 
-static void jffs2_close_nextblock(struct jffs2_sb_info *c, struct jffs2_eraseblock *jeb)
+void jffs2_close_nextblock(struct jffs2_sb_info *c, struct jffs2_eraseblock *jeb)
 {
 
 	if (c->nextblock == NULL) {
@@ -875,6 +875,10 @@ int jffs2_thread_should_wake(struct jffs2_sb_info *c)
 		}
 	}
 
+	/* Pending cleanup, always wake */
+	if (c->tidemark)
+		ret = 1;
+
 	jffs2_dbg(1, "%s(): nr_free_blocks %d, nr_erasing_blocks %d, dirty_size 0x%x, vdirty_blocks %d: %s\n",
 		  __func__, c->nr_free_blocks, c->nr_erasing_blocks,
 		  c->dirty_size, nr_very_dirty, ret ? "yes" : "no");
-- 
2.18.0

[PATCH 2/2] jffs2: Provide jffs2_sync files to track gc POLL progress

[Theuns Verwoerd]

When executing secure deletion under JFFS2 via the -POLL forced cleanup
option, userspace may need to know when the process has safely completed.
Provide debugfs files per mount that, when read, blocks while cleanup
is in progress: once the read completes it is safe to assume that all
obsoleted sensitive information has been erased.

If CONFIG_JFFS2_FS_SYNC is configured, create
/sys/kernel/debug/jffs2_sync_<fs name> files that block on read
while forced gc is in progress.

Signed-off-by: Theuns Verwoerd <theuns.verwoerd@alliedtelesis.co.nz>
---
 fs/jffs2/Kconfig |  8 ++++++++
 fs/jffs2/super.c | 40 ++++++++++++++++++++++++++++++++++++++++
 2 files changed, 48 insertions(+)

diff --git a/fs/jffs2/Kconfig b/fs/jffs2/Kconfig
index ad850c5bf2ca..191272729f06 100644
--- a/fs/jffs2/Kconfig
+++ b/fs/jffs2/Kconfig
@@ -96,6 +96,14 @@ config JFFS2_FS_SECURITY
 	  If you are not using a security module that requires using
 	  extended attributes for file security labels, say N.
 
+config JFFS2_FS_SYNC
+	bool "JFFS2 jffs2_sync file support"
+	depends on JFFS2_FS
+	help
+	  This enables creation of jffs2_sync files for tracking
+	  POLL forced garbage collection.  It is used as part of FIPS
+	  secure deletion support.
+
 config JFFS2_COMPRESSION_OPTIONS
 	bool "Advanced compression options for JFFS2"
 	depends on JFFS2_FS
diff --git a/fs/jffs2/super.c b/fs/jffs2/super.c
index 87bdf0f4cba1..264f68ab2e91 100644
--- a/fs/jffs2/super.c
+++ b/fs/jffs2/super.c
@@ -27,6 +27,7 @@
 #include <linux/namei.h>
 #include <linux/seq_file.h>
 #include <linux/exportfs.h>
+#include <linux/debugfs.h>
 #include "compr.h"
 #include "nodelist.h"
 
@@ -34,6 +35,26 @@ static void jffs2_put_super(struct super_block *);
 
 static struct kmem_cache *jffs2_inode_cachep;
 
+#ifdef CONFIG_JFFS2_FS_SYNC
+#define JFFS2_FS_SYNC_NAME "jffs2_sync"
+static struct dentry *jffs2_debugfs_dir;
+
+ssize_t jffs2_sync_file_read(struct file *f,
+	      char __user *b, size_t len, loff_t *ofs)
+{
+	struct jffs2_sb_info *c = file_inode(f)->i_private;
+
+	while (c->tidemark)
+		schedule();
+
+	return 0;
+}
+const struct file_operations jffs2_sync_ops = {
+	.owner   = THIS_MODULE,
+	.read    = jffs2_sync_file_read,
+};
+#endif
+
 static struct inode *jffs2_alloc_inode(struct super_block *sb)
 {
 	struct jffs2_inode_info *f;
@@ -307,6 +328,17 @@ static int jffs2_fill_super(struct super_block *sb, void *data, int silent)
 	sb->s_flags |= SB_POSIXACL;
 #endif
 	ret = jffs2_do_fill_super(sb, data, silent);
+
+#ifdef CONFIG_JFFS2_FS_SYNC
+	if (jffs2_debugfs_dir) {
+		char fname[128];
+
+		snprintf(fname, sizeof(fname), "%s_%s",
+				JFFS2_FS_SYNC_NAME, sb->s_mtd->name);
+		debugfs_create_file(fname, 0444,
+				jffs2_debugfs_dir, c, &jffs2_sync_ops);
+	}
+#endif
 	return ret;
 }
 
@@ -405,6 +437,11 @@ static int __init init_jffs2_fs(void)
 		pr_err("error: Failed to register filesystem\n");
 		goto out_slab;
 	}
+
+#ifdef CONFIG_JFFS2_FS_SYNC
+	jffs2_debugfs_dir = debugfs_create_dir("jffs2", NULL);
+#endif
+
 	return 0;
 
  out_slab:
@@ -419,6 +456,9 @@ static int __init init_jffs2_fs(void)
 static void __exit exit_jffs2_fs(void)
 {
 	unregister_filesystem(&jffs2_fs_type);
+#ifdef CONFIG_JFFS2_FS_SYNC
+	debugfs_remove_recursive(jffs2_debugfs_dir);
+#endif
 	jffs2_destroy_slab_caches();
 	jffs2_compressors_exit();
 
-- 
2.18.0

Re: [PATCH 2/2] jffs2: Provide jffs2_sync files to track gc POLL progress

[Al Viro]

On Fri, Jul 20, 2018 at 11:50:12AM +1200, Theuns Verwoerd wrote:

> +ssize_t jffs2_sync_file_read(struct file *f,
> +	      char __user *b, size_t len, loff_t *ofs)
> +{
> +	struct jffs2_sb_info *c = file_inode(f)->i_private;
> +
> +	while (c->tidemark)
> +		schedule();
> +
> +	return 0;
> +}

Brilliant.  So when that gets called with c->tidemark being true and
need_resched() - false, we shall...

Bonus question: what happens if that is called after that jffs2_sb_info
gets freed?

--
It Doesn't Need To Make Sense - It's For Security Purposes.

Re: [PATCH 2/2] jffs2: Provide jffs2_sync files to track gc POLL progress

[Richard Weinberger]

[fixing CC list]

On Fri, Jul 20, 2018 at 2:54 AM, Theuns Verwoerd
<Theuns.Verwoerd@alliedtelesis.co.nz> wrote:
>
> On 07/20/2018 12:04 PM, Al Viro wrote:
>> On Fri, Jul 20, 2018 at 11:50:12AM +1200, Theuns Verwoerd wrote:
>>
>>> +ssize_t jffs2_sync_file_read(struct file *f,
>>> +          char __user *b, size_t len, loff_t *ofs)
>>> +{
>>> +    struct jffs2_sb_info *c = file_inode(f)->i_private;
>>> +
>>> +    while (c->tidemark)
>>> +            schedule();
>>> +
>>> +    return 0;
>>> +}
>> Brilliant.  So when that gets called with c->tidemark being true and
>> need_resched() - false, we shall...
> By my reading, schedule() will happily force a reschedule (once) when
> need_resched() is false.  Every time the process is granted a time
> slice, it'll check the condition and surrender if not ready.
>
> What do you believe will happen?
>> Bonus question: what happens if that is called after that jffs2_sb_info
>> gets freed?
> That is probably a valid critique - if jffs2_kill_sb() gets called the
> cleanup in exit_jffs2_fs() is not sufficient.

Beside of that, IMHO this debugfs file is a gross hack.
Did you look into the possibility to add the GC phase into the unlink code?

-- 
Thanks,
//richard

Re: [PATCH 2/2] jffs2: Provide jffs2_sync files to track gc POLL progress

[Al Viro]

On Sun, Jul 22, 2018 at 03:17:07PM +0200, Richard Weinberger wrote:

> On Fri, Jul 20, 2018 at 2:54 AM, Theuns Verwoerd
> <Theuns.Verwoerd@alliedtelesis.co.nz> wrote:
> >
> > On 07/20/2018 12:04 PM, Al Viro wrote:
> >> On Fri, Jul 20, 2018 at 11:50:12AM +1200, Theuns Verwoerd wrote:
> >>
> >>> +ssize_t jffs2_sync_file_read(struct file *f,
> >>> +          char __user *b, size_t len, loff_t *ofs)
> >>> +{
> >>> +    struct jffs2_sb_info *c = file_inode(f)->i_private;
> >>> +
> >>> +    while (c->tidemark)
> >>> +            schedule();
> >>> +
> >>> +    return 0;
> >>> +}
> >> Brilliant.  So when that gets called with c->tidemark being true and
> >> need_resched() - false, we shall...
> > By my reading, schedule() will happily force a reschedule (once) when
> > need_resched() is false.  Every time the process is granted a time
> > slice, it'll check the condition and surrender if not ready.

Except that the task remains runnable (and not using its timeslice up).
It won't sleep; there's nothing to drive it into TASK_NOT_RUNNING
state.

Try it and watch what's going on when you trigger that loop.  That's
really not a good way to wait for something to happen.

Re: [PATCH 2/2] jffs2: Provide jffs2_sync files to track gc POLL progress

[Theuns Verwoerd]

On 07/23/2018 01:17 AM, Richard Weinberger wrote:
> Beside of that, IMHO this debugfs file is a gross hack.
> Did you look into the possibility to add the GC phase into the unlink code?
I'm not terribly keen on it either, but it's the least bad option I 
could think of.  The main issue is that this kind of forced GC can be 
very slow (typically 2min on my test platform), and is not necessary for 
the vast majority of unlink cases - we only need to securely delete 
keys, other files can be cleaned up eventually. Unlink calls have no way 
of identifying such sensitive files, however, nor is there a way of 
demarcating a series of back-to-back secure deletions that can be 
collected en masse.  Parallel to the problem of triggering the forced 
GC, is the issue of knowing when it has completed: blocking on file read 
seemed to be the most digestible to userspace way of doing that.

Alternatives I've considered include overwrite-before-unlink (the block 
history gets complicated, plus the problem of actually doing such an 
overwrite), automatic GC on unlink (see above), an alternative system 
call for secure unlink (that messes up the entire FS call chain), and 
encrypting the sensitive files (key to access keys must also be 
persistent, which gets us back to the same problem).  In my original 
version I'd used a /proc file, but debugfs has a more forgiving social 
contract.

Regards,

Theuns
KRN

Re: [PATCH 0/2] Secure deletion under JFFS2

[Richard Weinberger]

On Fri, Jul 20, 2018 at 1:50 AM, Theuns Verwoerd
<theuns.verwoerd@alliedtelesis.co.nz> wrote:
> Security certifications such as FIPS require the capability to securely
> delete files, which is problematic under JFFS2's log-based model.  We can

Can you please be a little more specific about the certifications?

These days secure deletion at file system level is almost impossible to achieve
since you don't have full control of the storage stack.
I know, I know, In the raw flash case we have, but still. It makes
things very complicated.

A common approach do delete a file in a secure way is having it
encrypted and upon deletion
you forget the key.
Wouldn't that work for you too?

-- 
Thanks,
//richard

Re: [PATCH 0/2] Secure deletion under JFFS2

[Theuns Verwoerd]

On 07/23/2018 07:06 AM, Richard Weinberger wrote:
> On Fri, Jul 20, 2018 at 1:50 AM, Theuns Verwoerd
> <theuns.verwoerd@alliedtelesis.co.nz> wrote:
>> Security certifications such as FIPS require the capability to securely
>> delete files, which is problematic under JFFS2's log-based model.  We can
> Can you please be a little more specific about the certifications?
https://www.niap-ccevs.org/Documents_and_Guidance/view_td.cfm?td_id=133

gives some level of context.  I believe both FIPS and CC have similar 
expectations around key deletion.
> These days secure deletion at file system level is almost impossible to achieve
> since you don't have full control of the storage stack.
> I know, I know, In the raw flash case we have, but still. It makes
> things very complicated.
>
> A common approach do delete a file in a secure way is having it
> encrypted and upon deletion
> you forget the key.
> Wouldn't that work for you too?
To retain granularity for managing individual keys, you'd require a 1:1 
key-to-access-key (ktak).  Because keys are expected to be persistent, 
so must the ktak be - at which point we've replaced the requirement for 
securely deleting a key with one to securely delete a ktak.
(In addition, since this approach falls outside the specific language 
used in the certification guidance documents, it'd need to be justified 
in detail, which adds risk.)

Regards,

Theuns
KRN