[prev in list] [next in list] [prev in thread] [next in thread] 

List:       v9fs-developer
Subject:    Re: [V9fs-developer] Readlink patch
From:       Martin Stava <martin.stava () gmail ! com>
Date:       2009-10-11 19:47:20
Message-ID: faaa6f890910111247h5b3faa8bhc54c47df1bd47507 () mail ! gmail ! com
[Download RAW message or body]

Hi,

 I do not know if you've looked on the patch, but unfortunately it is
incorrect. A suggested better version is in this email (the old
version didn't work in case the user provided buffer was not long
enough - it incorrectly appended null byte on a position of last char,
and thus broke the contract of the readlink method). However, I'm
still not sure this is 100% correct thing to do, I think readlink is
supposed to return buffer without last null byte in all cases, but we
do return last null byte (even the old version).. on the other hand it
is likely unspecified what is in the remaining part of the buffer, so
null character may be fine there ;):

diff -uprN -X linux-2.6.31.1-vanilla/Documentation/dontdiff
linux-2.6.31.1-vanilla/fs/9p/vfs_inode.c
linux-2.6.31.1/fs/9p/vfs_inode.c
--- linux-2.6.31.1-vanilla/fs/9p/vfs_inode.c	2009-09-24 17:45:25.000000000 +0200
+++ linux-2.6.31.1/fs/9p/vfs_inode.c	2009-10-03 14:04:42.000000000 +0200
@@ -939,7 +939,7 @@ static int v9fs_readlink(struct dentry *
 	P9_DPRINTK(P9_DEBUG_VFS,
 		"%s -> %s (%s)\n", dentry->d_name.name, st->extension, buffer);

-	retval = buflen;
+	retval = min(strlen(st->extension), (size_t)buflen);

 done:
 	kfree(st);
@@ -1007,7 +1007,7 @@ static void *v9fs_vfs_follow_link(struct
 			__putname(link);
 			link = ERR_PTR(len);
		} else
-			link[len] = 0;
+			link[min(len, PATH_MAX-1)] = 0;
 	}
 	nd_set_link(nd, link);


 Regards,
  Martin

On Mon, Oct 5, 2009 at 10:49 PM, Martin Stava <martin.stava@gmail.com> wrote:
> Hi,
>
>  As recommended, I'm resending the patch as a part of email.
>
>  Regards,
>  Martin
>
> diff -uprN -X linux-2.6.31.1-vanilla/Documentation/dontdiff
> linux-2.6.31.1-vanilla/fs/9p/vfs_inode.c
> linux-2.6.31.1/fs/9p/vfs_inode.c
> --- linux-2.6.31.1-vanilla/fs/9p/vfs_inode.c    2009-09-24 17:45:25.000000000 +0200
> +++ linux-2.6.31.1/fs/9p/vfs_inode.c    2009-10-03 14:04:42.000000000 +0200
> @@ -939,7 +939,8 @@ static int v9fs_readlink(struct dentry *
>        P9_DPRINTK(P9_DEBUG_VFS,
>                "%s -> %s (%s)\n", dentry->d_name.name, st->extension, buffer);
>
> -       retval = buflen;
> +       buffer[buflen-1] = '\0';
> +       retval = strlen(buffer);
>
>  done:
>        kfree(st);
> @@ -1006,8 +1007,7 @@ static void *v9fs_vfs_follow_link(struct
>                if (len < 0) {
>                        __putname(link);
>                        link = ERR_PTR(len);
> -               } else
> -                       link[len] = 0;
> +               }
>        }
>        nd_set_link(nd, link);
>
> On Sun, Oct 4, 2009 at 9:38 PM, Martin Stava <martin.stava@gmail.com> wrote:
>> Hi,
>>
>>  My next patch attempt, this time fixing bug in readlink.
>> v9fs_readlink was returning incorrect length of a read link and as a
>> result nulling of last byte in followlink touched memory outside of
>> the buffer.
>>
>>  Regards,
>>  Martin
>>
>

------------------------------------------------------------------------------
Come build with us! The BlackBerry(R) Developer Conference in SF, CA
is the only developer event you need to attend this year. Jumpstart your
developing skills, take BlackBerry mobile applications to market and stay 
ahead of the curve. Join us from November 9 - 12, 2009. Register now!
http://p.sf.net/sfu/devconference
_______________________________________________
V9fs-developer mailing list
V9fs-developer@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/v9fs-developer
[prev in list] [next in list] [prev in thread] [next in thread]