Jaunty SRU - #357345 - [PATCH] eCryptfs: Larger buffer for encrypted symlink targets

classic Classic list List threaded Threaded
3 messages Options
Reply | Threaded
Open this post in threaded view
|

Jaunty SRU - #357345 - [PATCH] eCryptfs: Larger buffer for encrypted symlink targets

Tim Gardner-7
The following changes since commit b7875cd8f03cdfb118f64b25d3739a254c7dc168:
  Stefan Bader (1):
        UBUNTU: Merge Ubuntu-2.6.28-11.42

are available in the git repository at:

  git://kernel.ubuntu.com/rtg/ubuntu-jaunty lp357345

Tyler Hicks (1):
      UBUNTU: SAUCE: (drop after 2.6.28) [PATCH] eCryptfs: Larger buffer for encrypted symlink targets

 fs/ecryptfs/inode.c |   24 ++++++++++++++++++------
 1 files changed, 18 insertions(+), 6 deletions(-)
From cdb33dfaf3dbf849656ebfe6833ab8cee697de7e Mon Sep 17 00:00:00 2001
From: Tyler Hicks <[hidden email]>
Date: Thu, 16 Apr 2009 18:35:37 -0500
Subject: [PATCH] UBUNTU: SAUCE: (drop after 2.6.28) [PATCH] eCryptfs: Larger buffer for encrypted symlink targets

Presumably this patch will make its way into 2.6.30.

Bug: #357345

When using filename encryption with eCryptfs, the value of the symlink
in the lower filesystem is encrypted and stored as a Tag 70 packet.
This results in a longer symlink target than if the target value wasn't
encrypted.

Users were reporting these messages in their syslog:

[ 45.653441] ecryptfs_parse_tag_70_packet: max_packet_size is [56]; real
packet size is [51]
[ 45.653444] ecryptfs_decode_and_decrypt_filename: Could not parse tag
70 packet from filename; copying through filename as-is

This was due to bufsiz, one the arguments in readlink(), being used to
when allocating the buffer passed to the lower inode's readlink().
That symlink target may be very large, but when decoded and decrypted,
could end up being smaller than bufsize.

To fix this, the buffer passed to the lower inode's readlink() will
always be PATH_MAX in size when filename encryption is enabled.  Any
necessary truncation occurs after the decoding and decrypting.

Signed-off-by: Tyler Hicks <[hidden email]>
Signed-off-by: Tim Gardner <[hidden email]>
---
 fs/ecryptfs/inode.c |   24 ++++++++++++++++++------
 1 files changed, 18 insertions(+), 6 deletions(-)

diff --git a/fs/ecryptfs/inode.c b/fs/ecryptfs/inode.c
index 82378fb..4b79e22 100644
--- a/fs/ecryptfs/inode.c
+++ b/fs/ecryptfs/inode.c
@@ -657,8 +657,9 @@ static int
 ecryptfs_readlink(struct dentry *dentry, char __user *buf, int bufsiz)
 {
  char *lower_buf;
+ size_t lower_bufsiz;
  struct dentry *lower_dentry;
- struct ecryptfs_crypt_stat *crypt_stat;
+ struct ecryptfs_mount_crypt_stat *mount_crypt_stat;
  char *plaintext_name;
  size_t plaintext_name_size;
  mm_segment_t old_fs;
@@ -670,12 +671,21 @@ ecryptfs_readlink(struct dentry *dentry, char __user *buf, int bufsiz)
  rc = -EINVAL;
  goto out;
  }
- crypt_stat = &ecryptfs_inode_to_private(dentry->d_inode)->crypt_stat;
+ mount_crypt_stat = &ecryptfs_superblock_to_private(
+ dentry->d_sb)->mount_crypt_stat;
+ /*
+ * If the lower filename is encrypted, it will result in a significantly
+ * longer name.  If needed, truncate the name after decode and decrypt.
+ */
+ if (mount_crypt_stat->flags & ECRYPTFS_GLOBAL_ENCRYPT_FILENAMES)
+ lower_bufsiz = PATH_MAX;
+ else
+ lower_bufsiz = bufsiz;
  /* Released in this function */
- lower_buf = kmalloc(bufsiz, GFP_KERNEL);
+ lower_buf = kmalloc(lower_bufsiz, GFP_KERNEL);
  if (lower_buf == NULL) {
  printk(KERN_ERR "%s: Out of memory whilst attempting to "
-       "kmalloc [%d] bytes\n", __func__, bufsiz);
+       "kmalloc [%d] bytes\n", __func__, lower_bufsiz);
  rc = -ENOMEM;
  goto out;
  }
@@ -683,7 +693,7 @@ ecryptfs_readlink(struct dentry *dentry, char __user *buf, int bufsiz)
  set_fs(get_ds());
  rc = lower_dentry->d_inode->i_op->readlink(lower_dentry,
    (char __user *)lower_buf,
-   bufsiz);
+   lower_bufsiz);
  set_fs(old_fs);
  if (rc >= 0) {
  rc = ecryptfs_decode_and_decrypt_filename(&plaintext_name,
@@ -696,7 +706,9 @@ ecryptfs_readlink(struct dentry *dentry, char __user *buf, int bufsiz)
  rc);
  goto out_free_lower_buf;
  }
- rc = copy_to_user(buf, plaintext_name, plaintext_name_size);
+ /* Check for bufsiz <= 0 done in sys_readlinkat() */
+ rc = copy_to_user(buf, plaintext_name,
+  min((unsigned) bufsiz, plaintext_name_size));
  if (rc)
  rc = -EFAULT;
  else
--
1.5.6.3


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

Re: Jaunty SRU - #357345 - [PATCH] eCryptfs: Larger buffer for encrypted symlink targets

Stefan Bader-2
Tim Gardner wrote:
> The following changes since commit b7875cd8f03cdfb118f64b25d3739a254c7dc168:
>   Stefan Bader (1):
>         UBUNTU: Merge Ubuntu-2.6.28-11.42
>
Looks right as far as I can tell. ACK

> are available in the git repository at:
>
>   git://kernel.ubuntu.com/rtg/ubuntu-jaunty lp357345
>
> Tyler Hicks (1):
>       UBUNTU: SAUCE: (drop after 2.6.28) [PATCH] eCryptfs: Larger buffer for encrypted symlink targets
>
>  fs/ecryptfs/inode.c |   24 ++++++++++++++++++------
>  1 files changed, 18 insertions(+), 6 deletions(-)
> From cdb33dfaf3dbf849656ebfe6833ab8cee697de7e Mon Sep 17 00:00:00 2001
> From: Tyler Hicks <[hidden email]>
> Date: Thu, 16 Apr 2009 18:35:37 -0500
> Subject: [PATCH] UBUNTU: SAUCE: (drop after 2.6.28) [PATCH] eCryptfs: Larger buffer for encrypted symlink targets
>
> Presumably this patch will make its way into 2.6.30.
>
> Bug: #357345
>
> When using filename encryption with eCryptfs, the value of the symlink
> in the lower filesystem is encrypted and stored as a Tag 70 packet.
> This results in a longer symlink target than if the target value wasn't
> encrypted.
>
> Users were reporting these messages in their syslog:
>
> [ 45.653441] ecryptfs_parse_tag_70_packet: max_packet_size is [56]; real
> packet size is [51]
> [ 45.653444] ecryptfs_decode_and_decrypt_filename: Could not parse tag
> 70 packet from filename; copying through filename as-is
>
> This was due to bufsiz, one the arguments in readlink(), being used to
> when allocating the buffer passed to the lower inode's readlink().
> That symlink target may be very large, but when decoded and decrypted,
> could end up being smaller than bufsize.
>
> To fix this, the buffer passed to the lower inode's readlink() will
> always be PATH_MAX in size when filename encryption is enabled.  Any
> necessary truncation occurs after the decoding and decrypting.
>
> Signed-off-by: Tyler Hicks <[hidden email]>
> Signed-off-by: Tim Gardner <[hidden email]>
> ---
>  fs/ecryptfs/inode.c |   24 ++++++++++++++++++------
>  1 files changed, 18 insertions(+), 6 deletions(-)
>
> diff --git a/fs/ecryptfs/inode.c b/fs/ecryptfs/inode.c
> index 82378fb..4b79e22 100644
> --- a/fs/ecryptfs/inode.c
> +++ b/fs/ecryptfs/inode.c
> @@ -657,8 +657,9 @@ static int
>  ecryptfs_readlink(struct dentry *dentry, char __user *buf, int bufsiz)
>  {
>   char *lower_buf;
> + size_t lower_bufsiz;
>   struct dentry *lower_dentry;
> - struct ecryptfs_crypt_stat *crypt_stat;
> + struct ecryptfs_mount_crypt_stat *mount_crypt_stat;
>   char *plaintext_name;
>   size_t plaintext_name_size;
>   mm_segment_t old_fs;
> @@ -670,12 +671,21 @@ ecryptfs_readlink(struct dentry *dentry, char __user *buf, int bufsiz)
>   rc = -EINVAL;
>   goto out;
>   }
> - crypt_stat = &ecryptfs_inode_to_private(dentry->d_inode)->crypt_stat;
> + mount_crypt_stat = &ecryptfs_superblock_to_private(
> + dentry->d_sb)->mount_crypt_stat;
> + /*
> + * If the lower filename is encrypted, it will result in a significantly
> + * longer name.  If needed, truncate the name after decode and decrypt.
> + */
> + if (mount_crypt_stat->flags & ECRYPTFS_GLOBAL_ENCRYPT_FILENAMES)
> + lower_bufsiz = PATH_MAX;
> + else
> + lower_bufsiz = bufsiz;
>   /* Released in this function */
> - lower_buf = kmalloc(bufsiz, GFP_KERNEL);
> + lower_buf = kmalloc(lower_bufsiz, GFP_KERNEL);
>   if (lower_buf == NULL) {
>   printk(KERN_ERR "%s: Out of memory whilst attempting to "
> -       "kmalloc [%d] bytes\n", __func__, bufsiz);
> +       "kmalloc [%d] bytes\n", __func__, lower_bufsiz);
>   rc = -ENOMEM;
>   goto out;
>   }
> @@ -683,7 +693,7 @@ ecryptfs_readlink(struct dentry *dentry, char __user *buf, int bufsiz)
>   set_fs(get_ds());
>   rc = lower_dentry->d_inode->i_op->readlink(lower_dentry,
>     (char __user *)lower_buf,
> -   bufsiz);
> +   lower_bufsiz);
>   set_fs(old_fs);
>   if (rc >= 0) {
>   rc = ecryptfs_decode_and_decrypt_filename(&plaintext_name,
> @@ -696,7 +706,9 @@ ecryptfs_readlink(struct dentry *dentry, char __user *buf, int bufsiz)
>   rc);
>   goto out_free_lower_buf;
>   }
> - rc = copy_to_user(buf, plaintext_name, plaintext_name_size);
> + /* Check for bufsiz <= 0 done in sys_readlinkat() */
> + rc = copy_to_user(buf, plaintext_name,
> +  min((unsigned) bufsiz, plaintext_name_size));
>   if (rc)
>   rc = -EFAULT;
>   else


--

When all other means of communication fail, try words!



--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

Re: Jaunty SRU - #357345 - [PATCH] eCryptfs: Larger buffer for encrypted symlink targets

Amit Kucheria-6
In reply to this post by Tim Gardner-7
On Fri, Apr 17, 2009 at 07:28:37AM -0600, Tim Gardner wrote:

> The following changes since commit b7875cd8f03cdfb118f64b25d3739a254c7dc168:
>   Stefan Bader (1):
>         UBUNTU: Merge Ubuntu-2.6.28-11.42
>
> are available in the git repository at:
>
>   git://kernel.ubuntu.com/rtg/ubuntu-jaunty lp357345
>
> Tyler Hicks (1):
>       UBUNTU: SAUCE: (drop after 2.6.28) [PATCH] eCryptfs: Larger buffer for encrypted symlink targets
>
>  fs/ecryptfs/inode.c |   24 ++++++++++++++++++------
>  1 files changed, 18 insertions(+), 6 deletions(-)
> From cdb33dfaf3dbf849656ebfe6833ab8cee697de7e Mon Sep 17 00:00:00 2001
> From: Tyler Hicks <[hidden email]>
> Date: Thu, 16 Apr 2009 18:35:37 -0500
> Subject: [PATCH] UBUNTU: SAUCE: (drop after 2.6.28) [PATCH] eCryptfs: Larger buffer for encrypted symlink targets
>
> Presumably this patch will make its way into 2.6.30.
>
> Bug: #357345

ACK.

--
----------------------------------------------------------------------
Amit Kucheria, Kernel Engineer || [hidden email]
----------------------------------------------------------------------

--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team