[PATCH 3/4] lwip: return EINTR when a select() IPC thread is cancelled

[Joan Lledó]

---
lwip/io-ops.c | 8 ++++++++
1 file changed, 8 insertions(+)

diff --git a/lwip/io-ops.c b/lwip/io-ops.c
index 1429bc55..72e08e26 100644
--- a/lwip/io-ops.c
+++ b/lwip/io-ops.c
@@ -198,6 +198,8 @@ lwip_io_select_common (struct sock_user *user,
int timeout;
struct pollfd fdp;
nfds_t nfds;
+ mach_port_type_t type;
+ error_t err;

if (!user)
return EOPNOTSUPP;
@@ -227,6 +229,12 @@ lwip_io_select_common (struct sock_user *user,
timeout = tv ? tv->tv_sec * 1000 + tv->tv_nsec / 1000000 : -1;
ret = lwip_poll (&fdp, nfds, timeout);

+ err = mach_port_type (mach_task_self (), reply, &type);
+ if (err || (type & MACH_PORT_TYPE_DEAD_NAME))
+ /* The reply port is dead, we were cancelled */
+ return EINTR;
+
+
if (ret > 0)
{
if (fdp.revents & POLLERR)

[Joan Lledó]

---
lwip/io-ops.c | 3 +++
1 file changed, 3 insertions(+)

diff --git a/lwip/io-ops.c b/lwip/io-ops.c
index 636c26f7..1429bc55 100644
--- a/lwip/io-ops.c
+++ b/lwip/io-ops.c
@@ -229,6 +229,9 @@ lwip_io_select_common (struct sock_user *user,

if (ret > 0)
{
+ if (fdp.revents & POLLERR)
+ return EIO;
+
if (fdp.revents & POLLIN)
*select_type |= SELECT_READ;

[Joan Lledó]

---
lwip/main.c | 42 +++++++++++-------------------------------
1 file changed, 11 insertions(+), 31 deletions(-)

diff --git a/lwip/main.c b/lwip/main.c
index 9f7eb9b2..4dfbe143 100644
--- a/lwip/main.c
+++ b/lwip/main.c
@@ -99,6 +99,7 @@ int
lwip_demuxer (mach_msg_header_t * inp, mach_msg_header_t * outp)
{
struct port_info *pi;
+ mig_routine_t routine = NULL;

/* Clear errno to prevent raising previous errors again */
errno = 0;
@@ -116,40 +117,19 @@ lwip_demuxer (mach_msg_header_t * inp, mach_msg_header_t * outp)
if (pi)
{
ports_port_deref (pi);
-
- mig_routine_t routine;
- if ((routine = lwip_io_server_routine (inp)) ||
- (routine = lwip_socket_server_routine (inp)) ||
- (routine = lwip_pfinet_server_routine (inp)) ||
- (routine = lwip_iioctl_server_routine (inp)) ||
- (routine = NULL, trivfs_demuxer (inp, outp)) ||
- (routine = lwip_startup_notify_server_routine (inp)))
- {
- if (routine)
- (*routine) (inp, outp);
- return TRUE;
- }
- else
- return FALSE;
+ routine = lwip_io_server_routine (inp);
}
- else
+
+ if (routine || (routine = lwip_socket_server_routine (inp)) ||
+ (routine = lwip_pfinet_server_routine (inp)) ||
+ (routine = lwip_iioctl_server_routine (inp)) ||
+ (routine = lwip_startup_notify_server_routine (inp)))
{
- mig_routine_t routine;
- if ((routine = lwip_socket_server_routine (inp)) ||
- (routine = lwip_pfinet_server_routine (inp)) ||
- (routine = lwip_iioctl_server_routine (inp)) ||
- (routine = NULL, trivfs_demuxer (inp, outp)) ||
- (routine = lwip_startup_notify_server_routine (inp)))
- {
- if (routine)
- (*routine) (inp, outp);
- return TRUE;
- }
- else
- return FALSE;
+ (*routine) (inp, outp);
+ return TRUE;
}
-
- return 0;
+ else
+ return trivfs_demuxer (inp, outp);
}

void

[Joan Lledó]

---
lwip/options.c | 2 +-
lwip/pfinet-ops.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/lwip/options.c b/lwip/options.c
index 6591ac52..d35b9f32 100644
--- a/lwip/options.c
+++ b/lwip/options.c
@@ -133,7 +133,7 @@ parse_opt (int opt, char *arg, struct argp_state *state)
}
in = h->curint;

- strncpy (in->dev_name, arg, DEV_NAME_LEN);
+ strncpy (in->dev_name, arg, sizeof(in->dev_name)-1);
break;

case 'a':
diff --git a/lwip/pfinet-ops.c b/lwip/pfinet-ops.c
index 56b7dcd1..1af2a3fe 100644
--- a/lwip/pfinet-ops.c
+++ b/lwip/pfinet-ops.c
@@ -61,7 +61,7 @@ dev_ifconf (struct ifconf *ifc)
memset (ifr, 0, sizeof (struct ifreq));

strncpy (ifr->ifr_name, netif_get_state (netif)->devname,
- strlen (netif_get_state (netif)->devname) + 1);
+ sizeof (ifr->ifr_name)-1);
saddr->sin_len = sizeof (struct sockaddr_in);
saddr->sin_family = AF_INET;
saddr->sin_addr.s_addr = netif_ip4_addr (netif)->addr;

[Samuel Thibault]

Hello,

Here are some patches for the lwip translator. They solve minor problems I've found when working on our lwip library.

Thanks :)

But could you write the proper ChangeLog entries to explain not only why
the change but also what change is made? See git log and the GNU Coding
Style. The idea is that they help a *lot* when having to dig back what
happened to a given function, to the use of a macro, etc.

Samuel

[Joan Lledó]

Needed to properly support poll in glibc (_hurd_select).

* lwip/io-ops.c (lwip_io_select_common):
If POLLERR is set, return EIO.
---
lwip/io-ops.c | 3 +++
1 file changed, 3 insertions(+)

diff --git a/lwip/io-ops.c b/lwip/io-ops.c
index 636c26f7..1429bc55 100644
--- a/lwip/io-ops.c
+++ b/lwip/io-ops.c
@@ -229,6 +229,9 @@ lwip_io_select_common (struct sock_user *user,

if (ret > 0)
{
+ if (fdp.revents & POLLERR)
+ return EIO;
+
if (fdp.revents & POLLIN)
*select_type |= SELECT_READ;

[Samuel Thibault]

Post by Joan Lledó
Needed to properly support poll in glibc (_hurd_select).

Applied, thanks!

Post by Joan Lledó
If POLLERR is set, return EIO.
---
lwip/io-ops.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/lwip/io-ops.c b/lwip/io-ops.c
index 636c26f7..1429bc55 100644
--- a/lwip/io-ops.c
+++ b/lwip/io-ops.c
@@ -229,6 +229,9 @@ lwip_io_select_common (struct sock_user *user,
if (ret > 0)
{
+ if (fdp.revents & POLLERR)
+ return EIO;
+
if (fdp.revents & POLLIN)
*select_type |= SELECT_READ;
--
2.11.0

[Joan Lledó]

GCC 8 new warning -Wstringop-truncation detected some buffer overflows.

* lwip/options.c (parse_opt): Fix the buffer overflow.
* lwip/pfinet-ops.c (dev_ifconf): Likewise.
---
lwip/options.c | 2 +-
lwip/pfinet-ops.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/lwip/options.c b/lwip/options.c
index 6591ac52..d35b9f32 100644
--- a/lwip/options.c
+++ b/lwip/options.c
@@ -133,7 +133,7 @@ parse_opt (int opt, char *arg, struct argp_state *state)
}
in = h->curint;

- strncpy (in->dev_name, arg, DEV_NAME_LEN);
+ strncpy (in->dev_name, arg, sizeof(in->dev_name)-1);
break;

case 'a':
diff --git a/lwip/pfinet-ops.c b/lwip/pfinet-ops.c
index 56b7dcd1..1af2a3fe 100644
--- a/lwip/pfinet-ops.c
+++ b/lwip/pfinet-ops.c
@@ -61,7 +61,7 @@ dev_ifconf (struct ifconf *ifc)
memset (ifr, 0, sizeof (struct ifreq));

strncpy (ifr->ifr_name, netif_get_state (netif)->devname,
- strlen (netif_get_state (netif)->devname) + 1);
+ sizeof (ifr->ifr_name)-1);
saddr->sin_len = sizeof (struct sockaddr_in);
saddr->sin_family = AF_INET;
saddr->sin_addr.s_addr = netif_ip4_addr (netif)->addr;

[Samuel Thibault]

Post by Joan Lledó
GCC 8 new warning -Wstringop-truncation detected some buffer overflows.
* lwip/options.c (parse_opt): Fix the buffer overflow.
* lwip/pfinet-ops.c (dev_ifconf): Likewise.
---
lwip/options.c | 2 +-
lwip/pfinet-ops.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/lwip/options.c b/lwip/options.c
index 6591ac52..d35b9f32 100644
--- a/lwip/options.c
+++ b/lwip/options.c
@@ -133,7 +133,7 @@ parse_opt (int opt, char *arg, struct argp_state *state)
}
in = h->curint;
- strncpy (in->dev_name, arg, DEV_NAME_LEN);
+ strncpy (in->dev_name, arg, sizeof(in->dev_name)-1);

Mmm, but if arg is longer than the given size and doesn't contain a \0,
in->dev_name will not contain one either?

Post by Joan Lledó
break;
diff --git a/lwip/pfinet-ops.c b/lwip/pfinet-ops.c
index 56b7dcd1..1af2a3fe 100644
--- a/lwip/pfinet-ops.c
+++ b/lwip/pfinet-ops.c
@@ -61,7 +61,7 @@ dev_ifconf (struct ifconf *ifc)
memset (ifr, 0, sizeof (struct ifreq));
strncpy (ifr->ifr_name, netif_get_state (netif)->devname,
- strlen (netif_get_state (netif)->devname) + 1);
+ sizeof (ifr->ifr_name)-1);

Similarly.

Samuel

[Joan Lledó]

Post by Samuel Thibault

Post by Joan Lledó
- strncpy (in->dev_name, arg, DEV_NAME_LEN);
+ strncpy (in->dev_name, arg, sizeof(in->dev_name)-1);

Mmm, but if arg is longer than the given size and doesn't contain a \0,
in->dev_name will not contain one either?

No, b/c at most sizeof(in->dev_name)-1 bytes from arg will be copied,
leaving the last byte in in->dev_name unwritten, which always will be
equal to zero as it's initialized in parse_hook_add_interface().

Post by Samuel Thibault

Post by Joan Lledó
strncpy (ifr->ifr_name, netif_get_state (netif)->devname,
- strlen (netif_get_state (netif)->devname) + 1);
+ sizeof (ifr->ifr_name)-1);

Similarly.

Same here, the last byte is never written and is initialized to \0 in
the previous line.

[Samuel Thibault]

Post by Joan Lledó

Post by Samuel Thibault

Post by Joan Lledó
- strncpy (in->dev_name, arg, DEV_NAME_LEN);
+ strncpy (in->dev_name, arg, sizeof(in->dev_name)-1);

Mmm, but if arg is longer than the given size and doesn't contain a \0,
in->dev_name will not contain one either?

No, b/c at most sizeof(in->dev_name)-1 bytes from arg will be copied,
leaving the last byte in in->dev_name unwritten, which always will be
equal to zero as it's initialized in parse_hook_add_interface().

Right.

Applied, thanks!

Samuel

Post by Joan Lledó

Post by Samuel Thibault

Post by Joan Lledó
strncpy (ifr->ifr_name, netif_get_state (netif)->devname,
- strlen (netif_get_state (netif)->devname) + 1);
+ sizeof (ifr->ifr_name)-1);

Similarly.

Same here, the last byte is never written and is initialized to \0 in
the previous line.

[Joan Lledó]

Needed to properly support poll in glibc (_hurd_select).

* lwip/io-ops.c (lwip_io_select_common): Detect when the
current RPC is cancelled by checking the reply port.
---
lwip/io-ops.c | 8 ++++++++
1 file changed, 8 insertions(+)

diff --git a/lwip/io-ops.c b/lwip/io-ops.c
index 1429bc55..72e08e26 100644
--- a/lwip/io-ops.c
+++ b/lwip/io-ops.c
@@ -198,6 +198,8 @@ lwip_io_select_common (struct sock_user *user,
int timeout;
struct pollfd fdp;
nfds_t nfds;
+ mach_port_type_t type;
+ error_t err;

if (!user)
return EOPNOTSUPP;
@@ -227,6 +229,12 @@ lwip_io_select_common (struct sock_user *user,
timeout = tv ? tv->tv_sec * 1000 + tv->tv_nsec / 1000000 : -1;
ret = lwip_poll (&fdp, nfds, timeout);

+ err = mach_port_type (mach_task_self (), reply, &type);
+ if (err || (type & MACH_PORT_TYPE_DEAD_NAME))
+ /* The reply port is dead, we were cancelled */
+ return EINTR;
+
+
if (ret > 0)
{
if (fdp.revents & POLLERR)

[Samuel Thibault]

Post by Joan Lledó
Needed to properly support poll in glibc (_hurd_select).
* lwip/io-ops.c (lwip_io_select_common): Detect when the
current RPC is cancelled by checking the reply port.

Applied, thanks!

Post by Joan Lledó
---
lwip/io-ops.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/lwip/io-ops.c b/lwip/io-ops.c
index 1429bc55..72e08e26 100644
--- a/lwip/io-ops.c
+++ b/lwip/io-ops.c
@@ -198,6 +198,8 @@ lwip_io_select_common (struct sock_user *user,
int timeout;
struct pollfd fdp;
nfds_t nfds;
+ mach_port_type_t type;
+ error_t err;
if (!user)
return EOPNOTSUPP;
@@ -227,6 +229,12 @@ lwip_io_select_common (struct sock_user *user,
timeout = tv ? tv->tv_sec * 1000 + tv->tv_nsec / 1000000 : -1;
ret = lwip_poll (&fdp, nfds, timeout);
+ err = mach_port_type (mach_task_self (), reply, &type);
+ if (err || (type & MACH_PORT_TYPE_DEAD_NAME))
+ /* The reply port is dead, we were cancelled */
+ return EINTR;
+
+
if (ret > 0)
{
if (fdp.revents & POLLERR)
--
2.11.0

[Joan Lledó]

* lwip/main.c (lwip_demuxer): Refactored.
---
lwip/main.c | 42 +++++++++++-------------------------------
1 file changed, 11 insertions(+), 31 deletions(-)

diff --git a/lwip/main.c b/lwip/main.c
index 9f7eb9b2..4dfbe143 100644
--- a/lwip/main.c
+++ b/lwip/main.c
@@ -99,6 +99,7 @@ int
lwip_demuxer (mach_msg_header_t * inp, mach_msg_header_t * outp)
{
struct port_info *pi;
+ mig_routine_t routine = NULL;

/* Clear errno to prevent raising previous errors again */
errno = 0;
@@ -116,40 +117,19 @@ lwip_demuxer (mach_msg_header_t * inp, mach_msg_header_t * outp)
if (pi)
{
ports_port_deref (pi);
-
- mig_routine_t routine;
- if ((routine = lwip_io_server_routine (inp)) ||
- (routine = lwip_socket_server_routine (inp)) ||
- (routine = lwip_pfinet_server_routine (inp)) ||
- (routine = lwip_iioctl_server_routine (inp)) ||
- (routine = NULL, trivfs_demuxer (inp, outp)) ||
- (routine = lwip_startup_notify_server_routine (inp)))
- {
- if (routine)
- (*routine) (inp, outp);
- return TRUE;
- }
- else
- return FALSE;
+ routine = lwip_io_server_routine (inp);
}
- else
+
+ if (routine || (routine = lwip_socket_server_routine (inp)) ||
+ (routine = lwip_pfinet_server_routine (inp)) ||
+ (routine = lwip_iioctl_server_routine (inp)) ||
+ (routine = lwip_startup_notify_server_routine (inp)))
{
- mig_routine_t routine;
- if ((routine = lwip_socket_server_routine (inp)) ||
- (routine = lwip_pfinet_server_routine (inp)) ||
- (routine = lwip_iioctl_server_routine (inp)) ||
- (routine = NULL, trivfs_demuxer (inp, outp)) ||
- (routine = lwip_startup_notify_server_routine (inp)))
- {
- if (routine)
- (*routine) (inp, outp);
- return TRUE;
- }
- else
- return FALSE;
+ (*routine) (inp, outp);
+ return TRUE;
}
-
- return 0;
+ else
+ return trivfs_demuxer (inp, outp);
}

void

[Samuel Thibault]

Post by Joan Lledó
* lwip/main.c (lwip_demuxer): Refactored.

Applied, thanks!

Post by Joan Lledó
---
lwip/main.c | 42 +++++++++++-------------------------------
1 file changed, 11 insertions(+), 31 deletions(-)
diff --git a/lwip/main.c b/lwip/main.c
index 9f7eb9b2..4dfbe143 100644
--- a/lwip/main.c
+++ b/lwip/main.c
@@ -99,6 +99,7 @@ int
lwip_demuxer (mach_msg_header_t * inp, mach_msg_header_t * outp)
{
struct port_info *pi;
+ mig_routine_t routine = NULL;
/* Clear errno to prevent raising previous errors again */
errno = 0;
@@ -116,40 +117,19 @@ lwip_demuxer (mach_msg_header_t * inp, mach_msg_header_t * outp)
if (pi)
{
ports_port_deref (pi);
-
- mig_routine_t routine;
- if ((routine = lwip_io_server_routine (inp)) ||
- (routine = lwip_socket_server_routine (inp)) ||
- (routine = lwip_pfinet_server_routine (inp)) ||
- (routine = lwip_iioctl_server_routine (inp)) ||
- (routine = NULL, trivfs_demuxer (inp, outp)) ||
- (routine = lwip_startup_notify_server_routine (inp)))
- {
- if (routine)
- (*routine) (inp, outp);
- return TRUE;
- }
- else
- return FALSE;
+ routine = lwip_io_server_routine (inp);
}
- else
+
+ if (routine || (routine = lwip_socket_server_routine (inp)) ||
+ (routine = lwip_pfinet_server_routine (inp)) ||
+ (routine = lwip_iioctl_server_routine (inp)) ||
+ (routine = lwip_startup_notify_server_routine (inp)))
{
- mig_routine_t routine;
- if ((routine = lwip_socket_server_routine (inp)) ||
- (routine = lwip_pfinet_server_routine (inp)) ||
- (routine = lwip_iioctl_server_routine (inp)) ||
- (routine = NULL, trivfs_demuxer (inp, outp)) ||
- (routine = lwip_startup_notify_server_routine (inp)))
- {
- if (routine)
- (*routine) (inp, outp);
- return TRUE;
- }
- else
- return FALSE;
+ (*routine) (inp, outp);
+ return TRUE;
}
-
- return 0;
+ else
+ return trivfs_demuxer (inp, outp);
}
void
--
2.11.0