2008-12-11 19:16:50

by Duane Griffin

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

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

Signed-off-by: Duane Griffin <[email protected]>
---
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

2008-12-11 19:21:59

by Matthew Wilcox

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

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 <[email protected]>
> ---
> 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 [email protected]
> 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."

2008-12-11 20:25:03

by Duane Griffin

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

2008/12/11 Matthew Wilcox <[email protected]>:
> 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

2008-12-11 22:23:59

by Duane Griffin

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

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

Signed-off-by: Duane Griffin <[email protected]>
---

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