Re: [PATCH] ubifs: fix auth node space and padding in journal buffers

[Zhihao Cheng <chengzhihao1@huawei.com>]

在 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;
> .
>