[PATCH 0/11] make link target handling more robust

[PATCH 0/11] make link target handling more robust

[Duane Griffin]

These patches fix potential bugs associated with link target handling,
primarily by NULL-terminating names read from disk.

The ext2 and generic page_getlink changes have been tested by listing
corrupted links and creating and listing valid ones. Other patches have
been compile-tested only.

Cheers,
Duane.

[PATCH] ext2: ensure link targets are NULL-terminated

[Duane Griffin]

Ensure link targets are NULL-terminated, even if corrupted on-disk.

Signed-off-by: Duane Griffin <duaneg@dghda.com>
---
 fs/ext2/symlink.c |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/fs/ext2/symlink.c b/fs/ext2/symlink.c
index 4e2426e..9b164ba 100644
--- a/fs/ext2/symlink.c
+++ b/fs/ext2/symlink.c
@@ -24,7 +24,9 @@
 static void *ext2_follow_link(struct dentry *dentry, struct nameidata *nd)
 {
 	struct ext2_inode_info *ei = EXT2_I(dentry->d_inode);
-	nd_set_link(nd, (char *)ei->i_data);
+	char *link = (char *) ei->i_data;
+	link[sizeof(ei->i_data) - 1] = '\0';
+	nd_set_link(nd, link);
 	return NULL;
 }
 
-- 
1.6.0.4

Re: [PATCH] ext2: ensure link targets are NULL-terminated

[Matthew Wilcox]

On Thu, Dec 11, 2008 at 07:16:28PM +0000, Duane Griffin wrote:
> Ensure link targets are NULL-terminated, even if corrupted on-disk.

FWIW, that's spelled 'NUL' -- NULL is a pointer value, NUL is an ASCII
code.

> Signed-off-by: Duane Griffin <duaneg@dghda.com>
> ---
>  fs/ext2/symlink.c |    4 +++-
>  1 files changed, 3 insertions(+), 1 deletions(-)
> 
> diff --git a/fs/ext2/symlink.c b/fs/ext2/symlink.c
> index 4e2426e..9b164ba 100644
> --- a/fs/ext2/symlink.c
> +++ b/fs/ext2/symlink.c
> @@ -24,7 +24,9 @@
>  static void *ext2_follow_link(struct dentry *dentry, struct nameidata *nd)
>  {
>  	struct ext2_inode_info *ei = EXT2_I(dentry->d_inode);
> -	nd_set_link(nd, (char *)ei->i_data);
> +	char *link = (char *) ei->i_data;
> +	link[sizeof(ei->i_data) - 1] = '\0';
> +	nd_set_link(nd, link);
>  	return NULL;
>  }
>  
> -- 
> 1.6.0.4
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

-- 
Matthew Wilcox				Intel Open Source Technology Centre
"Bill, look, we understand that you're interested in selling us this
operating system, but compare it to ours.  We can't possibly take such
a retrograde step."

Re: [PATCH] eCryptfs: check readlink result was not an error before using it

[Michael Halcrow]

On Thu, Dec 11, 2008 at 07:16:26PM +0000, Duane Griffin wrote:
> The result from readlink is being used to index into the link name
> buffer without checking whether it is a valid length. If readlink
> returns an error this will fault or cause memory corruption.
> 
> Signed-off-by: Duane Griffin <duaneg@dghda.com>

Acked-by: Michael Halcrow <mhalcrow@us.ibm.com>

> ---
>  fs/ecryptfs/inode.c |    3 ++-
>  1 files changed, 2 insertions(+), 1 deletions(-)
> 
> diff --git a/fs/ecryptfs/inode.c b/fs/ecryptfs/inode.c
> index 89209f0..5e78fc1 100644
> --- a/fs/ecryptfs/inode.c
> +++ b/fs/ecryptfs/inode.c
> @@ -673,10 +673,11 @@ static void *ecryptfs_follow_link(struct dentry *dentry, struct nameidata *nd)
>  	ecryptfs_printk(KERN_DEBUG, "Calling readlink w/ "
>  			"dentry->d_name.name = [%s]\n", dentry->d_name.name);
>  	rc = dentry->d_inode->i_op->readlink(dentry, (char __user *)buf, len);
> -	buf[rc] = '\0';
>  	set_fs(old_fs);
>  	if (rc < 0)
>  		goto out_free;
> +	else
> +		buf[rc] = '\0';
>  	rc = 0;
>  	nd_set_link(nd, buf);
>  	goto out;

Re: [PATCH] JFS: ensure link targets are NULL-terminated

[Dave Kleikamp]

On Thu, 2008-12-11 at 19:16 +0000, Duane Griffin wrote:
> Ensure link targets are NULL-terminated, even if corrupted on-disk.
> 
> Signed-off-by: Duane Griffin <duaneg@dghda.com>
> ---
>  fs/jfs/symlink.c |    2 ++
>  1 files changed, 2 insertions(+), 0 deletions(-)
> 
> diff --git a/fs/jfs/symlink.c b/fs/jfs/symlink.c
> index 4af1a05..f5f1b5d 100644
> --- a/fs/jfs/symlink.c
> +++ b/fs/jfs/symlink.c
> @@ -18,6 +18,7 @@
>  
>  #include <linux/fs.h>
>  #include <linux/namei.h>
> +#include "jfs_filsys.h"
>  #include "jfs_incore.h"
>  #include "jfs_inode.h"
>  #include "jfs_xattr.h"
> @@ -25,6 +26,7 @@
>  static void *jfs_follow_link(struct dentry *dentry, struct nameidata *nd)
>  {
>  	char *s = JFS_IP(dentry->d_inode)->i_inline;
> +	s[IDATASIZE - 1] = '\0';
>  	nd_set_link(nd, s);
>  	return NULL;
>  }

Thanks.

I came up with an alternate patch that I like slightly better.  This one
null-terminates the string when the inode is read rather than every time
the link is followed, and at the proper place (i_size).  It's not a big
deal, since the symlink will be corrupted either way.  What do you think
of this patch?

Signed-off-by: Dave Kleikamp <shaggy@linux.vnet.ibm.com>

diff --git a/fs/jfs/inode.c b/fs/jfs/inode.c
index 2103397..b00ee9f 100644
--- a/fs/jfs/inode.c
+++ b/fs/jfs/inode.c
@@ -59,8 +59,14 @@ struct inode *jfs_iget(struct super_block *sb, unsigned long ino)
 		if (inode->i_size >= IDATASIZE) {
 			inode->i_op = &page_symlink_inode_operations;
 			inode->i_mapping->a_ops = &jfs_aops;
-		} else
+		} else {
 			inode->i_op = &jfs_symlink_inode_operations;
+			/*
+			 * The inline data should be null-terminated, but
+			 * don't let on-disk corruption crash the kernel
+			 */
+			JFS_IP(inode)->i_inline[inode->i_size] = '\0';
+		}
 	} else {
 		inode->i_op = &jfs_file_inode_operations;
 		init_special_inode(inode, inode->i_mode, inode->i_rdev);

-- 
David Kleikamp
IBM Linux Technology Center

Re: [PATCH] ext2: ensure link targets are NULL-terminated

[Duane Griffin]

2008/12/11 Matthew Wilcox <matthew@wil.cx>:
> On Thu, Dec 11, 2008 at 07:16:28PM +0000, Duane Griffin wrote:
>> Ensure link targets are NULL-terminated, even if corrupted on-disk.
>
> FWIW, that's spelled 'NUL' -- NULL is a pointer value, NUL is an ASCII
> code.

Good point, thanks. I'll try to remember for the future!

Cheers,
Duane.

-- 
"I never could learn to drink that blood and call it wine" - Bob Dylan

Re: [PATCH] JFS: ensure link targets are NULL-terminated

[Duane Griffin]

2008/12/11 Dave Kleikamp <shaggy@linux.vnet.ibm.com>:
> Thanks.
>
> I came up with an alternate patch that I like slightly better.  This one
> null-terminates the string when the inode is read rather than every time
> the link is followed, and at the proper place (i_size).  It's not a big
> deal, since the symlink will be corrupted either way.  What do you think
> of this patch?
>
> Signed-off-by: Dave Kleikamp <shaggy@linux.vnet.ibm.com>
>
> diff --git a/fs/jfs/inode.c b/fs/jfs/inode.c
> index 2103397..b00ee9f 100644
> --- a/fs/jfs/inode.c
> +++ b/fs/jfs/inode.c
> @@ -59,8 +59,14 @@ struct inode *jfs_iget(struct super_block *sb, unsigned long ino)
>                if (inode->i_size >= IDATASIZE) {
>                        inode->i_op = &page_symlink_inode_operations;
>                        inode->i_mapping->a_ops = &jfs_aops;
> -               } else
> +               } else {
>                        inode->i_op = &jfs_symlink_inode_operations;
> +                       /*
> +                        * The inline data should be null-terminated, but
> +                        * don't let on-disk corruption crash the kernel
> +                        */
> +                       JFS_IP(inode)->i_inline[inode->i_size] = '\0';
> +               }
>        } else {
>                inode->i_op = &jfs_file_inode_operations;
>                init_special_inode(inode, inode->i_mode, inode->i_rdev);

Yes, I prefer your version too :)

I'll rework the other patches similarly, where appropriate. Thanks!

Cheers,
Duane.

-- 
"I never could learn to drink that blood and call it wine" - Bob Dylan

[PATCH, v2] ext2: ensure link targets are NUL-terminated

[Duane Griffin]

Ensure link targets are NUL-terminated, even if corrupted on-disk.

Signed-off-by: Duane Griffin <duaneg@dghda.com>
---

V2: terminate when the link is read instead of every time it is
followed, as suggested by Dave Kleikamp.

diff --git a/fs/ext2/inode.c b/fs/ext2/inode.c
index 7658b33..9bacf35 100644
--- a/fs/ext2/inode.c
+++ b/fs/ext2/inode.c
@@ -1286,9 +1286,10 @@ struct inode *ext2_iget (struct super_block *sb, unsigned long ino)
 		else
 			inode->i_mapping->a_ops = &ext2_aops;
 	} else if (S_ISLNK(inode->i_mode)) {
-		if (ext2_inode_is_fast_symlink(inode))
+		if (ext2_inode_is_fast_symlink(inode)) {
 			inode->i_op = &ext2_fast_symlink_inode_operations;
-		else {
+			((char *) ei->i_data)[inode->i_size] = '\0';
+		} else {
 			inode->i_op = &ext2_symlink_inode_operations;
 			if (test_opt(inode->i_sb, NOBH))
 				inode->i_mapping->a_ops = &ext2_nobh_aops;
-- 
"I never could learn to drink that blood and call it wine" - Bob Dylan

[PATCH, v2] ext3: ensure link targets are NULL-terminated

[Duane Griffin]

Ensure link targets are NUL-terminated, even if corrupted on-disk.

Signed-off-by: Duane Griffin <duaneg@dghda.com>
---

V2: terminate when the link is read instead of every time it is
followed, as suggested by Dave Kleikamp.

diff --git a/fs/ext3/inode.c b/fs/ext3/inode.c
index f8424ad..c168781 100644
--- a/fs/ext3/inode.c
+++ b/fs/ext3/inode.c
@@ -2817,9 +2817,10 @@ struct inode *ext3_iget(struct super_block *sb, unsigned long ino)
 		inode->i_op = &ext3_dir_inode_operations;
 		inode->i_fop = &ext3_dir_operations;
 	} else if (S_ISLNK(inode->i_mode)) {
-		if (ext3_inode_is_fast_symlink(inode))
+		if (ext3_inode_is_fast_symlink(inode)) {
 			inode->i_op = &ext3_fast_symlink_inode_operations;
-		else {
+			((char *) ei->i_data)[inode->i_size] = '\0';
+		} else {
 			inode->i_op = &ext3_symlink_inode_operations;
 			ext3_set_aops(inode);
 		}
-- 
"I never could learn to drink that blood and call it wine" - Bob Dylan

[PATCH, v2] ext4: ensure link targets are NUL-terminated

[Duane Griffin]

Ensure link targets are NUL-terminated, even if corrupted on-disk.

Signed-off-by: Duane Griffin <duaneg@dghda.com>
---

V2: terminate when the link is read instead of every time it is
followed, as suggested by Dave Kleikamp.

diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
index ca88060..e70122e 100644
--- a/fs/ext4/inode.c
+++ b/fs/ext4/inode.c
@@ -4200,9 +4200,10 @@ struct inode *ext4_iget(struct super_block *sb, unsigned long ino)
 		inode->i_op = &ext4_dir_inode_operations;
 		inode->i_fop = &ext4_dir_operations;
 	} else if (S_ISLNK(inode->i_mode)) {
-		if (ext4_inode_is_fast_symlink(inode))
+		if (ext4_inode_is_fast_symlink(inode)) {
 			inode->i_op = &ext4_fast_symlink_inode_operations;
-		else {
+			((char *) ei->i_data)[inode->i_size] = '\0';
+		} else {
 			inode->i_op = &ext4_symlink_inode_operations;
 			ext4_set_aops(inode);
 		}
-- 
"I never could learn to drink that blood and call it wine" - Bob Dylan

[PATCH, v2] sysv: ensure link targets are NUL-terminated

[Duane Griffin]

Ensure link targets are NUL-terminated, even if corrupted on-disk.

Signed-off-by: Duane Griffin <duaneg@dghda.com>
---

V2: terminate when the link is read instead of every time it is
followed, as suggested by Dave Kleikamp.

diff --git a/fs/sysv/inode.c b/fs/sysv/inode.c
index df0d435..1511228 100644
--- a/fs/sysv/inode.c
+++ b/fs/sysv/inode.c
@@ -163,8 +163,10 @@ void sysv_set_inode(struct inode *inode, dev_t rdev)
 		if (inode->i_blocks) {
 			inode->i_op = &sysv_symlink_inode_operations;
 			inode->i_mapping->a_ops = &sysv_aops;
-		} else
+		} else {
 			inode->i_op = &sysv_fast_symlink_inode_operations;
+			((char *) SYSV_I(inode)->i_data)[inode->i_size] = '\0';
+		}
 	} else
 		init_special_inode(inode, inode->i_mode, rdev);
 }
-- 
"I never could learn to drink that blood and call it wine" - Bob Dylan

[PATCH, v2] ufs: ensure link targets are NUL-terminated

[Duane Griffin]

Ensure link targets are NUL-terminated, even if corrupted on-disk.

Signed-off-by: Duane Griffin <duaneg@dghda.com>
---

V2: terminate when the link is read instead of every time it is
followed, as suggested by Dave Kleikamp.

diff --git a/fs/ufs/inode.c b/fs/ufs/inode.c
index 39f8778..a22f64d 100644
--- a/fs/ufs/inode.c
+++ b/fs/ufs/inode.c
@@ -606,9 +606,11 @@ static void ufs_set_inode_ops(struct inode *inode)
 		inode->i_fop = &ufs_dir_operations;
 		inode->i_mapping->a_ops = &ufs_aops;
 	} else if (S_ISLNK(inode->i_mode)) {
-		if (!inode->i_blocks)
+		if (!inode->i_blocks) {
+			char *link = UFS_I(inode)->i_u1.i_symlink;
 			inode->i_op = &ufs_fast_symlink_inode_operations;
-		else {
+			link[inode->i_size] = '\0';
+		} else {
 			inode->i_op = &page_symlink_inode_operations;
 			inode->i_mapping->a_ops = &ufs_aops;
 		}
-- 
"I never could learn to drink that blood and call it wine" - Bob Dylan

[PATCH, v2] freevxfs: ensure link targets are NUL-terminated

[Duane Griffin]

Ensure link targets are NUL-terminated, even if corrupted on-disk.

Signed-off-by: Duane Griffin <duaneg@dghda.com>
---

V2: terminate when the link is read instead of every time it is
followed, as suggested by Dave Kleikamp.

diff --git a/fs/freevxfs/vxfs_inode.c b/fs/freevxfs/vxfs_inode.c
index 9f3f2ce..03a6ea5 100644
--- a/fs/freevxfs/vxfs_inode.c
+++ b/fs/freevxfs/vxfs_inode.c
@@ -325,8 +325,10 @@ vxfs_iget(struct super_block *sbp, ino_t ino)
 		if (!VXFS_ISIMMED(vip)) {
 			ip->i_op = &page_symlink_inode_operations;
 			ip->i_mapping->a_ops = &vxfs_aops;
-		} else
+		} else {
 			ip->i_op = &vxfs_immed_symlink_iops;
+			vip->vii_immed.vi_immed[ip->i_size] = '\0';
+		}
 	} else
 		init_special_inode(ip, ip->i_mode, old_decode_dev(vip->vii_rdev));
 
-- 
"I never could learn to drink that blood and call it wine" - Bob Dylan

Re: [PATCH] eCryptfs: check readlink result was not an error before using it

[Tim Gardner]

Michael Halcrow wrote:
> On Thu, Dec 11, 2008 at 07:16:26PM +0000, Duane Griffin wrote:
>> The result from readlink is being used to index into the link name
>> buffer without checking whether it is a valid length. If readlink
>> returns an error this will fault or cause memory corruption.
>>
>> Signed-off-by: Duane Griffin <duaneg@dghda.com>
> 
> Acked-by: Michael Halcrow <mhalcrow@us.ibm.com>
> 
>> ---
>>  fs/ecryptfs/inode.c |    3 ++-
>>  1 files changed, 2 insertions(+), 1 deletions(-)
>>
>> diff --git a/fs/ecryptfs/inode.c b/fs/ecryptfs/inode.c
>> index 89209f0..5e78fc1 100644
>> --- a/fs/ecryptfs/inode.c
>> +++ b/fs/ecryptfs/inode.c
>> @@ -673,10 +673,11 @@ static void *ecryptfs_follow_link(struct dentry *dentry, struct nameidata *nd)
>>  	ecryptfs_printk(KERN_DEBUG, "Calling readlink w/ "
>>  			"dentry->d_name.name = [%s]\n", dentry->d_name.name);
>>  	rc = dentry->d_inode->i_op->readlink(dentry, (char __user *)buf, len);
>> -	buf[rc] = '\0';
>>  	set_fs(old_fs);
>>  	if (rc < 0)
>>  		goto out_free;
>> +	else
>> +		buf[rc] = '\0';
>>  	rc = 0;
>>  	nd_set_link(nd, buf);
>>  	goto out;
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
> 

Please add 'Cc: stable@kernel.org' in the commit message. This looks
like a good candidate.

rtg
-- 
Tim Gardner tim.gardner@canonical.com

Re: [PATCH, v2] ext3: ensure link targets are NULL-terminated

[Andrew Morton]

On Thu, 11 Dec 2008 22:26:05 +0000 "Duane Griffin" <duaneg@dghda.com> wrote:

> Ensure link targets are NUL-terminated, even if corrupted on-disk.
> 
> Signed-off-by: Duane Griffin <duaneg@dghda.com>
> ---
> 
> V2: terminate when the link is read instead of every time it is
> followed, as suggested by Dave Kleikamp.
> 
> diff --git a/fs/ext3/inode.c b/fs/ext3/inode.c
> index f8424ad..c168781 100644
> --- a/fs/ext3/inode.c
> +++ b/fs/ext3/inode.c
> @@ -2817,9 +2817,10 @@ struct inode *ext3_iget(struct super_block *sb, unsigned long ino)
>  		inode->i_op = &ext3_dir_inode_operations;
>  		inode->i_fop = &ext3_dir_operations;
>  	} else if (S_ISLNK(inode->i_mode)) {
> -		if (ext3_inode_is_fast_symlink(inode))
> +		if (ext3_inode_is_fast_symlink(inode)) {
>  			inode->i_op = &ext3_fast_symlink_inode_operations;
> -		else {
> +			((char *) ei->i_data)[inode->i_size] = '\0';
> +		} else {
>  			inode->i_op = &ext3_symlink_inode_operations;
>  			ext3_set_aops(inode);
>  		}

Really?  The ext2 on-disk format requires that the fast symlink be
null-terminated on disk?  Even though the length is already in i_size?

It seems that's true.  How un-ext2-like.

ext2 and ext4 need the same fix, yes?

Re: [PATCH, v2] ext3: ensure link targets are NULL-terminated

[Duane Griffin]

2008/12/12 Andrew Morton <akpm@linux-foundation.org>:
> Really?  The ext2 on-disk format requires that the fast symlink be
> null-terminated on disk?  Even though the length is already in i_size?
>
> It seems that's true.  How un-ext2-like.
>
> ext2 and ext4 need the same fix, yes?

Yes. I've sent them out already, but thanks to a monumental cock-up
with the CCs they may not have made it to the list. I'll check and
resend to real addresses if necessary.

Cheers,
Duane.

-- 
"I never could learn to drink that blood and call it wine" - Bob Dylan

Re: [PATCH, v2] ext3: ensure link targets are NULL-terminated

[Duane Griffin]

2008/12/12 Duane Griffin <duaneg@dghda.com>:
> 2008/12/12 Andrew Morton <akpm@linux-foundation.org>:
>> Really?  The ext2 on-disk format requires that the fast symlink be
>> null-terminated on disk?  Even though the length is already in i_size?
>>
>> It seems that's true.  How un-ext2-like.
>>
>> ext2 and ext4 need the same fix, yes?
>
> Yes. I've sent them out already, but thanks to a monumental cock-up
> with the CCs they may not have made it to the list. I'll check and
> resend to real addresses if necessary.

Seems they did make it:
http://marc.info/?l=linux-kernel&m=122903437006575&w=2
http://marc.info/?l=linux-kernel&m=122903451306859&w=2

Cheers,
Duane.

-- 
"I never could learn to drink that blood and call it wine" - Bob Dylan

Re: [PATCH, v2] ufs: ensure link targets are NUL-terminated

[Evgeniy Dushistov]

On Thu, Dec 11, 2008 at 10:36:08PM +0000, Duane Griffin wrote:
> Ensure link targets are NUL-terminated, even if corrupted on-disk.
> 
> Signed-off-by: Duane Griffin <duaneg@dghda.com>
> ---
> 
> V2: terminate when the link is read instead of every time it is
> followed, as suggested by Dave Kleikamp.
> 
> diff --git a/fs/ufs/inode.c b/fs/ufs/inode.c
> index 39f8778..a22f64d 100644
> --- a/fs/ufs/inode.c
> +++ b/fs/ufs/inode.c
> @@ -606,9 +606,11 @@ static void ufs_set_inode_ops(struct inode *inode)
>  		inode->i_fop = &ufs_dir_operations;
>  		inode->i_mapping->a_ops = &ufs_aops;
>  	} else if (S_ISLNK(inode->i_mode)) {
> -		if (!inode->i_blocks)
> +		if (!inode->i_blocks) {
> +			char *link = UFS_I(inode)->i_u1.i_symlink;
>  			inode->i_op = &ufs_fast_symlink_inode_operations;
> -		else {
> +			link[inode->i_size] = '\0';
> +		} else {
>  			inode->i_op = &page_symlink_inode_operations;
>  			inode->i_mapping->a_ops = &ufs_aops;
>  		}

if we talk about corrupted file system, may be also ensure
that inode->i_size <= sizeof(i_u1)-1 before write '\0' to it.

-- 
/Evgeniy

Re: [PATCH, v2] sysv: ensure link targets are NUL-terminated

[Al Viro]

On Thu, Dec 11, 2008 at 10:35:32PM +0000, Duane Griffin wrote:
> Ensure link targets are NUL-terminated, even if corrupted on-disk.
> 
> Signed-off-by: Duane Griffin <duaneg@dghda.com>
> ---
> 
> V2: terminate when the link is read instead of every time it is
> followed, as suggested by Dave Kleikamp.
> 
> diff --git a/fs/sysv/inode.c b/fs/sysv/inode.c
> index df0d435..1511228 100644
> --- a/fs/sysv/inode.c
> +++ b/fs/sysv/inode.c
> @@ -163,8 +163,10 @@ void sysv_set_inode(struct inode *inode, dev_t rdev)
>  		if (inode->i_blocks) {
>  			inode->i_op = &sysv_symlink_inode_operations;
>  			inode->i_mapping->a_ops = &sysv_aops;
> -		} else
> +		} else {
>  			inode->i_op = &sysv_fast_symlink_inode_operations;
> +			((char *) SYSV_I(inode)->i_data)[inode->i_size] = '\0';

If you do it that way, you want to verify that i_size is bounded.  Better yet,
add a helper for that (taking void *, len, max_len)

Re: [PATCH, v2] ext3: ensure link targets are NULL-terminated

[Andrew Morton]

On Fri, 12 Dec 2008 10:19:20 +0000
"Duane Griffin" <duaneg@dghda.com> wrote:

> 2008/12/12 Duane Griffin <duaneg@dghda.com>:
> > 2008/12/12 Andrew Morton <akpm@linux-foundation.org>:
> >> Really?  The ext2 on-disk format requires that the fast symlink be
> >> null-terminated on disk?  Even though the length is already in i_size?
> >>
> >> It seems that's true.  How un-ext2-like.
> >>
> >> ext2 and ext4 need the same fix, yes?
> >
> > Yes. I've sent them out already, but thanks to a monumental cock-up
> > with the CCs they may not have made it to the list. I'll check and
> > resend to real addresses if necessary.
> 
> Seems they did make it:
> http://marc.info/?l=linux-kernel&m=122903437006575&w=2
> http://marc.info/?l=linux-kernel&m=122903451306859&w=2
> 

OK, thanks, it seems I was sneakily not cc'ed ;)

As Al points out, the code which you implemented is still vulnerable to
on-disk corruption: bad values of i_size will cause the kernel to write
a zero byte to any address within the entire CPU address range.