mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH] ubifs: fix auth node space and padding in journal buffers
@ 2026-10-30  3:45 shao.mingyin
  2026-10-30  6:50 ` Zhihao Cheng
  0 siblings, 1 reply; 4+ messages in thread
From: shao.mingyin @ 2026-10-30  3:45 UTC (permalink / raw)
  To: richard, linux-mtd; +Cc: chengzhihao1, linux-kernel, stable

From: Shao Mingyin <shao.mingyin@zte.com.cn>

write_head() appends an authentication node to the node group when
authentication is enabled and writes
ALIGN(node_len, 8) + ubifs_auth_node_sz(c) bytes from the buffer.
ubifs_jnl_write_data() allocates its buffer for that, but when the
allocation fails it falls back to c->write_reserve_buf.  That buffer is
sized for a data node only (COMPRESSED_DATA_NODE_BUF_SZ plus the cipher
block, 4144/4160 bytes), while an incompressible 4 KiB block needs
4144/4160 bytes plus the auth node (24 + the HMAC length, 56 bytes with
sha256).  ubifs_hash_nodes() then writes the auth node past the end of
the reserve buffer, and write_head() reads it back from there.

The same write path also leaks uninitialized heap data.
ubifs_jnl_write_data(), ubifs_jnl_write_inode() and
ubifs_jnl_truncate() build their node groups in kmalloc()ed buffers, and
when the last node length is not a multiple of 8 the padding up to the
next auth node is written to the flash.  That padding is covered neither
by the node CRC nor by the rolling hash.

Size the reserve buffer for the largest possible auth node (the mount
path allocates it before authentication is initialized, so it cannot use
ubifs_auth_node_sz()) and zero the padding after the last node of each
group before it is written.  The padding is not part of any node, so
ch.len and the CRCs stay unchanged.

Fixes: 6a98bc4614de ("ubifs: Add authentication nodes to journal")
Cc: stable@vger.kernel.org
Signed-off-by: Shao Mingyin <shao.mingyin@zte.com.cn>
---
diff --git a/fs/ubifs/journal.c b/fs/ubifs/journal.c
index 40a95a2fad50..38fb59225f27 100644
--- a/fs/ubifs/journal.c
+++ b/fs/ubifs/journal.c
@@ -840,6 +840,24 @@ int ubifs_jnl_update(struct ubifs_info *c, const struct inode *dir,
 	return err;
 }

+/**
+ * ubifs_zero_node_pad - zero the padding after the last node of a group
+ * @c: UBIFS file-system description object
+ * @node: last node of the group
+ * @len: length of the node
+ *
+ * When authentication is enabled the auth node is placed at the next
+ * 8-byte aligned offset, so the bytes between the end of the last node
+ * and that offset are written to the media.  They are covered neither
+ * by the node CRC nor by the rolling hash, so clear them instead of
+ * writing stale heap contents.
+ */
+static void ubifs_zero_node_pad(const struct ubifs_info *c, void *node, int len)
+{
+	if (ubifs_authenticated(c))
+		memset((char *)node + len, 0, ALIGN(len, 8) - len);
+}
+
 /**
  * ubifs_jnl_write_data - write a data node to the journal.
  * @c: UBIFS file-system description object
@@ -925,6 +943,7 @@ int ubifs_jnl_write_data(struct ubifs_info *c, const struct inode *inode,
 	if (err)
 		goto out_free;

+	ubifs_zero_node_pad(c, data, dlen);
 	ubifs_prepare_node(c, data, dlen, 0);
 	err = write_head(c, DATAHD, data, write_len, &lnum, &offs, 0);
 	if (err)
@@ -1062,6 +1081,7 @@ int ubifs_jnl_write_inode(struct ubifs_info *c, const struct inode *inode)
 	}

 	pack_inode(c, ino, inode, 1);
+	ubifs_zero_node_pad(c, ino, ilen);
 	err = ubifs_node_calc_hash(c, ino, hash);
 	if (err)
 		goto out_release;
@@ -1781,6 +1801,7 @@ int ubifs_jnl_truncate(struct ubifs_info *c, const struct inode *inode,
 	ubifs_prep_grp_node(c, trun, UBIFS_TRUN_NODE_SZ, dlen ? 0 : 1);
 	if (dlen) {
 		ubifs_prep_grp_node(c, dn, dlen, 1);
+		ubifs_zero_node_pad(c, dn, dlen);
 		err = ubifs_node_calc_hash(c, dn, hash_dn);
 		if (err)
 			goto out_release;
diff --git a/fs/ubifs/super.c b/fs/ubifs/super.c
index 38972786817e..538907aba8a0 100644
--- a/fs/ubifs/super.c
+++ b/fs/ubifs/super.c
@@ -1263,8 +1263,16 @@ static int mount_ubifs(struct ubifs_info *c)
 		bu_init(c);

 	if (!c->ro_mount) {
-		c->write_reserve_buf = kmalloc(COMPRESSED_DATA_NODE_BUF_SZ + \
-					       UBIFS_CIPHER_BLOCK_SIZE,
+		/*
+		 * The reserve buffer is also used as a fallback for data
+		 * nodes which are followed by an authentication node, so
+		 * reserve room for the largest possible one.  Authentication
+		 * is initialized only later, hence the maximum sizes.
+		 */
+		c->write_reserve_buf = kmalloc(COMPRESSED_DATA_NODE_BUF_SZ +
+					       UBIFS_CIPHER_BLOCK_SIZE +
+					       UBIFS_AUTH_NODE_SZ +
+					       UBIFS_MAX_HMAC_LEN,
 					       GFP_KERNEL);
 		if (!c->write_reserve_buf)
 			goto out_free;
@@ -1723,8 +1731,10 @@ static int ubifs_remount_rw(struct ubifs_info *c)
 		goto out;
 	}

-	c->write_reserve_buf = kmalloc(COMPRESSED_DATA_NODE_BUF_SZ + \
-				       UBIFS_CIPHER_BLOCK_SIZE, GFP_KERNEL);
+	c->write_reserve_buf = kmalloc(COMPRESSED_DATA_NODE_BUF_SZ +
+				       UBIFS_CIPHER_BLOCK_SIZE +
+				       UBIFS_AUTH_NODE_SZ +
+				       UBIFS_MAX_HMAC_LEN, GFP_KERNEL);
 	if (!c->write_reserve_buf) {
 		err = -ENOMEM;
 		goto out;

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] ubifs: fix auth node space and padding in journal buffers
  2026-10-30  3:45 [PATCH] ubifs: fix auth node space and padding in journal buffers shao.mingyin
@ 2026-10-30  6:50 ` Zhihao Cheng
  2026-10-30  9:26   ` [PATCH v2] " shao.mingyin
  0 siblings, 1 reply; 4+ messages in thread
From: Zhihao Cheng @ 2026-10-30  6:50 UTC (permalink / raw)
  To: shao.mingyin, richard, linux-mtd; +Cc: linux-kernel, stable

在 2026/10/30 11:45, shao.mingyin@zte.com.cn 写道:
> From: Shao Mingyin <shao.mingyin@zte.com.cn>
> 
> write_head() appends an authentication node to the node group when
> authentication is enabled and writes
> ALIGN(node_len, 8) + ubifs_auth_node_sz(c) bytes from the buffer.
> ubifs_jnl_write_data() allocates its buffer for that, but when the
> allocation fails it falls back to c->write_reserve_buf.  That buffer is
> sized for a data node only (COMPRESSED_DATA_NODE_BUF_SZ plus the cipher
> block, 4144/4160 bytes), while an incompressible 4 KiB block needs
> 4144/4160 bytes plus the auth node (24 + the HMAC length, 56 bytes with
> sha256).  ubifs_hash_nodes() then writes the auth node past the end of
> the reserve buffer, and write_head() reads it back from there.
> 
> The same write path also leaks uninitialized heap data.
> ubifs_jnl_write_data(), ubifs_jnl_write_inode() and
> ubifs_jnl_truncate() build their node groups in kmalloc()ed buffers, and
> when the last node length is not a multiple of 8 the padding up to the
> next auth node is written to the flash.  That padding is covered neither
> by the node CRC nor by the rolling hash.
> 
> Size the reserve buffer for the largest possible auth node (the mount
> path allocates it before authentication is initialized, so it cannot use
> ubifs_auth_node_sz()) and zero the padding after the last node of each
> group before it is written.  The padding is not part of any node, so
> ch.len and the CRCs stay unchanged.
> 
> Fixes: 6a98bc4614de ("ubifs: Add authentication nodes to journal")
> Cc: stable@vger.kernel.org
> Signed-off-by: Shao Mingyin <shao.mingyin@zte.com.cn>
> ---
> diff --git a/fs/ubifs/journal.c b/fs/ubifs/journal.c
> index 40a95a2fad50..38fb59225f27 100644
> --- a/fs/ubifs/journal.c
> +++ b/fs/ubifs/journal.c
> @@ -840,6 +840,24 @@ int ubifs_jnl_update(struct ubifs_info *c, const struct inode *dir,
>   	return err;
>   }
> 
> +/**
> + * ubifs_zero_node_pad - zero the padding after the last node of a group
> + * @c: UBIFS file-system description object
> + * @node: last node of the group
> + * @len: length of the node
> + *
> + * When authentication is enabled the auth node is placed at the next
> + * 8-byte aligned offset, so the bytes between the end of the last node
> + * and that offset are written to the media.  They are covered neither
> + * by the node CRC nor by the rolling hash, so clear them instead of
> + * writing stale heap contents.
> + */
> +static void ubifs_zero_node_pad(const struct ubifs_info *c, void *node, int len)
> +{
> +	if (ubifs_authenticated(c))
> +		memset((char *)node + len, 0, ALIGN(len, 8) - len);
> +}
> +
>   /**
>    * ubifs_jnl_write_data - write a data node to the journal.
>    * @c: UBIFS file-system description object
> @@ -925,6 +943,7 @@ int ubifs_jnl_write_data(struct ubifs_info *c, const struct inode *inode,
>   	if (err)
>   		goto out_free;
> 
> +	ubifs_zero_node_pad(c, data, dlen);
>   	ubifs_prepare_node(c, data, dlen, 0);
>   	err = write_head(c, DATAHD, data, write_len, &lnum, &offs, 0);
>   	if (err)
Hi Mingyin,
Since ubifs_jnl_write_data() could use c->write_reserve_buf, we can keep 
the modification in ubifs_jnl_write_data(). I suggest that replace 
kmalloc with kzalloc in ubifs_jnl_write_inode() and 
ubifs_jnl_truncate(), and remove zero_trun_node_unused from 
ubifs_jnl_truncate().
> @@ -1062,6 +1081,7 @@ int ubifs_jnl_write_inode(struct ubifs_info *c, const struct inode *inode)
>   	}
> 
>   	pack_inode(c, ino, inode, 1);
> +	ubifs_zero_node_pad(c, ino, ilen);
>   	err = ubifs_node_calc_hash(c, ino, hash);
>   	if (err)
>   		goto out_release;
> @@ -1781,6 +1801,7 @@ int ubifs_jnl_truncate(struct ubifs_info *c, const struct inode *inode,
>   	ubifs_prep_grp_node(c, trun, UBIFS_TRUN_NODE_SZ, dlen ? 0 : 1);
>   	if (dlen) {
>   		ubifs_prep_grp_node(c, dn, dlen, 1);
> +		ubifs_zero_node_pad(c, dn, dlen);
>   		err = ubifs_node_calc_hash(c, dn, hash_dn);
>   		if (err)
>   			goto out_release;
> diff --git a/fs/ubifs/super.c b/fs/ubifs/super.c
> index 38972786817e..538907aba8a0 100644
> --- a/fs/ubifs/super.c
> +++ b/fs/ubifs/super.c
> @@ -1263,8 +1263,16 @@ static int mount_ubifs(struct ubifs_info *c)
>   		bu_init(c);
> 
>   	if (!c->ro_mount) {
> -		c->write_reserve_buf = kmalloc(COMPRESSED_DATA_NODE_BUF_SZ + \
> -					       UBIFS_CIPHER_BLOCK_SIZE,
> +		/*
> +		 * The reserve buffer is also used as a fallback for data
> +		 * nodes which are followed by an authentication node, so
> +		 * reserve room for the largest possible one.  Authentication
> +		 * is initialized only later, hence the maximum sizes.
> +		 */
> +		c->write_reserve_buf = kmalloc(COMPRESSED_DATA_NODE_BUF_SZ +
> +					       UBIFS_CIPHER_BLOCK_SIZE +
> +					       UBIFS_AUTH_NODE_SZ +
> +					       UBIFS_MAX_HMAC_LEN,
>   					       GFP_KERNEL);
>   		if (!c->write_reserve_buf)
>   			goto out_free;
> @@ -1723,8 +1731,10 @@ static int ubifs_remount_rw(struct ubifs_info *c)
>   		goto out;
>   	}
> 
> -	c->write_reserve_buf = kmalloc(COMPRESSED_DATA_NODE_BUF_SZ + \
> -				       UBIFS_CIPHER_BLOCK_SIZE, GFP_KERNEL);
> +	c->write_reserve_buf = kmalloc(COMPRESSED_DATA_NODE_BUF_SZ +
> +				       UBIFS_CIPHER_BLOCK_SIZE +
> +				       UBIFS_AUTH_NODE_SZ +
> +				       UBIFS_MAX_HMAC_LEN, GFP_KERNEL);
>   	if (!c->write_reserve_buf) {
>   		err = -ENOMEM;
>   		goto out;
> .
> 


^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH v2] ubifs: fix auth node space and padding in journal buffers
  2026-10-30  6:50 ` Zhihao Cheng
@ 2026-10-30  9:26   ` shao.mingyin
  2026-10-30  9:39     ` Zhihao Cheng
  0 siblings, 1 reply; 4+ messages in thread
From: shao.mingyin @ 2026-10-30  9:26 UTC (permalink / raw)
  To: richard, linux-mtd; +Cc: linux-kernel, stable, chengzhihao1

From: Shao Mingyin <shao.mingyin@zte.com.cn>

write_head() appends an authentication node to the node group when
authentication is enabled and writes
ALIGN(node_len, 8) + ubifs_auth_node_sz(c) bytes from the buffer.
ubifs_jnl_write_data() allocates its buffer for that, but when the
allocation fails it falls back to c->write_reserve_buf.  That buffer is
sized for a data node only (COMPRESSED_DATA_NODE_BUF_SZ plus the cipher
block, 4144/4160 bytes), while an incompressible 4 KiB block needs
4144/4160 bytes plus the auth node (24 + the HMAC length, 56 bytes with
sha256).  ubifs_hash_nodes() then writes the auth node past the end of
the reserve buffer, and write_head() reads it back from there.

The same write path also writes uninitialized heap data.
ubifs_jnl_write_data() and ubifs_jnl_write_inode() build their node
groups in kmalloc()ed buffers, and when the last node length is not a
multiple of 8 the padding up to the next auth node goes to the flash
uninitialized.  That padding is covered neither by the node CRC nor by
the rolling hash.

Size the reserve buffer for the largest possible auth node (the mount
path allocates it before authentication is initialized, so it cannot use
ubifs_auth_node_sz()), clear the padding after the data node before it is
written, and allocate the node group buffers of ubifs_jnl_write_inode()
and ubifs_jnl_truncate() with kzalloc().  The data write path cannot just
use kzalloc() because it may fall back to the reused reserve buffer.
ubifs_jnl_truncate() refills its data node with ubifs_tnc_lookup(), so
what is left after the re-compressed node comes from the old node on the
media, not from the heap.

The padding is not part of any node, so ch.len and the CRCs stay
unchanged.  With a zeroed truncation buffer, zero_trun_node_unused() is
redundant, so remove it and the comments which refer to it.

Fixes: 6a98bc4614de ("ubifs: Add authentication nodes to journal")
Link: https://lore.kernel.org/all/ab51df45-f763-358b-642c-8efe06f33ffb@huawei.com/
Cc: stable@vger.kernel.org
Suggested-by: Zhihao Cheng <chengzhihao1@huawei.com>
Signed-off-by: Shao Mingyin <shao.mingyin@zte.com.cn>
---
v2: Keep the explicit padding clear only in ubifs_jnl_write_data(), which
    may fall back to the reused write reserve buffer; allocate the node
    groups of ubifs_jnl_write_inode() and ubifs_jnl_truncate() with
    kzalloc(), and drop zero_trun_node_unused() together with the
    comments pointing at it.  Thanks to Zhihao Cheng for the review.

 fs/ubifs/journal.c     | 39 ++++++++++++++++++++++++++-------------
 fs/ubifs/super.c       | 18 ++++++++++++++----
 fs/ubifs/ubifs-media.h |  6 ++----
 3 files changed, 42 insertions(+), 21 deletions(-)

diff --git a/fs/ubifs/journal.c b/fs/ubifs/journal.c
index 40a95a2fad50..449bc8c0a224 100644
--- a/fs/ubifs/journal.c
+++ b/fs/ubifs/journal.c
@@ -68,16 +68,6 @@ static inline void zero_dent_node_unused(struct ubifs_dent_node *dent)
 	dent->padding1 = 0;
 }

-/**
- * zero_trun_node_unused - zero out unused fields of an on-flash truncation
- *                         node.
- * @trun: the truncation node to zero out
- */
-static inline void zero_trun_node_unused(struct ubifs_trun_node *trun)
-{
-	memset(trun->padding, 0, 12);
-}
-
 static void ubifs_add_auth_dirt(struct ubifs_info *c, int lnum)
 {
 	if (ubifs_authenticated(c))
@@ -840,6 +830,28 @@ int ubifs_jnl_update(struct ubifs_info *c, const struct inode *dir,
 	return err;
 }

+/**
+ * ubifs_zero_node_pad - zero the padding after a journal node
+ * @c: UBIFS file-system description object
+ * @node: node to pad
+ * @len: length of the node
+ *
+ * When authentication is enabled the auth node is placed at the next
+ * 8-byte aligned offset, so the bytes between the end of the node and
+ * that offset are written to the media.  They are covered neither by the
+ * node CRC nor by the rolling hash, so clear them instead of writing
+ * stale buffer contents.
+ *
+ * This is only needed when the buffer is reused, i.e. for the write
+ * reserve buffer of ubifs_jnl_write_data().  Buffer owners which get a
+ * fresh allocation use kzalloc() instead.
+ */
+static void ubifs_zero_node_pad(const struct ubifs_info *c, void *node, int len)
+{
+	if (ubifs_authenticated(c))
+		memset((char *)node + len, 0, ALIGN(len, 8) - len);
+}
+
 /**
  * ubifs_jnl_write_data - write a data node to the journal.
  * @c: UBIFS file-system description object
@@ -925,6 +937,7 @@ int ubifs_jnl_write_data(struct ubifs_info *c, const struct inode *inode,
 	if (err)
 		goto out_free;

+	ubifs_zero_node_pad(c, data, dlen);
 	ubifs_prepare_node(c, data, dlen, 0);
 	err = write_head(c, DATAHD, data, write_len, &lnum, &offs, 0);
 	if (err)
@@ -1007,9 +1020,10 @@ int ubifs_jnl_write_inode(struct ubifs_info *c, const struct inode *inode)
 	else
 		write_len += ilen;

-	ino_start = ino = kmalloc(write_len, GFP_NOFS);
+	ino = kzalloc(write_len, GFP_NOFS);
 	if (!ino)
 		return -ENOMEM;
+	ino_start = ino;

 	/* Make reservation before allocating sequence numbers */
 	err = make_reservation(c, BASEHD, write_len);
@@ -1716,7 +1730,7 @@ int ubifs_jnl_truncate(struct ubifs_info *c, const struct inode *inode,
 	sz =  UBIFS_TRUN_NODE_SZ + UBIFS_INO_NODE_SZ +
 		dn_size + ubifs_auth_node_sz(c);

-	ino = kmalloc(sz, GFP_NOFS);
+	ino = kzalloc(sz, GFP_NOFS);
 	if (!ino)
 		return -ENOMEM;

@@ -1725,7 +1739,6 @@ int ubifs_jnl_truncate(struct ubifs_info *c, const struct inode *inode,
 	trun->inum = cpu_to_le32(inum);
 	trun->old_size = cpu_to_le64(old_size);
 	trun->new_size = cpu_to_le64(new_size);
-	zero_trun_node_unused(trun);

 	dlen = new_size & (UBIFS_BLOCK_SIZE - 1);
 	if (dlen) {
diff --git a/fs/ubifs/super.c b/fs/ubifs/super.c
index 38972786817e..538907aba8a0 100644
--- a/fs/ubifs/super.c
+++ b/fs/ubifs/super.c
@@ -1263,8 +1263,16 @@ static int mount_ubifs(struct ubifs_info *c)
 		bu_init(c);

 	if (!c->ro_mount) {
-		c->write_reserve_buf = kmalloc(COMPRESSED_DATA_NODE_BUF_SZ + \
-					       UBIFS_CIPHER_BLOCK_SIZE,
+		/*
+		 * The reserve buffer is also used as a fallback for data
+		 * nodes which are followed by an authentication node, so
+		 * reserve room for the largest possible one.  Authentication
+		 * is initialized only later, hence the maximum sizes.
+		 */
+		c->write_reserve_buf = kmalloc(COMPRESSED_DATA_NODE_BUF_SZ +
+					       UBIFS_CIPHER_BLOCK_SIZE +
+					       UBIFS_AUTH_NODE_SZ +
+					       UBIFS_MAX_HMAC_LEN,
 					       GFP_KERNEL);
 		if (!c->write_reserve_buf)
 			goto out_free;
@@ -1723,8 +1731,10 @@ static int ubifs_remount_rw(struct ubifs_info *c)
 		goto out;
 	}

-	c->write_reserve_buf = kmalloc(COMPRESSED_DATA_NODE_BUF_SZ + \
-				       UBIFS_CIPHER_BLOCK_SIZE, GFP_KERNEL);
+	c->write_reserve_buf = kmalloc(COMPRESSED_DATA_NODE_BUF_SZ +
+				       UBIFS_CIPHER_BLOCK_SIZE +
+				       UBIFS_AUTH_NODE_SZ +
+				       UBIFS_MAX_HMAC_LEN, GFP_KERNEL);
 	if (!c->write_reserve_buf) {
 		err = -ENOMEM;
 		goto out;
diff --git a/fs/ubifs/ubifs-media.h b/fs/ubifs/ubifs-media.h
index 3c9792cbb6ff..7d4702f3e15f 100644
--- a/fs/ubifs/ubifs-media.h
+++ b/fs/ubifs/ubifs-media.h
@@ -591,14 +591,12 @@ struct ubifs_data_node {
  * @old_size: size before truncation
  * @new_size: size after truncation
  *
- * This node exists only in the journal and never goes to the main area. Note,
- * do not forget to amend 'zero_trun_node_unused()' function when changing the
- * padding fields.
+ * This node exists only in the journal and never goes to the main area.
  */
 struct ubifs_trun_node {
 	struct ubifs_ch ch;
 	__le32 inum;
-	__u8 padding[12]; /* Watch 'zero_trun_node_unused()' if changing! */
+	__u8 padding[12];
 	__le64 old_size;
 	__le64 new_size;
 } __packed;

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v2] ubifs: fix auth node space and padding in journal buffers
  2026-10-30  9:26   ` [PATCH v2] " shao.mingyin
@ 2026-10-30  9:39     ` Zhihao Cheng
  0 siblings, 0 replies; 4+ messages in thread
From: Zhihao Cheng @ 2026-10-30  9:39 UTC (permalink / raw)
  To: shao.mingyin, richard, linux-mtd; +Cc: linux-kernel, stable

在 2026/10/30 17:26, shao.mingyin@zte.com.cn 写道:
> From: Shao Mingyin <shao.mingyin@zte.com.cn>
> 
> write_head() appends an authentication node to the node group when
> authentication is enabled and writes
> ALIGN(node_len, 8) + ubifs_auth_node_sz(c) bytes from the buffer.
> ubifs_jnl_write_data() allocates its buffer for that, but when the
> allocation fails it falls back to c->write_reserve_buf.  That buffer is
> sized for a data node only (COMPRESSED_DATA_NODE_BUF_SZ plus the cipher
> block, 4144/4160 bytes), while an incompressible 4 KiB block needs
> 4144/4160 bytes plus the auth node (24 + the HMAC length, 56 bytes with
> sha256).  ubifs_hash_nodes() then writes the auth node past the end of
> the reserve buffer, and write_head() reads it back from there.
> 
> The same write path also writes uninitialized heap data.
> ubifs_jnl_write_data() and ubifs_jnl_write_inode() build their node
> groups in kmalloc()ed buffers, and when the last node length is not a
> multiple of 8 the padding up to the next auth node goes to the flash
> uninitialized.  That padding is covered neither by the node CRC nor by
> the rolling hash.
> 
> Size the reserve buffer for the largest possible auth node (the mount
> path allocates it before authentication is initialized, so it cannot use
> ubifs_auth_node_sz()), clear the padding after the data node before it is
> written, and allocate the node group buffers of ubifs_jnl_write_inode()
> and ubifs_jnl_truncate() with kzalloc().  The data write path cannot just
> use kzalloc() because it may fall back to the reused reserve buffer.
> ubifs_jnl_truncate() refills its data node with ubifs_tnc_lookup(), so
> what is left after the re-compressed node comes from the old node on the
> media, not from the heap.
> 
> The padding is not part of any node, so ch.len and the CRCs stay
> unchanged.  With a zeroed truncation buffer, zero_trun_node_unused() is
> redundant, so remove it and the comments which refer to it.
> 
> Fixes: 6a98bc4614de ("ubifs: Add authentication nodes to journal")
> Link: https://lore.kernel.org/all/ab51df45-f763-358b-642c-8efe06f33ffb@huawei.com/
> Cc: stable@vger.kernel.org
> Suggested-by: Zhihao Cheng <chengzhihao1@huawei.com>
> Signed-off-by: Shao Mingyin <shao.mingyin@zte.com.cn>
> ---
> v2: Keep the explicit padding clear only in ubifs_jnl_write_data(), which
>      may fall back to the reused write reserve buffer; allocate the node
>      groups of ubifs_jnl_write_inode() and ubifs_jnl_truncate() with
>      kzalloc(), and drop zero_trun_node_unused() together with the
>      comments pointing at it.  Thanks to Zhihao Cheng for the review.
> 
>   fs/ubifs/journal.c     | 39 ++++++++++++++++++++++++++-------------
>   fs/ubifs/super.c       | 18 ++++++++++++++----
>   fs/ubifs/ubifs-media.h |  6 ++----
>   3 files changed, 42 insertions(+), 21 deletions(-)
> 
Reviewed-by: Zhihao Cheng <chengzhihao1@huawei.com>
> diff --git a/fs/ubifs/journal.c b/fs/ubifs/journal.c
> index 40a95a2fad50..449bc8c0a224 100644
> --- a/fs/ubifs/journal.c
> +++ b/fs/ubifs/journal.c
> @@ -68,16 +68,6 @@ static inline void zero_dent_node_unused(struct ubifs_dent_node *dent)
>   	dent->padding1 = 0;
>   }
> 
> -/**
> - * zero_trun_node_unused - zero out unused fields of an on-flash truncation
> - *                         node.
> - * @trun: the truncation node to zero out
> - */
> -static inline void zero_trun_node_unused(struct ubifs_trun_node *trun)
> -{
> -	memset(trun->padding, 0, 12);
> -}
> -
>   static void ubifs_add_auth_dirt(struct ubifs_info *c, int lnum)
>   {
>   	if (ubifs_authenticated(c))
> @@ -840,6 +830,28 @@ int ubifs_jnl_update(struct ubifs_info *c, const struct inode *dir,
>   	return err;
>   }
> 
> +/**
> + * ubifs_zero_node_pad - zero the padding after a journal node
> + * @c: UBIFS file-system description object
> + * @node: node to pad
> + * @len: length of the node
> + *
> + * When authentication is enabled the auth node is placed at the next
> + * 8-byte aligned offset, so the bytes between the end of the node and
> + * that offset are written to the media.  They are covered neither by the
> + * node CRC nor by the rolling hash, so clear them instead of writing
> + * stale buffer contents.
> + *
> + * This is only needed when the buffer is reused, i.e. for the write
> + * reserve buffer of ubifs_jnl_write_data().  Buffer owners which get a
> + * fresh allocation use kzalloc() instead.
> + */
> +static void ubifs_zero_node_pad(const struct ubifs_info *c, void *node, int len)
> +{
> +	if (ubifs_authenticated(c))
> +		memset((char *)node + len, 0, ALIGN(len, 8) - len);
> +}
> +
>   /**
>    * ubifs_jnl_write_data - write a data node to the journal.
>    * @c: UBIFS file-system description object
> @@ -925,6 +937,7 @@ int ubifs_jnl_write_data(struct ubifs_info *c, const struct inode *inode,
>   	if (err)
>   		goto out_free;
> 
> +	ubifs_zero_node_pad(c, data, dlen);
>   	ubifs_prepare_node(c, data, dlen, 0);
>   	err = write_head(c, DATAHD, data, write_len, &lnum, &offs, 0);
>   	if (err)
> @@ -1007,9 +1020,10 @@ int ubifs_jnl_write_inode(struct ubifs_info *c, const struct inode *inode)
>   	else
>   		write_len += ilen;
> 
> -	ino_start = ino = kmalloc(write_len, GFP_NOFS);
> +	ino = kzalloc(write_len, GFP_NOFS);
>   	if (!ino)
>   		return -ENOMEM;
> +	ino_start = ino;
> 
>   	/* Make reservation before allocating sequence numbers */
>   	err = make_reservation(c, BASEHD, write_len);
> @@ -1716,7 +1730,7 @@ int ubifs_jnl_truncate(struct ubifs_info *c, const struct inode *inode,
>   	sz =  UBIFS_TRUN_NODE_SZ + UBIFS_INO_NODE_SZ +
>   		dn_size + ubifs_auth_node_sz(c);
> 
> -	ino = kmalloc(sz, GFP_NOFS);
> +	ino = kzalloc(sz, GFP_NOFS);
>   	if (!ino)
>   		return -ENOMEM;
> 
> @@ -1725,7 +1739,6 @@ int ubifs_jnl_truncate(struct ubifs_info *c, const struct inode *inode,
>   	trun->inum = cpu_to_le32(inum);
>   	trun->old_size = cpu_to_le64(old_size);
>   	trun->new_size = cpu_to_le64(new_size);
> -	zero_trun_node_unused(trun);
> 
>   	dlen = new_size & (UBIFS_BLOCK_SIZE - 1);
>   	if (dlen) {
> diff --git a/fs/ubifs/super.c b/fs/ubifs/super.c
> index 38972786817e..538907aba8a0 100644
> --- a/fs/ubifs/super.c
> +++ b/fs/ubifs/super.c
> @@ -1263,8 +1263,16 @@ static int mount_ubifs(struct ubifs_info *c)
>   		bu_init(c);
> 
>   	if (!c->ro_mount) {
> -		c->write_reserve_buf = kmalloc(COMPRESSED_DATA_NODE_BUF_SZ + \
> -					       UBIFS_CIPHER_BLOCK_SIZE,
> +		/*
> +		 * The reserve buffer is also used as a fallback for data
> +		 * nodes which are followed by an authentication node, so
> +		 * reserve room for the largest possible one.  Authentication
> +		 * is initialized only later, hence the maximum sizes.
> +		 */
> +		c->write_reserve_buf = kmalloc(COMPRESSED_DATA_NODE_BUF_SZ +
> +					       UBIFS_CIPHER_BLOCK_SIZE +
> +					       UBIFS_AUTH_NODE_SZ +
> +					       UBIFS_MAX_HMAC_LEN,
>   					       GFP_KERNEL);
>   		if (!c->write_reserve_buf)
>   			goto out_free;
> @@ -1723,8 +1731,10 @@ static int ubifs_remount_rw(struct ubifs_info *c)
>   		goto out;
>   	}
> 
> -	c->write_reserve_buf = kmalloc(COMPRESSED_DATA_NODE_BUF_SZ + \
> -				       UBIFS_CIPHER_BLOCK_SIZE, GFP_KERNEL);
> +	c->write_reserve_buf = kmalloc(COMPRESSED_DATA_NODE_BUF_SZ +
> +				       UBIFS_CIPHER_BLOCK_SIZE +
> +				       UBIFS_AUTH_NODE_SZ +
> +				       UBIFS_MAX_HMAC_LEN, GFP_KERNEL);
>   	if (!c->write_reserve_buf) {
>   		err = -ENOMEM;
>   		goto out;
> diff --git a/fs/ubifs/ubifs-media.h b/fs/ubifs/ubifs-media.h
> index 3c9792cbb6ff..7d4702f3e15f 100644
> --- a/fs/ubifs/ubifs-media.h
> +++ b/fs/ubifs/ubifs-media.h
> @@ -591,14 +591,12 @@ struct ubifs_data_node {
>    * @old_size: size before truncation
>    * @new_size: size after truncation
>    *
> - * This node exists only in the journal and never goes to the main area. Note,
> - * do not forget to amend 'zero_trun_node_unused()' function when changing the
> - * padding fields.
> + * This node exists only in the journal and never goes to the main area.
>    */
>   struct ubifs_trun_node {
>   	struct ubifs_ch ch;
>   	__le32 inum;
> -	__u8 padding[12]; /* Watch 'zero_trun_node_unused()' if changing! */
> +	__u8 padding[12];
>   	__le64 old_size;
>   	__le64 new_size;
>   } __packed;
> .
> 


^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2026-10-30  9:39 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-10-30  3:45 [PATCH] ubifs: fix auth node space and padding in journal buffers shao.mingyin
2026-10-30  6:50 ` Zhihao Cheng
2026-10-30  9:26   ` [PATCH v2] " shao.mingyin
2026-10-30  9:39     ` Zhihao Cheng

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®