Re: [PATCH v3] UBIFS: read crypto_lookup from datanodes to znodes

From: Artem Bityutskiy
Date: Sat May 19 2012 - 07:34:49 EST

Next message: Jeffrin Jose: "[PATCH 1/1] net:ipv6:fixed a trailing white space issue."
Previous message: tip-bot for Peter Zijlstra: "[tip:sched/numa] sched/numa: Implement numa balancer"
In reply to: Artem Bityutskiy: "Re: [PATCH v3] UBIFS: read crypto_lookup from datanodes to znodes"
Next in thread: Joel Reardon: "[patch] UBIFS: improved comment for data node's format history"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

On Sat, 2012-05-19 at 10:30 +0200, Joel Reardon wrote:
> Crypto_lookup values are now read from the data node header whenever a
> data node is read from flash and cached in the TNC. The value will be zero
> until it is read from flash. (If it is needed before it happens to be
> loaded, i.e., to delete a node, then it will be forced read.)
>
> It was tested by setting cryptolookup values for new datanodes by their
> lnum and offset on flash. This was later asserted in the TNC that they
> were either zero, or always equal. On mounting, the TNC crypto lookup
> values were confirmed to be zero and later, after reading the
> corresponding files, confirmed they were loaded and corresponded to the
> location on flash.
>
> Signed-off-by: Joel Reardon <reardonj@xxxxxxxxxxx>

I've pushed this patch with minor amendments to the "joel" branch, and
I've applied the renaming patch on top (it also change some logic a bit
to make it more compact) - if you do not like that patch - complain,
I'll remove it. Also I have a request below. Thanks!

> diff --git a/fs/ubifs/ubifs-media.h b/fs/ubifs/ubifs-media.h
> index 9ecbd37..e03adcf 100644
> --- a/fs/ubifs/ubifs-media.h
> +++ b/fs/ubifs/ubifs-media.h
> @@ -539,6 +539,7 @@ struct ubifs_dent_node {
> * struct ubifs_data_node - data node.
> * @ch: common header
> * @key: node key
> + * @crypto_lookup: the node's cryptographic key's position in the KSA
> * @size: uncompressed data size in bytes
> * @compr_type: compression type (%UBIFS_COMPR_NONE, %UBIFS_COMPR_LZO, etc)
> * @padding: reserved for future, zeroes
> @@ -550,7 +551,7 @@ struct ubifs_dent_node {
> struct ubifs_data_node {
> struct ubifs_ch ch;
> __u8 key[UBIFS_KEY_LEN];
> - __le64 padding0; /* Watch 'zero_data_node_unused()' if changing! */
> + __le64 crypto_lookup;
> __le32 size;
> __le16 compr_type;
> __u8 padding1[2]; /* Watch 'zero_data_node_unused()' if changing! */

This file is copied to user-space (mtd-utils.git) and I try to keep it
well-documented. Could you please extend 'struct ubifs_data_node' coment
and explain that older versions of UBIFS did not have secure deletion
support and the 'crypto_lookup' field contained all zeroes.

The patch I've applied on top:

From 87c912b858520939da9ef9e258b01a6fe85a342b Mon Sep 17 00:00:00 2001
From: Artem Bityutskiy <artem.bityutskiy@xxxxxxxxxxxxxxx>
Date: Sat, 19 May 2012 14:23:01 +0300
Subject: [PATCH] UBIFS: rename crypto_lookup

Rename 'crypto_lookup' to 'ksa_pos' in order to has shorter lines - the old
name is too long. Also makes code around assertions in the node reading
function a bit more compact.

Signed-off-by: Artem Bityutskiy <artem.bityutskiy@xxxxxxxxxxxxxxx>
---
fs/ubifs/gc.c | 6 +++---
fs/ubifs/journal.c | 4 ++--
fs/ubifs/tnc.c | 33 ++++++++++++++-------------------
fs/ubifs/tnc_misc.c | 10 ++++------
fs/ubifs/ubifs-media.h | 4 ++--
fs/ubifs/ubifs.h | 8 ++++----
6 files changed, 29 insertions(+), 36 deletions(-)

diff --git a/fs/ubifs/gc.c b/fs/ubifs/gc.c
index 4a21358..6c84af3 100644
--- a/fs/ubifs/gc.c
+++ b/fs/ubifs/gc.c
@@ -322,7 +322,7 @@ static int move_node(struct ubifs_info *c, struct ubifs_scan_leb *sleb,
struct ubifs_scan_node *snod, struct ubifs_wbuf *wbuf)
{
int err, new_lnum = wbuf->lnum, new_offs = wbuf->offs + wbuf->used;
- long long crypto_lookup = 0;
+ long long ksa_pos = 0;

cond_resched();
err = ubifs_wbuf_write_nolock(wbuf, snod->node, snod->len);
@@ -332,11 +332,11 @@ static int move_node(struct ubifs_info *c, struct ubifs_scan_leb *sleb,
if (key_type(c, &snod->key) == UBIFS_DATA_KEY) {
struct ubifs_data_node *dn = snod->node;

- crypto_lookup = le64_to_cpu(dn->crypto_lookup);
+ ksa_pos = le64_to_cpu(dn->ksa_pos);
}
err = ubifs_tnc_replace(c, &snod->key, sleb->lnum,
snod->offs, new_lnum, new_offs,
- snod->len, crypto_lookup);
+ snod->len, ksa_pos);
list_del(&snod->list);
kfree(snod);
return err;
diff --git a/fs/ubifs/journal.c b/fs/ubifs/journal.c
index bd69a30..d1c21d1 100644
--- a/fs/ubifs/journal.c
+++ b/fs/ubifs/journal.c
@@ -734,7 +734,7 @@ int ubifs_jnl_write_data(struct ubifs_info *c, const struct inode *inode,

dlen = UBIFS_DATA_NODE_SZ + out_len;
data->compr_type = cpu_to_le16(compr_type);
- data->crypto_lookup = cpu_to_le64(0);
+ data->ksa_pos = cpu_to_le64(0);

/* Make reservation before allocating sequence numbers */
err = make_reservation(c, DATAHD, dlen);
@@ -1227,7 +1227,7 @@ int ubifs_jnl_truncate(struct ubifs_info *c, const struct inode *inode,
if (dlen) {
sz = offs + UBIFS_INO_NODE_SZ + UBIFS_TRUN_NODE_SZ;
err = ubifs_tnc_add(c, &key, lnum, sz, dlen,
- le64_to_cpu(dn->crypto_lookup));
+ le64_to_cpu(dn->ksa_pos));
if (err)
goto out_ro;
}
diff --git a/fs/ubifs/tnc.c b/fs/ubifs/tnc.c
index b36d01c..d9de581 100644
--- a/fs/ubifs/tnc.c
+++ b/fs/ubifs/tnc.c
@@ -499,8 +499,8 @@ static int try_read_node(const struct ubifs_info *c, void *buf, int type,
*
* This function tries to read a node and returns %1 if the node is read, %0
* if the node is not present, and a negative error code in the case of error.
- * It sets @zbr's @crypto_lookup field to the value in the read node if it is
- * a data node.
+ * It sets @zbr's @ksa_pos field to the value in the read node if it is a data
+ * node.
*/
static int fallible_read_node(struct ubifs_info *c, const union ubifs_key *key,
struct ubifs_zbranch *zbr, void *node)
@@ -520,17 +520,12 @@ static int fallible_read_node(struct ubifs_info *c, const union ubifs_key *key,
if (keys_cmp(c, key, &node_key) != 0)
ret = 0;

- /* If it is actually a data node, read the @crypto_lookup */
+ /* If it is actually a data node, read the @ksa_pos */
if (key_type(c, key) == UBIFS_DATA_KEY) {
- long long crypto_lookup;
+ long long ksa_pos = le64_to_cpu(dn->ksa_pos);

- crypto_lookup = le64_to_cpu(dn->crypto_lookup);
- if (zbr->crypto_lookup != 0)
- ubifs_assert(zbr->crypto_lookup ==
- crypto_lookup);
- else
- zbr->crypto_lookup =
- crypto_lookup;
+ ubifs_assert(!zbr->ksa_pos || zbr->ksa_pos == ksa_pos);
+ zbr->ksa_pos = ksa_pos;
}

}
@@ -2176,14 +2171,14 @@ do_split:
* @lnum: LEB number of node
* @offs: node offset
* @len: node length
- * @crypto_lookup: the node's cryptographic key's position in the KSA
+ * @ksa_pos: the node's cryptographic key's position in the KSA
*
* This function adds a node with key @key to TNC. The node may be new or it may
* obsolete some existing one. Returns %0 on success or negative error code on
* failure.
*/
int ubifs_tnc_add(struct ubifs_info *c, const union ubifs_key *key, int lnum,
- int offs, int len, long long crypto_lookup)
+ int offs, int len, long long ksa_pos)
{
int found, n, err = 0;
struct ubifs_znode *znode;
@@ -2198,7 +2193,7 @@ int ubifs_tnc_add(struct ubifs_info *c, const union ubifs_key *key, int lnum,
zbr.lnum = lnum;
zbr.offs = offs;
zbr.len = len;
- zbr.crypto_lookup = crypto_lookup;
+ zbr.ksa_pos = ksa_pos;
key_copy(c, key, &zbr.key);
err = tnc_insert(c, znode, &zbr, n + 1);
} else if (found == 1) {
@@ -2209,7 +2204,7 @@ int ubifs_tnc_add(struct ubifs_info *c, const union ubifs_key *key, int lnum,
zbr->lnum = lnum;
zbr->offs = offs;
zbr->len = len;
- zbr->crypto_lookup = crypto_lookup;
+ zbr->ksa_pos = ksa_pos;
} else
err = found;
if (!err)
@@ -2228,7 +2223,7 @@ int ubifs_tnc_add(struct ubifs_info *c, const union ubifs_key *key, int lnum,
* @lnum: LEB number of node
* @offs: node offset
* @len: node length
- * @crypto_lookup: the node's updated cryptographic key's position in the KSA
+ * @ksa_pos: the node's updated cryptographic key's position in the KSA
*
* This function replaces a node with key @key in the TNC only if the old node
* is found. This function is called by garbage collection when node are moved.
@@ -2236,7 +2231,7 @@ int ubifs_tnc_add(struct ubifs_info *c, const union ubifs_key *key, int lnum,
*/
int ubifs_tnc_replace(struct ubifs_info *c, const union ubifs_key *key,
int old_lnum, int old_offs, int lnum, int offs, int len,
- long long crypto_lookup)
+ long long ksa_pos)
{
int found, n, err = 0;
struct ubifs_znode *znode;
@@ -2262,7 +2257,7 @@ int ubifs_tnc_replace(struct ubifs_info *c, const union ubifs_key *key,
zbr->lnum = lnum;
zbr->offs = offs;
zbr->len = len;
- zbr->crypto_lookup = crypto_lookup;
+ zbr->ksa_pos = ksa_pos;
found = 1;
} else if (is_hash_key(c, key)) {
found = resolve_collision_directly(c, key, &znode, &n,
@@ -2500,7 +2495,7 @@ static int tnc_delete(struct ubifs_info *c, struct ubifs_znode *znode, int n)
c->zroot.lnum = zbr->lnum;
c->zroot.offs = zbr->offs;
c->zroot.len = zbr->len;
- c->zroot.crypto_lookup = zbr->crypto_lookup;
+ c->zroot.ksa_pos = zbr->ksa_pos;
c->zroot.znode = znode;
ubifs_assert(!ubifs_zn_obsolete(zp));
ubifs_assert(ubifs_zn_dirty(zp));
diff --git a/fs/ubifs/tnc_misc.c b/fs/ubifs/tnc_misc.c
index aa8a51d..169c9f3 100644
--- a/fs/ubifs/tnc_misc.c
+++ b/fs/ubifs/tnc_misc.c
@@ -309,7 +309,7 @@ static int read_znode(struct ubifs_info *c, int lnum, int offs, int len,
zbr->lnum = le32_to_cpu(br->lnum);
zbr->offs = le32_to_cpu(br->offs);
zbr->len = le32_to_cpu(br->len);
- zbr->crypto_lookup = 0;
+ zbr->ksa_pos = 0;
zbr->znode = NULL;

/* Validate branch */
@@ -493,12 +493,10 @@ int ubifs_tnc_read_node(struct ubifs_info *c, struct ubifs_zbranch *zbr,

if (key_type(c, &(zbr->key)) == UBIFS_DATA_KEY) {
struct ubifs_data_node *dn = node;
- long long crypto_lookup = le64_to_cpu(dn->crypto_lookup);
+ long long ksa_pos = le64_to_cpu(dn->ksa_pos);

- if (zbr->crypto_lookup != 0)
- ubifs_assert(zbr->crypto_lookup == crypto_lookup);
- else
- zbr->crypto_lookup = crypto_lookup;
+ ubifs_assert(!zbr->ksa_pos || zbr->ksa_pos == ksa_pos);
+ zbr->ksa_pos = ksa_pos;
}

return 0;
diff --git a/fs/ubifs/ubifs-media.h b/fs/ubifs/ubifs-media.h
index e03adcf..55f2ccd 100644
--- a/fs/ubifs/ubifs-media.h
+++ b/fs/ubifs/ubifs-media.h
@@ -539,7 +539,7 @@ struct ubifs_dent_node {
* struct ubifs_data_node - data node.
* @ch: common header
* @key: node key
- * @crypto_lookup: the node's cryptographic key's position in the KSA
+ * @ksa_pos: the node's cryptographic key's position in the KSA
* @size: uncompressed data size in bytes
* @compr_type: compression type (%UBIFS_COMPR_NONE, %UBIFS_COMPR_LZO, etc)
* @padding: reserved for future, zeroes
@@ -551,7 +551,7 @@ struct ubifs_dent_node {
struct ubifs_data_node {
struct ubifs_ch ch;
__u8 key[UBIFS_KEY_LEN];
- __le64 crypto_lookup;
+ __le64 ksa_pos;
__le32 size;
__le16 compr_type;
__u8 padding1[2]; /* Watch 'zero_data_node_unused()' if changing! */
diff --git a/fs/ubifs/ubifs.h b/fs/ubifs/ubifs.h
index 4283a51..cf990e2 100644
--- a/fs/ubifs/ubifs.h
+++ b/fs/ubifs/ubifs.h
@@ -738,7 +738,7 @@ struct ubifs_jhead {
* @lnum: LEB number of the target node (indexing node or data node)
* @offs: target node offset within @lnum
* @len: target node length
- * @crypto_lookup: the node's cryptographic key's position in the KSA
+ * @ksa_pos: the node's cryptographic key's position in the KSA
*/
struct ubifs_zbranch {
union ubifs_key key;
@@ -749,7 +749,7 @@ struct ubifs_zbranch {
int lnum;
int offs;
int len;
- long long crypto_lookup;
+ long long ksa_pos;
};

/**
@@ -1577,10 +1577,10 @@ int ubifs_tnc_lookup_nm(struct ubifs_info *c, const union ubifs_key *key,
int ubifs_tnc_locate(struct ubifs_info *c, const union ubifs_key *key,
void *node, int *lnum, int *offs);
int ubifs_tnc_add(struct ubifs_info *c, const union ubifs_key *key, int lnum,
- int offs, int len, long long crypto_lookup);
+ int offs, int len, long long ksa_pos);
int ubifs_tnc_replace(struct ubifs_info *c, const union ubifs_key *key,
int old_lnum, int old_offs, int lnum, int offs, int len,
- long long crypto_lookup);
+ long long ksa_pos);
int ubifs_tnc_add_nm(struct ubifs_info *c, const union ubifs_key *key,
int lnum, int offs, int len, const struct qstr *nm);
int ubifs_tnc_remove(struct ubifs_info *c, const union ubifs_key *key);
--
1.7.7.6

--
Best Regards,
Artem Bityutskiy

Attachment: signature.asc
Description: This is a digitally signed message part

Next message: Jeffrin Jose: "[PATCH 1/1] net:ipv6:fixed a trailing white space issue."
Previous message: tip-bot for Peter Zijlstra: "[tip:sched/numa] sched/numa: Implement numa balancer"
In reply to: Artem Bityutskiy: "Re: [PATCH v3] UBIFS: read crypto_lookup from datanodes to znodes"
Next in thread: Joel Reardon: "[patch] UBIFS: improved comment for data node's format history"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]