This is the mail archive of the
cygwin-developers@cygwin.com
mailing list for the Cygwin project.
Re: Can somebody pinch me, please?
Egor,
[and all lurkers on cygwin-developers looking for a chance to help]
On Mar 11 19:19, egor duda wrote:
> If the connection cannot be established immediately and O_NONBLOCK is
> set for the file descriptor for the socket, connect() will fail and set
> errno to [EINPROGRESS], but the connection request will not be aborted,
> and the connection will be established asynchronously. Subsequent calls
> to connect() for the same socket, before the connection is established,
> will fail and set errno to [EALREADY].
as you might have seen, I've changed the credential transaction so that
it's using the socket itself for the transaction. This works for
non-blocking sockets so that the transaction is deferred to select.
When select indicates a successful connect (write bit set, but not read
bit), then the transaction is called here instead of in connect itself.
I've prepared a patch which moves the secret event handshake into two
functions, sec_event_accept and sec_event_connect, which are then
called from accept and connect. This simplifies reading the accept and
connect code. It also allows to defer the secret event handshake to
select for non-blocking sockets.
I think this is the right way to do it. A non-blocking connect is not
finshed until either connect itself returned success, or select has
returned with the write bit set for this socket. I've created a method
af_local_connect, which combines secret event handshake and credential
transaction, so select doesn't need to know about these gory details.
I've attached the patch to this posting. Regardless of its size, it
has only two really interesting bits in it:
- The connect side uses the connect_secret member in this, instead of
a local variable. I didn't see any reason why the connect side
shouldn't use it the same way as the accept side does.
- For non-blocking sockets, the secret event handshake is now deferred to
select. If it fails, it sets the read bit to indicate failure.
Could you please have a look and tell me if you see any problem with this?
Corinna
* fhandler.h (fhandler_socket::eid_connect): Make private.
(fhandler_socket::set_connect_secret): Ditto.
(fhandler_socket::get_connect_secret): Ditto.
(fhandler_socket::create_secret_event): Ditto. Remove secret argument.
(fhandler_socket::check_peer_secret_event): Ditto.
(fhandler_socket::signal_secret_event): Make private.
(fhandler_socket::close_secret_event): Ditto.
(fhandler_socket::sec_event_accept): New private method.
(fhandler_socket::sec_event_connect): Ditto.
(fhandler_socket::af_local_connect): New public method.
* fhandler_socket.cc: Use 'struct sockaddr' and 'struct sockaddr_in'
rather than just 'sockaddr' and 'sockaddr_in' throughout.
(fhandler_socket::eid_connect): Drop AF_LOCAL/SOCK_STREAM test.
(fhandler_socket::create_secret_event): Remove secret argument.
Always use connect_secret instead.
(fhandler_socket::check_peer_secret_event): Ditto.
(fhandler_socket::sec_event_connect): New method, combining entire
secret event handshake on connect side.
(fhandler_socket::af_local_connect): New method, combining secret
event handshake and eid credential transaction on connect side, to
be called from select.
(fhandler_socket::sec_event_accept): New method, combining entire
secret event handshake on accept side.
(fhandler_socket::connect): Drop secret, use connect_secret instead.
Move entire secret event handshake to sec_event_connect.
(fhandler_socket::accept): Move entire secret event handshake to
sec_event_accept.
* select.cc (set_bits): Just call af_local_connect here.
Index: fhandler.h
===================================================================
RCS file: /cvs/src/src/winsup/cygwin/fhandler.h,v
retrieving revision 1.233
diff -u -p -r1.233 fhandler.h
--- fhandler.h 21 Mar 2005 19:10:45 -0000 1.233
+++ fhandler.h 22 Mar 2005 09:39:17 -0000
@@ -376,8 +376,8 @@ class fhandler_socket: public fhandler_b
bool eid_recv (void);
bool eid_send (void);
void eid_accept (void);
- public:
void eid_connect (void);
+ public:
void set_socketpair_eids (void);
private:
@@ -458,12 +458,19 @@ class fhandler_socket: public fhandler_b
int get_socket_type () {return type;}
void set_sun_path (const char *path);
char *get_sun_path () {return sun_path;}
+
+ private:
void set_connect_secret ();
void get_connect_secret (char*);
- HANDLE create_secret_event (int *secret = NULL);
- int check_peer_secret_event (struct sockaddr_in *peer, int *secret = NULL);
+ HANDLE create_secret_event ();
+ int check_peer_secret_event (struct sockaddr_in *peer);
void signal_secret_event ();
void close_secret_event ();
+ int sec_event_accept (int, struct sockaddr_in *);
+ int sec_event_connect (struct sockaddr_in *peer);
+ public:
+ int af_local_connect (void);
+
int __stdcall fstat (struct __stat64 *buf) __attribute__ ((regparm (2)));
int __stdcall fchmod (mode_t mode) __attribute__ ((regparm (1)));
int __stdcall fchown (__uid32_t uid, __gid32_t gid) __attribute__ ((regparm (2)));
Index: fhandler_socket.cc
===================================================================
RCS file: /cvs/src/src/winsup/cygwin/fhandler_socket.cc,v
retrieving revision 1.155
diff -u -p -r1.155 fhandler_socket.cc
--- fhandler_socket.cc 21 Mar 2005 18:56:50 -0000 1.155
+++ fhandler_socket.cc 22 Mar 2005 09:39:18 -0000
@@ -65,7 +65,7 @@ get_inet_addr (const struct sockaddr *in
if (in->sa_family == AF_INET)
{
- *out = * (sockaddr_in *)in;
+ *out = * (struct sockaddr_in *)in;
*outlen = inlen;
return 1;
}
@@ -100,7 +100,7 @@ get_inet_addr (const struct sockaddr *in
memset (buf, 0, sizeof buf);
if (ReadFile (fh, buf, 128, &len, 0))
{
- sockaddr_in sin;
+ struct sockaddr_in sin;
sin.sin_family = AF_INET;
sscanf (buf + strlen (SOCKET_COOKIE), "%hu %08x-%08x-%08x-%08x",
&sin.sin_port,
@@ -238,10 +238,6 @@ fhandler_socket::eid_send (void)
void
fhandler_socket::eid_connect (void)
{
- /* This test allows to keep select.cc clean from boring implementation
- details. */
- if (get_addr_family () != AF_LOCAL || get_socket_type () != SOCK_STREAM)
- return;
debug_printf ("eid_connect called");
bool orig_async_io, orig_is_nonblocking;
eid_setblocking (orig_async_io, orig_is_nonblocking);
@@ -306,7 +302,7 @@ fhandler_socket::get_connect_secret (cha
}
HANDLE
-fhandler_socket::create_secret_event (int* secret)
+fhandler_socket::create_secret_event ()
{
struct sockaddr_in sin;
int sin_len = sizeof (sin);
@@ -321,7 +317,7 @@ fhandler_socket::create_secret_event (in
}
char event_name[CYG_MAX_PATH];
- secret_event_name (event_name, sin.sin_port, secret ?: connect_secret);
+ secret_event_name (event_name, sin.sin_port, connect_secret);
secret_event = CreateEvent (&sec_all, FALSE, FALSE, event_name);
if (!secret_event)
@@ -354,12 +350,12 @@ fhandler_socket::close_secret_event ()
}
int
-fhandler_socket::check_peer_secret_event (struct sockaddr_in* peer, int* secret)
+fhandler_socket::check_peer_secret_event (struct sockaddr_in *peer)
{
char event_name[CYG_MAX_PATH];
- secret_event_name (event_name, peer->sin_port, secret ?: connect_secret);
+ secret_event_name (event_name, peer->sin_port, connect_secret);
HANDLE ev = CreateEvent (&sec_all_nih, FALSE, FALSE, event_name);
if (!ev)
debug_printf("create event %E");
@@ -377,6 +373,78 @@ fhandler_socket::check_peer_secret_event
return 0;
}
+int
+fhandler_socket::sec_event_connect (struct sockaddr_in *peer)
+{
+ bool secret_check_failed = false;
+ struct sockaddr_in sin;
+ int siz = sizeof sin;
+
+ debug_printf ("sec_event_connect called");
+ if (!peer)
+ {
+ if (::getpeername (get_socket (), (struct sockaddr *) &sin, &siz))
+ goto err;
+ peer = &sin;
+ }
+ if (!create_secret_event ())
+ secret_check_failed = true;
+ if (!secret_check_failed && !check_peer_secret_event (peer))
+ {
+ debug_printf ("accept from unauthorized server");
+ secret_check_failed = true;
+ }
+ if (!secret_check_failed)
+ return 0;
+
+err:
+ close_secret_event ();
+ closesocket (get_socket ());
+ WSASetLastError (WSAECONNREFUSED);
+ set_winsock_errno ();
+ return -1;
+}
+
+/* Called from select(). It combines the secret event handshake and
+ the eid credential transaction into one call. This keeps implementation
+ details from select. */
+int
+fhandler_socket::af_local_connect (void)
+{
+ if (get_addr_family () != AF_LOCAL || get_socket_type () != SOCK_STREAM)
+ return 0;
+ int ret = sec_event_connect (NULL);
+ if (!ret)
+ eid_connect ();
+ return ret;
+}
+
+int
+fhandler_socket::sec_event_accept (int sock, struct sockaddr_in *peer)
+{
+ bool secret_check_failed = false;
+
+ debug_printf ("sec_event_accept called");
+
+ if (!create_secret_event ())
+ secret_check_failed = true;
+
+ if (!secret_check_failed
+ && !check_peer_secret_event (peer))
+ {
+ debug_printf ("connect from unauthorized client");
+ secret_check_failed = true;
+ }
+ if (secret_check_failed)
+ {
+ close_secret_event ();
+ closesocket (sock);
+ set_errno (ECONNABORTED);
+ return -1;
+ }
+ return sock;
+}
+
void
fhandler_socket::fixup_before_fork_exec (DWORD win_proc_id)
{
@@ -687,16 +755,14 @@ int
fhandler_socket::connect (const struct sockaddr *name, int namelen)
{
int res = -1;
- bool secret_check_failed = false;
bool in_progress = false;
- sockaddr_in sin;
- int secret [4];
+ struct sockaddr_in sin;
DWORD err;
- if (!get_inet_addr (name, namelen, &sin, &namelen, secret))
+ if (!get_inet_addr (name, namelen, &sin, &namelen, connect_secret))
return -1;
- res = ::connect (get_socket (), (sockaddr *) &sin, namelen);
+ res = ::connect (get_socket (), (struct sockaddr *) &sin, namelen);
if (res)
{
@@ -721,34 +787,6 @@ fhandler_socket::connect (const struct s
if (get_addr_family () == AF_LOCAL && get_socket_type () == SOCK_STREAM)
{
- if (!res || in_progress)
- {
- if (!create_secret_event (secret))
- {
- secret_check_failed = true;
- }
- else if (in_progress)
- signal_secret_event ();
- }
-
- if (!secret_check_failed && !res)
- {
- if (!check_peer_secret_event (&sin, secret))
- {
- debug_printf ("accept from unauthorized server");
- secret_check_failed = true;
- }
- }
-
- if (secret_check_failed)
- {
- close_secret_event ();
- if (res)
- closesocket (res);
- set_errno (ECONNREFUSED);
- res = -1;
- }
-
/* Prepare eid credential transaction. */
sec_pid = getpid ();
sec_uid = geteuid32 ();
@@ -758,6 +796,9 @@ fhandler_socket::connect (const struct s
sec_peer_gid = (__gid32_t) -1;
if (!res)
+ res = sec_event_connect (&sin);
+
+ if (!res)
{
/* eid credential transaction. If connect is in progress,
we're deferring the eid transaction to the successful select,
@@ -801,7 +842,6 @@ int
fhandler_socket::accept (struct sockaddr *peer, int *len)
{
int res = -1;
- bool secret_check_failed = false;
/* Allows NULL peer and len parameters. */
struct sockaddr_in peer_dummy;
@@ -825,27 +865,7 @@ fhandler_socket::accept (struct sockaddr
if ((SOCKET) res != INVALID_SOCKET && get_addr_family () == AF_LOCAL
&& get_socket_type () == SOCK_STREAM)
- {
- if (!create_secret_event ())
- secret_check_failed = true;
-
- if (!secret_check_failed)
- {
- if (!check_peer_secret_event ((struct sockaddr_in*) peer))
- {
- debug_printf ("connect from unauthorized client");
- secret_check_failed = true;
- }
- }
-
- if (secret_check_failed)
- {
- close_secret_event ();
- closesocket (res);
- set_errno (ECONNABORTED);
- return -1;
- }
- }
+ res = sec_event_accept (res, (struct sockaddr_in *) peer);
if ((SOCKET) res == INVALID_SOCKET)
set_winsock_errno ();
@@ -1254,7 +1274,7 @@ int
fhandler_socket::sendto (const void *ptr, size_t len, int flags,
const struct sockaddr *to, int tolen)
{
- sockaddr_in sin;
+ struct sockaddr_in sin;
if (to && !get_inet_addr (to, tolen, &sin, &tolen))
return SOCKET_ERROR;
Index: select.cc
===================================================================
RCS file: /cvs/src/src/winsup/cygwin/select.cc,v
retrieving revision 1.108
diff -u -p -r1.108 select.cc
--- select.cc 21 Mar 2005 18:56:50 -0000 1.108
+++ select.cc 22 Mar 2005 09:39:18 -0000
@@ -338,11 +338,10 @@ set_bits (select_record *me, fd_set *rea
UNIX_FD_SET (me->fd, writefds);
if (me->except_on_write && (sock = me->fh->is_socket ()))
{
- /* eid credential transaction on successful non-blocking connect.
- Since the read bit indicates an error, don't start transaction
- if it's set. */
- if (!me->read_ready && sock->connect_state () == connect_pending)
- sock->eid_connect ();
+ /* Special AF_LOCAL handling. */
+ if (!me->read_ready && sock->connect_state () == connect_pending
+ && sock->af_local_connect () && me->read_selected)
+ UNIX_FD_SET (me->fd, readfds);
sock->connect_state (connected);
}
ready++;
@@ -354,8 +353,10 @@ set_bits (select_record *me, fd_set *rea
UNIX_FD_SET (me->fd, writefds);
if ((sock = me->fh->is_socket ()))
{
- if (!me->read_ready && sock->connect_state () == connect_pending)
- sock->eid_connect ();
+ /* Special AF_LOCAL handling. */
+ if (!me->read_ready && sock->connect_state () == connect_pending
+ && sock->af_local_connect () && me->read_selected)
+ UNIX_FD_SET (me->fd, readfds);
sock->connect_state (connected);
}
}
--
Corinna Vinschen Please, send mails regarding Cygwin to
Cygwin Project Co-Leader mailto:cygwin@cygwin.com
Red Hat, Inc.