[fuse-discuss] webrev - fusefs changes (remove & rename)

[Frank.Hofmann at Sun.COM]

On Tue, 13 Nov 2007, Mark Phalan wrote:

>
> On Fri, 2007-11-02 at 20:09 +0100, Mark Phalan wrote:
>> On Fri, 2007-10-26 at 15:04 -0500, subra056 at umn.edu wrote:
>>> Webrev of remove and rename implementation, this completes all the vnode op
>>> functionality at the kernel making it feature complete, but still far from
>>> being production ready.
>>> http://www-users.cs.umn.edu/~subram/removeimpl/webrev/
>>>

Hmm, I'm wondering how the use of strlcpy() here:

2724         strptr = (char *)fri + sizeof (*fri);
2725         (void *) strlcpy(strptr, oldname, old_namelen);
2726         strptr[old_namelen] = '\0';
2727         strptr += old_namelen;
2728         (void *) strlcpy(strptr, newname, new_namelen);
2729         strptr[new_namelen] = '\0';

is beneficial / correct.
In any case, the explicit zeroing of the trailing byte is unnecessary with 
strl*(), whatever strl*() returns is guaranteed to be terminated.

What sort of overrun are you trying to prevent here ?

 	* if "oldname" / "newname" are not guaranteed to be terminated
 	  (i.e. if strlen() may run off its end), then strl*() might
 	  overrun no matter what. It's not safe to use then.

 	* if you're not trying to prevent an overrun on the source
 	  string, then the use of strlcpy() as shown is incorrect
 	  because you're not specifying the length of the target
 	  string.

(sorry, sensitive to these things - they're so easy to get wrong).

I do think you want either:

 	if (strlcpy(strptr, oldname, STRPTR_BUF_LEN) >= STRPTR_BUF_LEN ||
 	    strlcat(strptr, newname, STRPTR_BUF_LEN) >= STRPTR_BUF_LEN)
 		return (EFAULT);	/* or whatever */

or (in case it's a use of the buffer for two strings):

 	trailing_sz = STRPTR_BUF_LEN - old_namelen - 1;
 	if (strlcpy(strptr, oldname, STRPTR_BUF_LEN) >= STRPTR_BUF_LEN ||
 	    strlcpy(strptr + old_namelen, newname, trailing_sz) >= trailing_sz)
 		return (EFAULT);	/* or whatever */

but then, pls clarify ?

Thanks,
FrankH.