aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorSergey Bugaev <bugaevc@gmail.com>2024-12-04 14:29:15 +0300
committerSamuel Thibault <samuel.thibault@ens-lyon.org>2024-12-05 08:49:35 +0100
commit8cbab3b729a6a2335aeafb34d55d758c7062f2d8 (patch)
tree8c351367a8f2a8790e13a6de8f79217a74f4def7
parent00de38e531f55ca095910e655a0666ccb33fb6f6 (diff)
downloadglibc-8cbab3b729a6a2335aeafb34d55d758c7062f2d8.zip
glibc-8cbab3b729a6a2335aeafb34d55d758c7062f2d8.tar.gz
glibc-8cbab3b729a6a2335aeafb34d55d758c7062f2d8.tar.bz2
hurd: Protect against servers returning bogus read/write lengths
There already was a branch checking for this case in _hurd_fd_read () when the data is returned out-of-line. Do the same for inline data, as well as for _hurd_fd_write (). It's also not possible for the length to be negative, since it's stored in an unsigned integer. Not verifying the returned length can confuse the callers who assume the returned length is always reasonable. This manifested as libzstd test suite failing on writes to /dev/zero, even though the write () call appeared to succeed. In fact, the zero store backing /dev/zero was returning a larger written length than the size actually submitted to it, which is a separate bug to be fixed on the Hurd side. With this patch, EGRATUITOUS is now propagated to the caller. Reported-by: Diego Nieto Cid <dnietoc@gmail.com> Signed-off-by: Sergey Bugaev <bugaevc@gmail.com> Message-ID: <20241204112915.540032-1-bugaevc@gmail.com>
-rw-r--r--hurd/fd-read.c12
-rw-r--r--hurd/fd-write.c10
2 files changed, 14 insertions, 8 deletions
diff --git a/hurd/fd-read.c b/hurd/fd-read.c
index 7ce5d5b..e492c32 100644
--- a/hurd/fd-read.c
+++ b/hurd/fd-read.c
@@ -38,13 +38,15 @@ _hurd_fd_read (struct hurd_fd *fd, void *buf, size_t *nbytes, loff_t offset)
if (err = HURD_FD_PORT_USE_CANCEL (fd, _hurd_ctty_input (port, ctty, readfd)))
return err;
+ if (__glibc_unlikely (nread > *nbytes)) /* Sanity check for bogus server. */
+ {
+ if (data != buf)
+ __vm_deallocate (__mach_task_self (), (vm_address_t) data, nread);
+ return EGRATUITOUS;
+ }
+
if (data != buf)
{
- if (nread > *nbytes) /* Sanity check for bogus server. */
- {
- __vm_deallocate (__mach_task_self (), (vm_address_t) data, nread);
- return EGRATUITOUS;
- }
memcpy (buf, data, nread);
__vm_deallocate (__mach_task_self (), (vm_address_t) data, nread);
}
diff --git a/hurd/fd-write.c b/hurd/fd-write.c
index 7978807..2f070c5 100644
--- a/hurd/fd-write.c
+++ b/hurd/fd-write.c
@@ -34,9 +34,13 @@ _hurd_fd_write (struct hurd_fd *fd,
}
err = HURD_FD_PORT_USE_CANCEL (fd, _hurd_ctty_output (port, ctty, writefd));
+ if (err)
+ return err;
- if (! err)
- *nbytes = wrote;
+ if (__glibc_unlikely (wrote > *nbytes)) /* Sanity check for bogus server. */
+ return EGRATUITOUS;
- return err;
+ *nbytes = wrote;
+
+ return 0;
}