Core: Avoid memcpy from NULL

Ben Kallus benjamin.p.kallus.gr at dartmouth.edu
Wed Jan 3 23:57:57 UTC 2024


> Still, general style guidelines suggests that the code shouldn't
> be written this way, and the only reason for j++ in the line in
> question is that it mimics corresponding IPv4 code.

> It's not "just happens".

The point I'm trying to make is that ensuring correctness with
function-like macros is difficult, both because of operator precedence
and argument reevaluation. Expecting contributors to read the
definitions of every macro they use becomes more and more cumbersome
as the codebase expands, especially when some symbols are variably
macros or functions depending on the state of (even infrequently-used)
compile-time constants.

All that said, upon further reflection, I think the UB issue is best
solved outside of ngx_strcpy, where the overhead of an extra check may
have a performance impact. The following patch is sufficient to
silence UBSan in my configuration:

# HG changeset patch
# User Ben Kallus <benjamin.p.kallus.gr at dartmouth.edu>
# Date 1704322684 18000
#      Wed Jan 03 17:58:04 2024 -0500
# Node ID 04eb4b1622d1a488f14bb6d5af25e422ff23d82d
# Parent  ee40e2b1d0833b46128a357fbc84c6e23be9be07
Add check to ngx_pstrdup to prevent 0-length memcpy.

diff -r ee40e2b1d083 -r 04eb4b1622d1 src/core/ngx_string.c
--- a/src/core/ngx_string.c     Mon Dec 25 21:15:48 2023 +0400
+++ b/src/core/ngx_string.c     Wed Jan 03 17:58:04 2024 -0500
@@ -77,8 +77,8 @@
     u_char  *dst;

     dst = ngx_pnalloc(pool, src->len);
-    if (dst == NULL) {
-        return NULL;
+    if (dst == NULL || src->len == 0) {
+        return dst;
     }

     ngx_memcpy(dst, src->data, src->len);


More information about the nginx-devel mailing list