[bug #29655] linkat() fails because __file_name_lookup_at() problems

[Emilio Pozuelo Monfort]

URL:
  <http://savannah.gnu.org/bugs/?29655>

                 Summary: linkat() fails because __file_name_lookup_at()
problems
                 Project: The GNU Hurd
            Submitted by: pochu
            Submitted on: Sat 24 Apr 2010 12:50:52 PM CEST
                Category: glibc
                Severity: 3 - Normal
                Priority: 5 - Normal
              Item Group: None
                  Status: None
                 Privacy: Public
             Assigned to: None
         Originator Name: 
        Originator Email: 
             Open/Closed: Open
         Discussion Lock: Any
         Reproducibility: None
              Size (loc): None
         Planned Release: None
                  Effort: 0.00
Wiki-like text discussion box: 

    _______________________________________________________

Details:

While looking at the coreutils test suite failures, I found that the new ln
fails miserably for hard links:

pochu@strauss:~$ touch a && /home/pochu/coreutils-8.4/src/ln a b
/home/pochu/coreutils-8.4/src/ln: creating hard link `b' => `a': Computer
bought the farm

Samuel helped me to debug the problem and found this:


00:09 <     youpi> one difference is that the newer ln uses linkat
00:20 <     youpi> seems like the AT_SYMLINK_FOLLOW is the culprit, making
__file_name_lookup_at return -1 with errno == EINVAL
00:20 <     youpi> while __file_name_lookup is supposed to return a port, not
-1
00:20 <     youpi> resulting to a bogus error handling
00:24 <     youpi> it seems to me like the logic in the hurd port is
inverted: susv says that by default linkat shouldn't dereference symlinks,
i.e. the flags passed by linkat should be O_NOLINK, but if.
                   AT_SYMLINK_FOLLOW is given, it should be cleared in
__file_name_lookup_at()
00:24 <     youpi> pochu: could you try this out?
00:25 <     youpi> and of course, btw, the EINVAL case should be fixed
00:26 <     youpi> (using _hurd_fail is really completely bogus here)
00:27 <     youpi> (just setting errno to EINVAL and returning MACH_PORT_NULL
should be fine)
00:35 <     pochu> youpi: so, __file_name_lookup_at() is returning -1 (bug),
which is then passed to __file_getlinknode(), which sets errno to
EMACH_SEND_INVALID_DEST, and __hurd_fail() change it to EIEIO?
00:35 <     youpi> probably yes


He was right. I first tested a one liner to return MACH_PORT_NULL instead of
-1 in __file_name_lookup_at(),

-    return __hurd_fail (EINVAL);
+    return (__hurd_fail (EINVAL), MACH_PORT_NULL);

 and with that ln failed with invalid argument:

pochu@strauss:~/coreutils-8.4/src$ LD_LIBRARY_PATH=/home/pochu/lib ./ln
/tmp/a /tmp/b
./ln: creating hard link `/tmp/b' => `/tmp/a': Invalid argument

And then I tried to fix the AT_SYMLINK_FOLLOW, and the attached patch make ln
work fine and the failing tests to pass.

Basically, __file_name_lookup_at() doesn't allow other AT_ flags than
AT_SYMLINK_NOFOLLOW, and it doesn't default to not follow links, as Samuel
said, see
http://www.opengroup.org/onlinepubs/9699919799/functions/linkat.html:
"If the AT_SYMLINK_FOLLOW flag is clear in the flag argument and the path1
argument names a symbolic link, a new link is created for the symbolic link
path1 and not its target."



    _______________________________________________________

File Attachments:


-------------------------------------------------------
Date: Sat 24 Apr 2010 12:50:52 PM CEST  Name: glibc-ln2.patch  Size: 2kB  
By: pochu
Fix linkat() and __file_name_lookup_at()
<http://savannah.gnu.org/bugs/download.php?file_id=20309>

    _______________________________________________________

Reply to this item at:

  <http://savannah.gnu.org/bugs/?29655>

_______________________________________________
  Message sent via/by Savannah
  http://savannah.gnu.org/