Core: Avoid memcpy from NULL

[Maxim Dounin]

Hello!

On Wed, Jan 03, 2024 at 11:57:57PM +0000, Ben Kallus wrote:

> > 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.

Sure, and hence the style.

> 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:

I've already pointed you to the Vladimir's patch which contains at 
least a dozen of places where the same "UB issue" is reported when 
running nginx tests with UBSan.  This demonstrates that your patch 
is clearly insufficient.  Further, Vladimir's patch is clearly 
insufficient too, as shown for the another patch in the same 
patch series.

If we want to follow this approach, we need some way to trace 
places where zero length memcpy() can occur.  My best guess is 
that the only way is to look through all ngx_cpymem() / 
ngx_memcpy() / ngx_memmove() / ngx_movemem() calls, as nginx 
routinely uses { 0, NULL } as an empty string.  Given that there 
are 600+ of such calls in the codebase, and at least some need 
serious audit to find out if { 0, NULL } can appear in the call, 
this is going to be a huge work.  And, given that the only 
expected effect is theoretical correctness of the code, I doubt it 
worth the effort, especially given that the end result will likely 
reduce readability of the code.

-- 
Maxim Dounin
http://mdounin.ru/