| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <1337087022.2528.204.camel@sauron.fi.intel.com>
Date: Tue, 15 May 2012 16:03:42 +0300
From: Artem Bityutskiy <dedekind1@...il.com>
To: Joel Reardon <joel@...mbassador.com>
Cc: linux-mtd@...ts.infradead.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] UBIFS: read crypto_lookup from datanodes to znodes
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 <reardonj@....ethz.ch>
> ---
> 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
Download attachment "signature.asc" of type "application/pgp-signature" (837 bytes)
Powered by blists - more mailing lists