Reject streams that set ->drr_payloadlen to unreasonably large values

In the zstream code, Coverity reported:

"The argument could be controlled by an attacker, who could invoke the
function with arbitrary values (for example, a very high or negative
buffer size)."

It did not report this in the kernel. This is likely because the
userspace code stored this in an int before passing it into the
allocator, while the kernel code stored it in a uint32_t.

However, this did reveal a potentially real problem. On 32-bit systems
and systems with only 4GB of physical memory or less in general, it is
possible to pass a large enough value that the system will hang. Even
worse, on Linux systems, the kernel memory allocator is not able to
support allocations up to the maximum 4GB allocation size that this
allows.

This had already been limited in userspace to 64MB by
`ZFS_SENDRECV_MAX_NVLIST`, but we need a hard limit in the kernel to
protect systems. After some discussion, we settle on 256MB as a hard
upper limit. Attempting to receive a stream that requires more memory
than that will result in E2BIG being returned to user space.

Reported-by: Coverity (CID-1529836)
Reported-by: Coverity (CID-1529837)
Reported-by: Coverity (CID-1529838)
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Signed-off-by: Richard Yao <richard.yao@alumni.stonybrook.edu>
Closes #14285

diff --git a/cmd/zstream/zstream_decompress.c b/cmd/zstream/zstream_decompress.c
index 726c3be6dc6e07f4a661740b1263f5fd911cd63f..0cef36c0441fc8a87302b6f16876203707ed6e2c 100644 (file)
--- a/cmd/zstream/zstream_decompress.c
+++ b/cmd/zstream/zstream_decompress.c
@@ -179,7 +179,10 @@ zstream_do_decompress(int argc, char *argv[])
                        VERIFY0(begin++);
                        seen = B_TRUE;
 
-                       int sz = drr->drr_payloadlen;
+                       uint32_t sz = drr->drr_payloadlen;
+
+                       VERIFY3U(sz, <=, 1U << 28);
+
                        if (sz != 0) {
                                if (sz > bufsz) {
                                        buf = realloc(buf, sz);

diff --git a/cmd/zstream/zstream_recompress.c b/cmd/zstream/zstream_recompress.c
index 38ef758f8ea47a2263dc99138c500e2491fa4510..8392ef3de72fd76011bf1fb655eaa0dd419765aa 100644 (file)
--- a/cmd/zstream/zstream_recompress.c
+++ b/cmd/zstream/zstream_recompress.c
@@ -160,7 +160,10 @@ zstream_do_recompress(int argc, char *argv[])
                        VERIFY0(begin++);
                        seen = B_TRUE;
 
-                       int sz = drr->drr_payloadlen;
+                       uint32_t sz = drr->drr_payloadlen;
+
+                       VERIFY3U(sz, <=, 1U << 28);
+
                        if (sz != 0) {
                                if (sz > bufsz) {
                                        buf = realloc(buf, sz);

diff --git a/cmd/zstream/zstream_redup.c b/cmd/zstream/zstream_redup.c
index 8b12303c5d306d00e9a8f340a2484744d0870a16..c56a09cee75dfd3ede93195538c0342568c49f38 100644 (file)
--- a/cmd/zstream/zstream_redup.c
+++ b/cmd/zstream/zstream_redup.c
@@ -254,7 +254,10 @@ zfs_redup_stream(int infd, int outfd, boolean_t verbose)
                        /* cppcheck-suppress syntaxError */
                        DMU_SET_FEATUREFLAGS(drrb->drr_versioninfo, fflags);
 
-                       int sz = drr->drr_payloadlen;
+                       uint32_t sz = drr->drr_payloadlen;
+
+                       VERIFY3U(sz, <=, 1U << 28);
+
                        if (sz != 0) {
                                if (sz > bufsz) {
                                        free(buf);

diff --git a/lib/libzfs/libzfs_sendrecv.c b/lib/libzfs/libzfs_sendrecv.c
index 49ae7d449b70a8f62110d237cd754beff8eecfdb..038613a1fcfadcf029a98b92d09e950923eed63b 100644 (file)
--- a/lib/libzfs/libzfs_sendrecv.c
+++ b/lib/libzfs/libzfs_sendrecv.c
@@ -5197,6 +5197,14 @@ zfs_receive_one(libzfs_handle_t *hdl, int infd, const char *tosnap,
                            destsnap);
                        *cp = '@';
                        break;
+               case E2BIG:
+                       zfs_error_aux(hdl, dgettext(TEXT_DOMAIN,
+                           "zfs receive required kernel memory allocation "
+                           "larger than the system can support. Please file "
+                           "an issue at the OpenZFS issue tracker:\n"
+                           "https://github.com/openzfs/zfs/issues/new"));
+                       (void) zfs_error(hdl, EZFS_BADSTREAM, errbuf);
+                       break;
                case EBUSY:
                        if (hastoken) {
                                zfs_error_aux(hdl, dgettext(TEXT_DOMAIN,

diff --git a/module/zfs/dmu_recv.c b/module/zfs/dmu_recv.c
index 9461643ceef88550c3601ea798478229f5472569..61cfe36515a309c9e4c3d6f2aed925a26458773d 100644 (file)
--- a/module/zfs/dmu_recv.c
+++ b/module/zfs/dmu_recv.c
@@ -31,6 +31,7 @@
  * Copyright (c) 2022 Axcient.
  */
 
+#include <sys/arc.h>
 #include <sys/spa_impl.h>
 #include <sys/dmu.h>
 #include <sys/dmu_impl.h>
@@ -1246,19 +1247,29 @@ dmu_recv_begin(char *tofs, char *tosnap, dmu_replay_record_t *drr_begin,
 
        uint32_t payloadlen = drc->drc_drr_begin->drr_payloadlen;
        void *payload = NULL;
+
+       /*
+        * Since OpenZFS 2.0.0, we have enforced a 64MB limit in userspace
+        * configurable via ZFS_SENDRECV_MAX_NVLIST. We enforce 256MB as a hard
+        * upper limit. Systems with less than 1GB of RAM will see a lower
+        * limit from `arc_all_memory() / 4`.
+        */
+       if (payloadlen > (MIN((1U << 28), arc_all_memory() / 4)))
+               return (E2BIG);
+
        if (payloadlen != 0)
-               payload = kmem_alloc(payloadlen, KM_SLEEP);
+               payload = vmem_alloc(payloadlen, KM_SLEEP);
 
        err = receive_read_payload_and_next_header(drc, payloadlen,
            payload);
        if (err != 0) {
-               kmem_free(payload, payloadlen);
+               vmem_free(payload, payloadlen);
                return (err);
        }
        if (payloadlen != 0) {
                err = nvlist_unpack(payload, payloadlen, &drc->drc_begin_nvl,
                    KM_SLEEP);
-               kmem_free(payload, payloadlen);
+               vmem_free(payload, payloadlen);
                if (err != 0) {
                        kmem_free(drc->drc_next_rrd,
                            sizeof (*drc->drc_next_rrd));