Hello,

So I think my last attempt was misguided; attached is a simpler
reproducer that exhibits the problem following these steps:

  1. create a socket, bind it, and mark it as O_NONBLOCK;
  2. select on that socket to wait for an incoming connection;
  3. clear its O_NONBLOCK flag;
  4. call accept, which returns a connected socket;
  5. notice that O_NONBLOCK is set on that new socket (it shouldn’t).

I believe the problem is that the connection is made why the original
socket is marked O_NONBLOCK, and pflocal’s ‘S_socket_connect’ inherits
that O_NONBLOCK flag in the socket it gets via ‘sock_clone’.  The test
program later clears O_NONBLOCK but it has no effect since in pflocal
the connected socket has already been created with O_NONBLOCK and queued.

The patch below fixes that (and thus fixes the Shepherd socket
activation, as yelninei initially reported).

Thoughts?

Ludo’.

PS: I wanted to test my modified pflocal by setting it on an arbitrary
node and remapping but that kind of trick doesn’t work:

--8<---------------cut here---------------start------------->8---
ludo@childhurd ~$ settrans -ca pflocal-server /hurd/pflocal 
ludo@childhurd ~$ ./remap /servers/socket/1 $PWD/pflocal-server --  ./simple 
bind: Cannot assign requested address
settrans: fsys_goaway: (ipc/mig) server died
--8<---------------cut here---------------end--------------->8---

As Samuel found out, this is because /hurd/ifsock uses
/servers/socket/1 and there must be a “confused deputy” problem where
ifsock talks to the wrong pflocal.

#define _GNU_SOURCE 1
#include <stdlib.h>
#include <stdio.h>
#include <unistd.h>
#include <sys/socket.h>
#include <sys/un.h>
#include <sys/select.h>
#include <string.h>
#include <fcntl.h>
#include <error.h>

static void
terminate (const char *message)
{
  perror (message);
  exit (EXIT_FAILURE);
}

static void
child (const struct sockaddr *addr, socklen_t len)
{
  int sock = socket (PF_UNIX, SOCK_STREAM | SOCK_NONBLOCK, 0);
  if (sock < 0) terminate ("socket");

  int err = connect (sock, addr, len);
  if (err) terminate ("connect");

  static const char buf[] = "hello, world!\n";
  ssize_t written = write (sock, buf, sizeof buf);
  if (written < 0) terminate ("write");

  exit (EXIT_SUCCESS);
}

int
main ()
{
  int err;
  int sock = socket (PF_UNIX, SOCK_STREAM | SOCK_NONBLOCK, 0);
  if (sock < 0) terminate ("socket");

  struct sockaddr_un addr;
  addr.sun_family = AF_LOCAL;
  strcpy (addr.sun_path, "/tmp/sock");

  unlink ("/tmp/sock");

  err = bind (sock, &addr, sizeof addr);
  if (err) terminate ("bind");

  err = listen (sock, 1);
  if (err) terminate ("listen");

  if (fork () == 0)
    child ((struct sockaddr *) &addr, sizeof addr);

  fd_set read_fds;
  FD_ZERO (&read_fds);
  FD_SET (sock, &read_fds);
  struct timeval timeout = { .tv_sec = 100, .tv_usec = 0 };
  err = select (FD_SETSIZE, &read_fds, NULL, NULL, &timeout);
  if (err < 0) terminate ("select");

  int flags = fcntl (sock, F_GETFL);
  err = fcntl (sock, F_SETFL, flags & ~O_NONBLOCK);
  if (err) terminate ("F_SETFL");

  if ((fcntl (sock, F_GETFL) & O_NONBLOCK) != 0)
    printf ("sock %d O_NONBLOCK!\n", sock);

  struct sockaddr_storage peer;
  socklen_t len;
  int connection = accept (sock, (struct sockaddr *) &peer, &len);
  if (connection < 0) terminate ("accept");

  if ((fcntl (connection, F_GETFL) & O_NONBLOCK) != 0)
    printf ("connection %d O_NONBLOCK!\n", connection);

  char buf[1024];
  err = read (connection, buf, sizeof buf);
  if (err < 0) terminate ("read");

  printf ("read '%s'\n", buf);
}
>From 5d55fd67221696187c697bbdc5067f617059e8fa Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ludovic=20Court=C3=A8s?= <l...@gnu.org>
Date: Thu, 8 May 2025 23:11:36 +0200
Subject: [PATCH] pflocal: Do not inherit PFLOCAL_SOCK_NONBLOCK across
 connect/accept.
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Previously, ‘accept’ would return an O_NONBLOCK socket if the listening
socket was O_NONBLOCK at the time the connection was made.  With this
change, ‘accept’ always returns a socket where O_NONBLOCK is cleared.
---
 pflocal/sock.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/pflocal/sock.c b/pflocal/sock.c
index 90c618ec..6bc061d6 100644
--- a/pflocal/sock.c
+++ b/pflocal/sock.c
@@ -1,6 +1,6 @@
 /* Sock functions
 
-   Copyright (C) 1995,96,2000,01,02, 2005 Free Software Foundation, Inc.
+   Copyright (C) 1995,96,2000,01,02, 2005, 2025 Free Software Foundation, Inc.
    Written by Miles Bader <mi...@gnu.org>
 
    This program is free software; you can redistribute it and/or
@@ -167,8 +167,11 @@ sock_clone (struct sock *template, struct sock **sock)
   if (err)
     return err;
 
-  /* Copy some properties from TEMPLATE.  */
-  (*sock)->flags = template->flags & ~PFLOCAL_SOCK_CONNECTED;
+  /* Copy some properties from TEMPLATE.  Clear O_NONBLOCK because the socket
+     returned by 'accept' must not inherit O_NONBLOCK from the parent
+     socket.  */
+  (*sock)->flags =
+    template->flags & ~(PFLOCAL_SOCK_CONNECTED | PFLOCAL_SOCK_NONBLOCK);
 
   return 0;
 }

base-commit: 50f494ee743a0892380eebfbf428f398bc3b40f5
-- 
2.49.0

Reply via email to