On 09/05/2013 03:35 PM, Linus Torvalds wrote:
No. Stop all these stupid games.
No d_lock, no trying to make d_name/d_len consistent.
No "compare d_name against d_iname".
No EINVAL.
No idiotic racy "let's fetch each byte one-by one and test them
against NUL", which is just racy and stupid.
Just do what I told you to do: *copy* the name one byte at a time, and
stop copying if you hit NUL. IOW, make prepend() just use "strncpy()"
instead of "memcpy()". You don't even need to check the end result -
if it's bogus, the sequence count will fix it.
No special cases. No games. No crap. Just make "prepend()" able to
handle the special case of "oops, the name length didn't match, but we
must not ever go past the end of the buffer".
We can optimize strncpy() to use word accesses if it ends up ever
being a performance issue. I suspect it never even shows up, but it's
not hard to do if if does.
Linus
It is not as simple as doing a strncpy(). The pathname was built from
the leaf up to the root, and from the end of buffer toward the
beginning. As it goes through the while loop, the buffer will look like:
" /c"
" /b/c"
"/a/b/c"
If the content of the string is unreliable, I have to do at least 2 passes:
1) Locate the end of the string and determine the actual length
2) Copy the whole string or byte-by-byte backward
That is why I am looking for the null byte. Using strncpy() alone won't
work. However, if the actual length doesn't match that of the dlen, the
name string will be invalid and there is no point in proceeding any further.
I also consider the possible, but unlikely, scenario that during a
rename operation, a short old name could be freed and replaced by a long
new name. The old name could then be allocated to another user filling
it up with non-NULL byte. If the buffer happen to be the end of
valid-to-invalid memory page boundary, the code may step over to an
invalid address by looking for the null byte. The current code probably
won't free the buffer while the RCU lock is being hold, but future code
change may make this assumption not valid. Blindly taking the d_lock may
be too expensive as the original code does. That is why I come up with
the internal vs. external dname check and take the lock only for
external dname for safety.
I can change the code to do just locating the end of string and copying
it backward, but not using strncpy().
-Longman
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
