hurd: Check return value of mach_port_mod_refs() in the dup routine of fcntl()
Checks
| Context |
Check |
Description |
| redhat-pt-bot/TryBot-apply_patch |
success
|
Patch applied to master at the time it was sent
|
| linaro-tcwg-bot/tcwg_glibc_build--master-aarch64 |
success
|
Build passed
|
| linaro-tcwg-bot/tcwg_glibc_check--master-aarch64 |
success
|
Test passed
|
| linaro-tcwg-bot/tcwg_glibc_build--master-arm |
success
|
Build passed
|
| redhat-pt-bot/TryBot-32bit |
success
|
Build for i686
|
| linaro-tcwg-bot/tcwg_glibc_check--master-arm |
success
|
Test passed
|
Commit Message
Ignoring the return value of mach_port_mod_ref() causes the situation
that when the reference count of the io server port is full, the caller
of dup() still supposes a new file discriptor is allocated. In this
patch we check the return value of mach_port_mod_ref() and return an
error when it fails.
Signed-off-by: Zhaoming Luo <zhmingluo@163.com>
---
sysdeps/mach/hurd/fcntl.c | 38 +++++++++++++++++++++++++++-----------
1 file changed, 27 insertions(+), 11 deletions(-)
Comments
Hello,
While at it, we'd better also fix sysdeps/mach/hurd/dup3.c the exact
same way (which will also fix dup2).
Thanks,
Samuel
Zhaoming Luo, le jeu. 06 mars 2025 06:39:04 +0800, a ecrit:
> Ignoring the return value of mach_port_mod_ref() causes the situation
> + if (err)
> + {
> + /* When an error occurs during giving a user ref to the
> + io server port */
> + result = -1;
> +
> + if (err == KERN_UREFS_OVERFLOW)
> + errno = EMFILE;
> + else
> + errno = EINVAL;
The idiomatic way is to use __hurd_fail:
if (err == KERN_UREFS_OVERFLOW)
result = __hurd_failed (EMFILE);
else
result = __hurd_failed (EINVAL);
> + }
> + else
> + {
> + if (ctty != MACH_PORT_NULL)
> + __mach_port_mod_refs (__mach_task_self (), ctty,
> + MACH_PORT_RIGHT_SEND, 1);
This one could fail too, we want to fail the whole thing in that case
(and __mach_port_mod_refs(port, -1) to clean the ref successfully
acquired above).
> +
> + /* Install the ports and flags in the new descriptor. */
> + if (ctty != MACH_PORT_NULL)
> + _hurd_port_set (&new->ctty, ctty);
> + new->flags = flags;
> + _hurd_port_locked_set (&new->port, port); /* Unlocks NEW. */
> + }
> }
>
> HURD_CRITICAL_END;
@@ -83,18 +83,34 @@ __libc_fcntl (int fd, int cmd, ...)
result = -1;
else
{
- /* Give the ports each a user ref for the new descriptor. */
- __mach_port_mod_refs (__mach_task_self (), port,
+ /* Give the ports (i.e. `ctty` and `port`) each a user ref for
+ the new descriptor. */
+ err = __mach_port_mod_refs (__mach_task_self (), port,
MACH_PORT_RIGHT_SEND, 1);
- if (ctty != MACH_PORT_NULL)
- __mach_port_mod_refs (__mach_task_self (), ctty,
- MACH_PORT_RIGHT_SEND, 1);
-
- /* Install the ports and flags in the new descriptor. */
- if (ctty != MACH_PORT_NULL)
- _hurd_port_set (&new->ctty, ctty);
- new->flags = flags;
- _hurd_port_locked_set (&new->port, port); /* Unlocks NEW. */
+
+ if (err)
+ {
+ /* When an error occurs during giving a user ref to the
+ io server port */
+ result = -1;
+
+ if (err == KERN_UREFS_OVERFLOW)
+ errno = EMFILE;
+ else
+ errno = EINVAL;
+ }
+ else
+ {
+ if (ctty != MACH_PORT_NULL)
+ __mach_port_mod_refs (__mach_task_self (), ctty,
+ MACH_PORT_RIGHT_SEND, 1);
+
+ /* Install the ports and flags in the new descriptor. */
+ if (ctty != MACH_PORT_NULL)
+ _hurd_port_set (&new->ctty, ctty);
+ new->flags = flags;
+ _hurd_port_locked_set (&new->port, port); /* Unlocks NEW. */
+ }
}
HURD_CRITICAL_END;