diff mbox

[Hurd] bind() fails when umask is 0000

Message ID 20140826212615.6E7F62C39C1@topped-with-meat.com
State New
Headers show

Commit Message

Roland McGrath Aug. 26, 2014, 9:26 p.m. UTC 
I assume you mean umask is 0666 or more, such that (0600 & ~umask) is 0.

The fix to use dir_lookup of "" to invoke the active translator seems
correct and orthogonal to the umask issue.  What that fixes is a race where
you could wind up with a different node than the one you just created, if
the by-name lookup happens after someone swooped in and renamed/linked
another node over the name bind just dir_link'd.  I don't know why I wrote
it the wrong way in the first place.  The EADDRINUSE check it does after
the fact seems insufficient.  Can you send a fix for just that issue
separately first?  (And be careful with your whitespace.)

The ifsock translator checks the underlying node for the access allowed to
enforce that you must have read access to the naming node to connect an
AF_LOCAL socket to a listener.  It's incidentally wrong that it's check for
read access, because POSIX says that the permission necessary for connect
is write access--to wit, on Linux you can connect to a socket you own with
mode 0200 but not to one with mode 0400 (I haven't tested other Unices but
I imagine they are the same).  But more relevant is that it feels like it
is using the wrong model for the check.  Why is it checking how the user
could have opened the underlying node, instead of checking how the user
actually did open the translated node?  i.e.:



Oh yeah, but diskfs/ext2fs is short-circuiting and not actually using the
ifsock translator.  The diskfs ifsock_getsockaddr already checks for write
access rather than read.  It too does it on the underlying node, but AFAICT
for S_IFSOCK diskfs never actually makes a distinction between the
underlying and translated node.  That's kind of grotty.  Is it really true
that a file port to an ext2fs S_IFSOCK node reads as having _HURD_IFSOCK as
translator whether or not you used O_NOTRANS to get the port?  When you
have a port to the translated node (i.e. opened without O_NOTRANS), then
you really should not be able to distinguish talking to /hurd/ifsock from
talking to a diskfs that is short-cutting it, but this is a way you can.

Then we'd still need to fix bind so it can get an O_WRITE-capable port to
the new translator.  But dir_lookup will usually deny O_WRITE open on an
S_IFSOCK node.  Hmm.  I still don't like changing the mode after the
mkfile, though it is probably indeed the easiest fix.

Bletch.  It looks like trivfs/ifsock doesn't constrain opening the virtual
node with O_READ and/or O_WRITE and/or O_EXEC, as diskfs does for opening
the underlying "shortcut" node.  So with diskfs/ext2fs you could just use
O_WRITE in the dir_mkfile call and then call ifsock_getsockaddr on the
underlying node port after setting the translator; and with trivfs/ifsock
you could do the dir_lookup with O_WRITE.  But each only works because it's
wrong!  If we made diskfs distinguish the underlying node from the virtual
node to fix the "appears to be infinite layers of _HURD_IFSOCK translator"
bug, then it wouldn't let you get an O_WRITE port on the virtual node.  If
we made ifsock refuse to open the virtual node with O_* bits set, then it
too wouldn't let you get an O_WRITE port on the virtual node.

Harumph.
diff mbox

Patch

diff --git a/trans/ifsock.c b/trans/ifsock.c
index 4ed6589..b2ad02b 100644
--- a/trans/ifsock.c
+++ b/trans/ifsock.c
@@ -1,5 +1,5 @@ 
 /* Server for S_IFSOCK nodes
-   Copyright (C) 1994, 1995, 2001, 02, 2006 Free Software Foundation
+   Copyright (C) 1994, 1995, 2001, 02, 2006, 2014 Free Software Foundation
 
    This program is free software; you can redistribute it and/or
    modify it under the terms of the GNU General Public License as
@@ -134,19 +134,15 @@  error_t
 S_ifsock_getsockaddr (struct trivfs_protid *cred,
 		      mach_port_t *address)
 {
-  int perms;
-  error_t err;
-
   if (!cred
       || cred->pi.bucket != port_bucket
       || cred->pi.class != node_class)
     return EOPNOTSUPP;
 
-  err = file_check_access (cred->realnode, &perms);
-  if (!err && !(perms & O_READ))
-    err = EACCES;
+  /* Require write access to connect, per POSIX.  */
+  if (!(cred->po->openmodes & O_WRITE))
+    return EACCES;
 
-  if (!err)
-    *address = address_port;
-  return err;
+  *address = address_port;
+  return 0;
 }