Crypto_lookup values are now oppertunistically 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 <[email protected]>
---
fs/ubifs/gc.c | 8 +++++++-
fs/ubifs/journal.c | 9 ++++++---
fs/ubifs/tnc.c | 40 ++++++++++++++++++++++++++++++++++++++++
fs/ubifs/ubifs-media.h | 3 ++-
4 files changed, 55 insertions(+), 5 deletions(-)
diff --git a/fs/ubifs/gc.c b/fs/ubifs/gc.c
index f146447..16df2f4 100644
--- a/fs/ubifs/gc.c
+++ b/fs/ubifs/gc.c
@@ -322,15 +322,21 @@ 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;
cond_resched();
err = ubifs_wbuf_write_nolock(wbuf, snod->node, snod->len);
if (err)
return err;
+ if (key_type(c, &snod->key) == UBIFS_DATA_KEY) {
+ struct ubifs_data_node *dn =
+ (struct ubifs_data_node *) snod->node;
+ crypto_lookup = le64_to_cpu(dn->crypto_lookup);
+ }
err = ubifs_tnc_replace(c, &snod->key, sleb->lnum,
snod->offs, new_lnum, new_offs,
- snod->len, 0);
+ snod->len, crypto_lookup);
list_del(&snod->list);
kfree(snod);
return err;
diff --git a/fs/ubifs/journal.c b/fs/ubifs/journal.c
index 2072b9a..3f7aa05 100644
--- a/fs/ubifs/journal.c
+++ b/fs/ubifs/journal.c
@@ -89,7 +89,6 @@ static inline void zero_dent_node_unused(struct ubifs_dent_node *dent)
*/
static inline void zero_data_node_unused(struct ubifs_data_node *data)
{
- data->padding0 = 0;
memset(data->padding1, 0, 2);
}
@@ -735,6 +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);
/* Make reservation before allocating sequence numbers */
err = make_reservation(c, DATAHD, dlen);
@@ -742,12 +742,14 @@ int ubifs_jnl_write_data(struct ubifs_info *c, const struct inode *inode,
goto out_free;
err = write_node(c, DATAHD, data, dlen, &lnum, &offs);
+
if (err)
goto out_release;
ubifs_wbuf_add_ino_nolock(&c->jheads[DATAHD].wbuf, key_inum(c, key));
release_head(c, DATAHD);
- err = ubifs_tnc_add(c, key, lnum, offs, dlen, 0);
+ err = ubifs_tnc_add(c, key, lnum, offs, dlen,
+ le64_to_cpu(data->crypto_lookup));
if (err)
goto out_ro;
@@ -1226,7 +1228,8 @@ 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, 0);
+ err = ubifs_tnc_add(c, &key, lnum, sz, dlen,
+ le64_to_cpu(dn->crypto_lookup));
if (err)
goto out_ro;
}
diff --git a/fs/ubifs/tnc.c b/fs/ubifs/tnc.c
index b222a72..ac0d03c 100644
--- a/fs/ubifs/tnc.c
+++ b/fs/ubifs/tnc.c
@@ -1431,6 +1431,35 @@ static int maybe_leb_gced(struct ubifs_info *c, int lnum, int gc_seq1)
}
/**
+ * tnc_set_crypto_lookup - set the crypto_lookup field in a zbranch.
+ * @c: UBIFS file-system description object
+ * @zbr: the crypto_lookup field is set in this zbranch
+ * @node: a pointer to a node containing the zbranch.
+ *
+ * Crypto_lookup values are stored on flash memory inside the data node.
+ * When the system is running, they should be oppertunistically stored in
+ * memory inside the zbranch. This function is called whenever a node
+ * is read from flash memory. If @node is a data node (therefore containing a
+ * @crypto_lookup field), then:
+ * if @zbr has an unset @crypto_lookup, set it to the value from @node
+ * if @zbr has already a @crypto_lookup, assert they are the same.
+ */
+static void tnc_set_crypto_lookup(struct ubifs_info *c,
+ struct ubifs_zbranch *zbr, void *node)
+{
+ if (key_type(c, &(zbr->key)) == UBIFS_DATA_KEY) {
+ struct ubifs_data_node *dn =
+ (struct ubifs_data_node *) node;
+ if (zbr->crypto_lookup != 0) {
+ ubifs_assert(zbr->crypto_lookup ==
+ le64_to_cpu(dn->crypto_lookup));
+ } else {
+ zbr->crypto_lookup = le64_to_cpu(dn->crypto_lookup);
+ }
+ }
+}
+
+/**
* ubifs_tnc_locate - look up a file-system node and return it and its location.
* @c: UBIFS file-system description object
* @key: node key to lookup
@@ -1485,6 +1514,11 @@ again:
if (ubifs_get_wbuf(c, zbr.lnum)) {
/* We do not GC journal heads */
err = ubifs_tnc_read_node(c, &zbr, node);
+ if (!err) {
+ mutex_lock(&c->tnc_mutex);
+ tnc_set_crypto_lookup(c, &(znode->zbranch[n]), node);
+ mutex_unlock(&c->tnc_mutex);
+ }
return err;
}
@@ -1497,9 +1531,15 @@ again:
safely = 1;
goto again;
}
+
+ mutex_lock(&c->tnc_mutex);
+ tnc_set_crypto_lookup(c, &(znode->zbranch[n]), node);
+ mutex_unlock(&c->tnc_mutex);
return 0;
out:
+ if (!err)
+ tnc_set_crypto_lookup(c, zt, node);
mutex_unlock(&c->tnc_mutex);
return err;
}
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! */
--
1.7.5.4
On Tue, 2012-05-15 at 11:01 +0200, Joel Reardon wrote:
> Crypto_lookup values are now oppertunistically 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 <[email protected]>
> ---
> fs/ubifs/gc.c | 8 +++++++-
> fs/ubifs/journal.c | 9 ++++++---
> fs/ubifs/tnc.c | 40 ++++++++++++++++++++++++++++++++++++++++
> fs/ubifs/ubifs-media.h | 3 ++-
> 4 files changed, 55 insertions(+), 5 deletions(-)
>
> diff --git a/fs/ubifs/gc.c b/fs/ubifs/gc.c
> index f146447..16df2f4 100644
> --- a/fs/ubifs/gc.c
> +++ b/fs/ubifs/gc.c
> @@ -322,15 +322,21 @@ 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;
>
> cond_resched();
> err = ubifs_wbuf_write_nolock(wbuf, snod->node, snod->len);
> if (err)
> return err;
>
> + if (key_type(c, &snod->key) == UBIFS_DATA_KEY) {
> + struct ubifs_data_node *dn =
> + (struct ubifs_data_node *) snod->node;
We do not cast void *.
> + crypto_lookup = le64_to_cpu(dn->crypto_lookup);
> + }
> err = ubifs_tnc_replace(c, &snod->key, sleb->lnum,
> snod->offs, new_lnum, new_offs,
> - snod->len, 0);
> + snod->len, crypto_lookup);
> list_del(&snod->list);
> kfree(snod);
> return err;
> diff --git a/fs/ubifs/journal.c b/fs/ubifs/journal.c
> index 2072b9a..3f7aa05 100644
> --- a/fs/ubifs/journal.c
> +++ b/fs/ubifs/journal.c
> @@ -89,7 +89,6 @@ static inline void zero_dent_node_unused(struct ubifs_dent_node *dent)
> */
> static inline void zero_data_node_unused(struct ubifs_data_node *data)
> {
> - data->padding0 = 0;
> memset(data->padding1, 0, 2);
> }
>
> @@ -735,6 +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);
>
> /* Make reservation before allocating sequence numbers */
> err = make_reservation(c, DATAHD, dlen);
> @@ -742,12 +742,14 @@ int ubifs_jnl_write_data(struct ubifs_info *c, const struct inode *inode,
> goto out_free;
>
> err = write_node(c, DATAHD, data, dlen, &lnum, &offs);
> +
> if (err)
Please, do not add junk newlines.
> goto out_release;
> ubifs_wbuf_add_ino_nolock(&c->jheads[DATAHD].wbuf, key_inum(c, key));
> release_head(c, DATAHD);
>
> - err = ubifs_tnc_add(c, key, lnum, offs, dlen, 0);
> + err = ubifs_tnc_add(c, key, lnum, offs, dlen,
> + le64_to_cpu(data->crypto_lookup));
> if (err)
> goto out_ro;
>
> @@ -1226,7 +1228,8 @@ 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, 0);
> + err = ubifs_tnc_add(c, &key, lnum, sz, dlen,
> + le64_to_cpu(dn->crypto_lookup));
> if (err)
> goto out_ro;
> }
> diff --git a/fs/ubifs/tnc.c b/fs/ubifs/tnc.c
> index b222a72..ac0d03c 100644
> --- a/fs/ubifs/tnc.c
> +++ b/fs/ubifs/tnc.c
> @@ -1431,6 +1431,35 @@ static int maybe_leb_gced(struct ubifs_info *c, int lnum, int gc_seq1)
> }
>
> /**
> + * tnc_set_crypto_lookup - set the crypto_lookup field in a zbranch.
> + * @c: UBIFS file-system description object
> + * @zbr: the crypto_lookup field is set in this zbranch
> + * @node: a pointer to a node containing the zbranch.
> + *
> + * Crypto_lookup values are stored on flash memory inside the data node.
> + * When the system is running, they should be oppertunistically stored in
What do you mean by "oppertunistically stored" ? Should be
"opportunistically", but my concern is that we _always_ initialize this
when read, not by opportunity.
> + * memory inside the zbranch. This function is called whenever a node
> + * is read from flash memory. If @node is a data node (therefore containing a
> + * @crypto_lookup field), then:
> + * if @zbr has an unset @crypto_lookup, set it to the value from @node
> + * if @zbr has already a @crypto_lookup, assert they are the same.
> + */
> +static void tnc_set_crypto_lookup(struct ubifs_info *c,
> + struct ubifs_zbranch *zbr, void *node)
> +{
> + if (key_type(c, &(zbr->key)) == UBIFS_DATA_KEY) {
> + struct ubifs_data_node *dn =
> + (struct ubifs_data_node *) node;
No cast.
> + if (zbr->crypto_lookup != 0) {
No braces.
> + ubifs_assert(zbr->crypto_lookup ==
> + le64_to_cpu(dn->crypto_lookup));
> + } else {
Please, introduce a temporary variable for crypto_lookup instead.
--
Best Regards,
Artem Bityutskiy
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 <[email protected]>
---
fs/ubifs/gc.c | 7 ++++++-
fs/ubifs/journal.c | 8 +++++---
fs/ubifs/tnc.c | 39 +++++++++++++++++++++++++++++++++++++++
fs/ubifs/ubifs-media.h | 3 ++-
4 files changed, 52 insertions(+), 5 deletions(-)
diff --git a/fs/ubifs/gc.c b/fs/ubifs/gc.c
index f146447..98e6683 100644
--- a/fs/ubifs/gc.c
+++ b/fs/ubifs/gc.c
@@ -322,15 +322,20 @@ 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;
cond_resched();
err = ubifs_wbuf_write_nolock(wbuf, snod->node, snod->len);
if (err)
return err;
+ if (key_type(c, &snod->key) == UBIFS_DATA_KEY) {
+ struct ubifs_data_node *dn = snod->node;
+ crypto_lookup = le64_to_cpu(dn->crypto_lookup);
+ }
err = ubifs_tnc_replace(c, &snod->key, sleb->lnum,
snod->offs, new_lnum, new_offs,
- snod->len, 0);
+ snod->len, crypto_lookup);
list_del(&snod->list);
kfree(snod);
return err;
diff --git a/fs/ubifs/journal.c b/fs/ubifs/journal.c
index 2072b9a..5f39f75 100644
--- a/fs/ubifs/journal.c
+++ b/fs/ubifs/journal.c
@@ -89,7 +89,6 @@ static inline void zero_dent_node_unused(struct ubifs_dent_node *dent)
*/
static inline void zero_data_node_unused(struct ubifs_data_node *data)
{
- data->padding0 = 0;
memset(data->padding1, 0, 2);
}
@@ -735,6 +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);
/* Make reservation before allocating sequence numbers */
err = make_reservation(c, DATAHD, dlen);
@@ -747,7 +747,8 @@ int ubifs_jnl_write_data(struct ubifs_info *c, const struct inode *inode,
ubifs_wbuf_add_ino_nolock(&c->jheads[DATAHD].wbuf, key_inum(c, key));
release_head(c, DATAHD);
- err = ubifs_tnc_add(c, key, lnum, offs, dlen, 0);
+ err = ubifs_tnc_add(c, key, lnum, offs, dlen,
+ le64_to_cpu(data->crypto_lookup));
if (err)
goto out_ro;
@@ -1226,7 +1227,8 @@ 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, 0);
+ err = ubifs_tnc_add(c, &key, lnum, sz, dlen,
+ le64_to_cpu(dn->crypto_lookup));
if (err)
goto out_ro;
}
diff --git a/fs/ubifs/tnc.c b/fs/ubifs/tnc.c
index b222a72..006caa7 100644
--- a/fs/ubifs/tnc.c
+++ b/fs/ubifs/tnc.c
@@ -1431,6 +1431,34 @@ static int maybe_leb_gced(struct ubifs_info *c, int lnum, int gc_seq1)
}
/**
+ * tnc_set_crypto_lookup - set the crypto_lookup field in a zbranch.
+ * @c: UBIFS file-system description object
+ * @zbr: the crypto_lookup field is set in this zbranch
+ * @node: a pointer to a node containing the zbranch.
+ *
+ * Crypto_lookup values are stored on flash memory inside the data node.
+ * When the system is running, they are also stored in memory inside the
+ * zbranch to avoid requiring reading the old data node simply to retrieve the
+ * crypto_lookup when performing a truncation / overwrite / delete operation.
+ * This function is called whenever a node is read from flash memory. If @node
+ * is a data node (therefore containing a @crypto_lookup field), then:
+ * if @zbr has an unset @crypto_lookup, set it to the value from @node
+ * if @zbr has already a @crypto_lookup, assert they are the same.
+ */
+static void tnc_set_crypto_lookup(struct ubifs_info *c,
+ struct ubifs_zbranch *zbr, void *node)
+{
+ 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);
+ if (zbr->crypto_lookup != 0)
+ ubifs_assert(zbr->crypto_lookup == crypto_lookup);
+ else
+ zbr->crypto_lookup = crypto_lookup;
+ }
+}
+
+/**
* ubifs_tnc_locate - look up a file-system node and return it and its location.
* @c: UBIFS file-system description object
* @key: node key to lookup
@@ -1485,6 +1513,11 @@ again:
if (ubifs_get_wbuf(c, zbr.lnum)) {
/* We do not GC journal heads */
err = ubifs_tnc_read_node(c, &zbr, node);
+ if (!err) {
+ mutex_lock(&c->tnc_mutex);
+ tnc_set_crypto_lookup(c, &(znode->zbranch[n]), node);
+ mutex_unlock(&c->tnc_mutex);
+ }
return err;
}
@@ -1497,9 +1530,15 @@ again:
safely = 1;
goto again;
}
+
+ mutex_lock(&c->tnc_mutex);
+ tnc_set_crypto_lookup(c, &(znode->zbranch[n]), node);
+ mutex_unlock(&c->tnc_mutex);
return 0;
out:
+ if (!err)
+ tnc_set_crypto_lookup(c, zt, node);
mutex_unlock(&c->tnc_mutex);
return err;
}
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! */
--
1.7.5.4
On Wed, 2012-05-16 at 14:52 +0200, Joel Reardon wrote:
> + 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);
> + if (zbr->crypto_lookup != 0)
> + ubifs_assert(zbr->crypto_lookup == crypto_lookup);
> + else
> + zbr->crypto_lookup = crypto_lookup;
> + }
Could you please just inject this code directly to
'ubifs_tnc_read_node()' ? I think it is much easier and more nicer, no?
--
Best Regards,
Artem Bityutskiy
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 <[email protected]>
---
fs/ubifs/gc.c | 8 +++++++-
fs/ubifs/journal.c | 8 +++++---
fs/ubifs/tnc.c | 21 ++++++++++++++++++---
fs/ubifs/tnc_misc.c | 9 +++++++++
fs/ubifs/ubifs-media.h | 3 ++-
5 files changed, 41 insertions(+), 8 deletions(-)
diff --git a/fs/ubifs/gc.c b/fs/ubifs/gc.c
index f146447..4a21358 100644
--- a/fs/ubifs/gc.c
+++ b/fs/ubifs/gc.c
@@ -322,15 +322,21 @@ 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;
cond_resched();
err = ubifs_wbuf_write_nolock(wbuf, snod->node, snod->len);
if (err)
return err;
+ if (key_type(c, &snod->key) == UBIFS_DATA_KEY) {
+ struct ubifs_data_node *dn = snod->node;
+
+ crypto_lookup = le64_to_cpu(dn->crypto_lookup);
+ }
err = ubifs_tnc_replace(c, &snod->key, sleb->lnum,
snod->offs, new_lnum, new_offs,
- snod->len, 0);
+ snod->len, crypto_lookup);
list_del(&snod->list);
kfree(snod);
return err;
diff --git a/fs/ubifs/journal.c b/fs/ubifs/journal.c
index 2072b9a..5f39f75 100644
--- a/fs/ubifs/journal.c
+++ b/fs/ubifs/journal.c
@@ -89,7 +89,6 @@ static inline void zero_dent_node_unused(struct ubifs_dent_node *dent)
*/
static inline void zero_data_node_unused(struct ubifs_data_node *data)
{
- data->padding0 = 0;
memset(data->padding1, 0, 2);
}
@@ -735,6 +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);
/* Make reservation before allocating sequence numbers */
err = make_reservation(c, DATAHD, dlen);
@@ -747,7 +747,8 @@ int ubifs_jnl_write_data(struct ubifs_info *c, const struct inode *inode,
ubifs_wbuf_add_ino_nolock(&c->jheads[DATAHD].wbuf, key_inum(c, key));
release_head(c, DATAHD);
- err = ubifs_tnc_add(c, key, lnum, offs, dlen, 0);
+ err = ubifs_tnc_add(c, key, lnum, offs, dlen,
+ le64_to_cpu(data->crypto_lookup));
if (err)
goto out_ro;
@@ -1226,7 +1227,8 @@ 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, 0);
+ err = ubifs_tnc_add(c, &key, lnum, sz, dlen,
+ le64_to_cpu(dn->crypto_lookup));
if (err)
goto out_ro;
}
diff --git a/fs/ubifs/tnc.c b/fs/ubifs/tnc.c
index b222a72..08185c1 100644
--- a/fs/ubifs/tnc.c
+++ b/fs/ubifs/tnc.c
@@ -499,6 +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.
*/
static int fallible_read_node(struct ubifs_info *c, const union ubifs_key *key,
struct ubifs_zbranch *zbr, void *node)
@@ -511,12 +513,25 @@ static int fallible_read_node(struct ubifs_info *c, const union ubifs_key *key,
zbr->offs);
if (ret == 1) {
union ubifs_key node_key;
- struct ubifs_dent_node *dent = node;
+ struct ubifs_data_node *dn = node;
- /* All nodes have key in the same place */
- key_read(c, &dent->key, &node_key);
+ /* All node types have key in the same place */
+ key_read(c, &dn->key, &node_key);
if (keys_cmp(c, key, &node_key) != 0)
ret = 0;
+ /* If it is actually a data node, read the crypto_lookup */
+ if (key_type(c, key) == UBIFS_DATA_KEY) {
+ long long crypto_lookup;
+
+ 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;
+ }
+
}
if (ret == 0 && c->replaying)
dbg_mntk(key, "dangling branch LEB %d:%d len %d, key ",
diff --git a/fs/ubifs/tnc_misc.c b/fs/ubifs/tnc_misc.c
index 38fdfd0..44c3699 100644
--- a/fs/ubifs/tnc_misc.c
+++ b/fs/ubifs/tnc_misc.c
@@ -490,6 +490,15 @@ int ubifs_tnc_read_node(struct ubifs_info *c, struct ubifs_zbranch *zbr,
dbg_dump_node(c, node);
return -EINVAL;
}
+ 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);
+
+ if (zbr->crypto_lookup != 0)
+ ubifs_assert(zbr->crypto_lookup == crypto_lookup);
+ else
+ zbr->crypto_lookup = crypto_lookup;
+ }
return 0;
}
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! */
--
1.7.5.4
On Sat, 2012-05-19 at 10:30 +0200, Joel Reardon wrote:
> + crypto_lookup = le64_to_cpu(dn->crypto_lookup);
> + if (zbr->crypto_lookup != 0)
> + ubifs_assert(zbr->crypto_lookup ==
> + crypto_lookup);
This "crypto_lookup" name is too long. Would you mind if we re-name it
to "ksa_pos" instead (KSA position)? Then we'll wrap less lines.
We can do this as a separate patch on top of this one. How does this
sound to you?
--
Best Regards,
Artem Bityutskiy
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 <[email protected]>
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 <[email protected]>
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 <[email protected]>
---
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
A few more lines of comment on ksa_pos field in struct ubifs_data_node.
Signed-off-by: Joel Reardon <[email protected]>
---
fs/ubifs/ubifs-media.h | 5 +++++
1 files changed, 5 insertions(+), 0 deletions(-)
diff --git a/fs/ubifs/ubifs-media.h b/fs/ubifs/ubifs-media.h
index 55f2ccd..90f348c 100644
--- a/fs/ubifs/ubifs-media.h
+++ b/fs/ubifs/ubifs-media.h
@@ -545,6 +545,11 @@ struct ubifs_dent_node {
* @padding: reserved for future, zeroes
* @data: data
*
+ * The @ksa_pos value is used for secure deletion enhancement of UBIFS.
+ * In older versions of UBIFS, @ksa_pos did not exist but was instead an
+ * unused part of @key and was set to all zeros. UBIFS partitions with secure
+ * deletion disabled also store all zeros for ksa_pos.
+ *
* Note, do not forget to amend 'zero_data_node_unused()' function when
* changing the padding fields.
*/
--
1.7.5.4
On Sun, 2012-05-20 at 13:35 +0200, Joel Reardon wrote:
> A few more lines of comment on ksa_pos field in struct ubifs_data_node.
>
> Signed-off-by: Joel Reardon <[email protected]>
Pushed to the "joel" branch, thanks!
--
Best Regards,
Artem Bityutskiy