On Tue, Sep 11, 2007 at 07:33:43PM +0200, Neil Brown wrote:
> On Monday September 10, [EMAIL PROTECTED] wrote:
> > On Mon, Sep 10, 2007 at 02:46:32PM -0400, J. Bruce Fields wrote:
> > > * This forceful removal will result in ugly /proc output if
> > > * somebody holds a file open that got deleted due to a rename.
> > > * We could be nicer about the deleted file, and let it show
> > > - * up under the name it got deleted rather than the name that
> > > - * deleted it.
> > > + * up under the name it had before it was deleted rather than
> > > + * under the original name of the file that was moved on top of it.
> >
> > By the way, on further examination of the code it doesn't actually do
> > what's described in the case where the target name is large and the
> > moved-from name is small. Instead, it reports random garbage (usually
> > part of a name left over from some other dentry?) as far as I can tell:
> >
> > from switch_names():
> >
> >
> > if (dname_external(target)) {
> > if (dname_external(dentry)) {
> > ...
> > } else {
> > /*
> > * dentry:internal, target:external. Steal target's
> > * storage and make target internal.
> > */
> > dentry->d_name.name = target->d_name.name;
> > target->d_name.name = target->d_iname;
> >
> > ... but target->d_iname could have anything in it, right?
>
> Right, but not relevant.
The effect of it is that the name reported in /proc/<pid>/fd/<fd> is
random garbage if you're holding the target file open. In quick tests,
I found that
touch abcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyz
tail -f abcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyz
touch foo
mv foo abcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyz
readlink /proc/<pid>/fd/<fd>
prints the initial portion of some other random name (often, not always,
"foo").
In theory I think that could disclose a little uninitialized kernel
memory, couldn't it? I don't know if there's any practical way that
could be exploited.
> The name "switch_names" is somewhat misleading. It is really
> "copyname" or similar. From the comment at the top:
>
> * When switching names, the actual string doesn't strictly have to
> * be preserved in the target - because we're dropping the target
> * anyway. As such, we can just do a simple memcpy() to copy over
> * the new name before we switch.
>
> so the apparent name of 'target' after the 'swap' is not important.
>
> The purpose of the assignment
> target->d_name.name = target->d_iname;
> is to make "dname_external(target)" false, that making "target
> internal" as the comment says.
Right. But it looks like the contents of the buffer target->d_iname
also need to be initialized in this case--I suppose somebody just didn't
want to perform a memcpy they thought was pointless--so the name
reported in /proc is undefined.
--b.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
