[Hurd] bind() fails when umask is 0777
Commit Message
Roland McGrath, le Tue 26 Aug 2014 14:26:15 -0700, a écrit :
> I assume you mean umask is 0666 or more, such that (0600 & ~umask) is 0.
Oops, sure, yes :)
> The fix to use dir_lookup of "" to invoke the active translator seems
> correct and orthogonal to the umask issue. [...] Can you send a fix
> for just that issue separately first?
Yes, here it is.
Samuel
2014-08-27 Samuel Thibault <samuel.thibault@ens-lyon.org>
Simplify atomicity of socket creation in bind.
* sysdeps/mach/hurd/bind.c (__bind): Use dir_lookup(node, "") instead of
looking up the name after linking the file.
---
bind.c | 48 +++++++++++++++++++++++++-----------------------
1 file changed, 25 insertions(+), 23 deletions(-)
Comments
> + if (! err)
> + if (doretry != FS_RETRY_NORMAL || retryname[0] != '\0')
> + err = EADDRINUSE;
Please use a simple && expression rather than a nested if here.
> + /* Get the address port. */
> + err = __ifsock_getsockaddr (ifsock, &aport);
> + if (err == MIG_BAD_ID || err == EOPNOTSUPP)
> + /* We are not talking to /hurd/ifsock. Probably
> + someone came in after we linked our node, unlinked
> + it, and replaced it with a different node, before we
> + did our lookup. Treat it as if our link had failed
> + with EEXIST. */
> + err = EADDRINUSE;
This failure scenario should now be impossible, so I don't think it makes
sense to preserve a comment describing it. We could remove this check
entirely and just let the underlying error percolate. But I'd be more
inclined to change it to return EGRATUITOUS because that's exactly the case
here--a system translator with a clear mandate to support a protocol is
failing to support that protocol correctly.
> + /* Link the node, now a socket with proper mode, into the
> + * target directory. */
Fix comment formatting (no *).
Thanks,
Roland
@@ -40,7 +40,7 @@ __bind (int fd, __CONST_SOCKADDR_ARG ad
char *name = _hurd_sun_path_dupa (addr, len);
/* For the local domain, we must create a node in the filesystem
using the ifsock translator and then fetch the address from it. */
- file_t dir, node;
+ file_t dir, node, ifsock;
char *n;
dir = __file_name_split (name, &n);
@@ -61,36 +61,38 @@ __bind (int fd, __CONST_SOCKADDR_ARG ad
MACH_MSG_TYPE_COPY_SEND);
if (! err)
{
- /* Link the node, now a socket, into the target directory. */
- err = __dir_link (dir, node, n, 1);
- if (err == EEXIST)
- err = EADDRINUSE;
+ enum retry_type doretry;
+ char retryname[1024];
+ /* Get a port to the ifsock translator. */
+ err = __dir_lookup (node, "", 0, 0, &doretry, retryname, &ifsock);
+ if (! err)
+ if (doretry != FS_RETRY_NORMAL || retryname[0] != '\0')
+ err = EADDRINUSE;
}
- __mach_port_deallocate (__mach_task_self (), node);
if (! err)
{
- /* Get a port to the ifsock translator. */
- file_t ifsock = __file_name_lookup_under (dir, n, 0, 0);
- if (ifsock == MACH_PORT_NULL)
- {
- err = errno;
- /* If we failed, get rid of the node we created. */
- __dir_unlink (dir, n);
- }
- else
+ /* Get the address port. */
+ err = __ifsock_getsockaddr (ifsock, &aport);
+ if (err == MIG_BAD_ID || err == EOPNOTSUPP)
+ /* We are not talking to /hurd/ifsock. Probably
+ someone came in after we linked our node, unlinked
+ it, and replaced it with a different node, before we
+ did our lookup. Treat it as if our link had failed
+ with EEXIST. */
+ err = EADDRINUSE;
+ if (! err)
{
- /* Get the address port. */
- err = __ifsock_getsockaddr (ifsock, &aport);
- if (err == MIG_BAD_ID || err == EOPNOTSUPP)
- /* We are not talking to /hurd/ifsock. Probably
- someone came in after we linked our node, unlinked
- it, and replaced it with a different node, before we
- did our lookup. Treat it as if our link had failed
- with EEXIST. */
+ /* Link the node, now a socket with proper mode, into the
+ * target directory. */
+ err = __dir_link (dir, node, n, 1);
+ if (err == EEXIST)
err = EADDRINUSE;
+ if (err)
+ __mach_port_deallocate (__mach_task_self (), aport);
}
__mach_port_deallocate (__mach_task_self (), ifsock);
}
+ __mach_port_deallocate (__mach_task_self (), node);
}
__mach_port_deallocate (__mach_task_self (), dir);