Re: [PATCH] ecryptfs: validate packet parser buffer lengths
From: Tyler Hicks
Date: Tue Jul 14 2026 - 00:19:13 EST
On 2026-06-27 17:02:08, Yichong Chen wrote:
> ecryptfs_parse_packet_set() receives a pointer into the file header,
> but it calculates the remaining packet buffer size from PAGE_SIZE - 8.
> For version 1 headers the packet set starts later in the header, so
> this can overstate the available buffer.
>
> Pass the actual packet set buffer length from the caller and use the
> remaining length after each parsed packet. Also fix the tag 11 packet
> exact-fit bounds check and reject too-small tag 70 packet bodies before
> subtracting fixed metadata sizes.
My apologies for just now getting to the review of this patch but could
you please split out these two separate fixes (tag 11 exact-fit check
and too-small tag 70 check) into their own patches? I feel like these
are three distinct fixes and should be treated that way in the git
history.
These are nice improvements! I only have one small, additional comment
below.
>
> Fixes: 237fead61998 ("[PATCH] ecryptfs: fs/Makefile and fs/Kconfig")
> Fixes: 9c79f34f7ee7 ("eCryptfs: Filename Encryption: Tag 70 packets")
> Signed-off-by: Yichong Chen <chenyichong@xxxxxxxxxxxxx>
> ---
> fs/ecryptfs/crypto.c | 2 +-
> fs/ecryptfs/ecryptfs_kernel.h | 3 ++-
> fs/ecryptfs/keystore.c | 32 +++++++++++++++++++++++++++-----
> 3 files changed, 30 insertions(+), 7 deletions(-)
>
> diff --git a/fs/ecryptfs/crypto.c b/fs/ecryptfs/crypto.c
> index 74b02b55e3f6..e67119b6029c 100644
> --- a/fs/ecryptfs/crypto.c
> +++ b/fs/ecryptfs/crypto.c
> @@ -1197,7 +1197,7 @@ static int ecryptfs_read_headers_virt(char *page_virt,
> } else
> set_default_header_data(crypt_stat);
> rc = ecryptfs_parse_packet_set(crypt_stat, (page_virt + offset),
> - ecryptfs_dentry);
> + PAGE_SIZE - offset, ecryptfs_dentry);
> out:
> return rc;
> }
> diff --git a/fs/ecryptfs/ecryptfs_kernel.h b/fs/ecryptfs/ecryptfs_kernel.h
> index f4f56a92bd56..7d2488a10b17 100644
> --- a/fs/ecryptfs/ecryptfs_kernel.h
> +++ b/fs/ecryptfs/ecryptfs_kernel.h
> @@ -580,7 +580,8 @@ int ecryptfs_generate_key_packet_set(char *dest_base,
> size_t *len, size_t max);
> int
> ecryptfs_parse_packet_set(struct ecryptfs_crypt_stat *crypt_stat,
> - unsigned char *src, struct dentry *ecryptfs_dentry);
> + unsigned char *src, size_t src_size,
> + struct dentry *ecryptfs_dentry);
> int ecryptfs_truncate(struct dentry *dentry, loff_t new_length);
> ssize_t
> ecryptfs_getxattr_lower(struct dentry *lower_dentry, struct inode *lower_inode,
> diff --git a/fs/ecryptfs/keystore.c b/fs/ecryptfs/keystore.c
> index ebebc9551f1f..888599739274 100644
> --- a/fs/ecryptfs/keystore.c
> +++ b/fs/ecryptfs/keystore.c
> @@ -894,6 +894,12 @@ ecryptfs_parse_tag_70_packet(char **filename, size_t *filename_size,
> "rc = [%d]\n", __func__, rc);
> goto out;
> }
> + if (s->parsed_tag_70_packet_size < (ECRYPTFS_SIG_SIZE + 2)) {
> + ecryptfs_printk(KERN_WARNING, "Invalid packet size [%zd]\n",
> + s->parsed_tag_70_packet_size);
> + rc = -EINVAL;
> + goto out;
> + }
> s->block_aligned_filename_size = (s->parsed_tag_70_packet_size
> - ECRYPTFS_SIG_SIZE - 1);
> if ((1 + s->packet_size_len + s->parsed_tag_70_packet_size)
> @@ -1537,7 +1543,7 @@ parse_tag_11_packet(unsigned char *data, unsigned char *contents,
> }
> (*packet_size) += length_size;
> (*tag_11_contents_size) = (body_size - 14);
> - if (unlikely((*packet_size) + body_size + 1 > max_packet_size)) {
> + if (unlikely((*packet_size) + body_size > max_packet_size)) {
> printk(KERN_ERR "Packet size exceeds max\n");
> rc = -EINVAL;
> goto out;
> @@ -1704,6 +1710,7 @@ decrypt_passphrase_encrypted_session_key(struct ecryptfs_auth_tok *auth_tok,
> * ecryptfs_parse_packet_set
> * @crypt_stat: The cryptographic context
> * @src: Virtual address of region of memory containing the packets
> + * @src_size: Size of the packet set buffer
> * @ecryptfs_dentry: The eCryptfs dentry associated with the packet set
> *
> * Get crypt_stat to have the file's session key if the requisite key
> @@ -1714,7 +1721,7 @@ decrypt_passphrase_encrypted_session_key(struct ecryptfs_auth_tok *auth_tok,
> * conditions.
> */
> int ecryptfs_parse_packet_set(struct ecryptfs_crypt_stat *crypt_stat,
> - unsigned char *src,
> + unsigned char *src, size_t src_size,
> struct dentry *ecryptfs_dentry)
> {
> size_t i = 0;
> @@ -1736,7 +1743,11 @@ int ecryptfs_parse_packet_set(struct ecryptfs_crypt_stat *crypt_stat,
> * added the our &auth_tok_list */
> next_packet_is_auth_tok_packet = 1;
> while (next_packet_is_auth_tok_packet) {
> - size_t max_packet_size = ((PAGE_SIZE - 8) - i);
> + size_t max_packet_size;
> +
> + if (i >= src_size)
> + break;
> + max_packet_size = src_size - i;
>
> switch (src[i]) {
> case ECRYPTFS_TAG_3_PACKET_TYPE:
> @@ -1751,12 +1762,16 @@ int ecryptfs_parse_packet_set(struct ecryptfs_crypt_stat *crypt_stat,
> goto out_wipe_list;
> }
> i += packet_size;
> + if (i > src_size) {
> + rc = -EIO;
> + goto out_wipe_list;
> + }
> rc = parse_tag_11_packet((unsigned char *)&src[i],
> sig_tmp_space,
> ECRYPTFS_SIG_SIZE,
> &tag_11_contents_size,
> &tag_11_packet_size,
> - max_packet_size);
> + src_size - i);
> if (rc) {
> ecryptfs_printk(KERN_ERR, "No valid "
> "(ecryptfs-specific) literal "
> @@ -1768,6 +1783,10 @@ int ecryptfs_parse_packet_set(struct ecryptfs_crypt_stat *crypt_stat,
> goto out_wipe_list;
> }
> i += tag_11_packet_size;
> + if (i > src_size) {
> + rc = -EIO;
> + goto out_wipe_list;
> + }
> if (ECRYPTFS_SIG_SIZE != tag_11_contents_size) {
> ecryptfs_printk(KERN_ERR, "Expected "
> "signature of size [%d]; "
> @@ -1793,6 +1812,10 @@ int ecryptfs_parse_packet_set(struct ecryptfs_crypt_stat *crypt_stat,
> goto out_wipe_list;
> }
> i += packet_size;
> + if (i > src_size) {
> + rc = -EIO;
> + goto out_wipe_list;
> + }
> crypt_stat->flags |= ECRYPTFS_ENCRYPTED;
> break;
> case ECRYPTFS_TAG_11_PACKET_TYPE:
> @@ -2480,4 +2503,3 @@ ecryptfs_add_global_auth_tok(struct ecryptfs_mount_crypt_stat *mount_crypt_stat,
> mutex_unlock(&mount_crypt_stat->global_auth_tok_list_mutex);
> return 0;
> }
> -
Please drop this unnecessary line removal.
Tyler
> --
> 2.51.0
>