Welcome! Log In Create A New Profile

Advanced

Re: Core: Avoid memcpy from NULL

This forum is currently read only. You can not log in or make any changes. This is a temporary situation.
Maxim Dounin
January 04, 2024 11:12AM
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/
_______________________________________________
nginx-devel mailing list
nginx-devel@nginx.org
https://mailman.nginx.org/mailman/listinfo/nginx-devel
Subject Author Views Posted

Core: Avoid memcpy from NULL

Ben Kallus 689 December 13, 2023 11:10AM

Re: Core: Avoid memcpy from NULL

Maxim Dounin 154 December 14, 2023 09:44PM

Re: Core: Avoid memcpy from NULL

Dipl. Ing. Sergey Brester via nginx-devel 151 December 15, 2023 09:48AM

Re: Core: Avoid memcpy from NULL

Ben Kallus 123 December 15, 2023 11:30AM

Re: Core: Avoid memcpy from NULL

Maxim Dounin 138 December 15, 2023 11:18PM

Re: Core: Avoid memcpy from NULL

Ben Kallus 154 December 16, 2023 04:28PM

Re: Core: Avoid memcpy from NULL

Maxim Dounin 149 December 20, 2023 07:36PM

Re: Core: Avoid memcpy from NULL

Ben Kallus 127 December 29, 2023 11:52AM

Re: Core: Avoid memcpy from NULL

Maxim Dounin 151 December 31, 2023 02:54PM

Re: Core: Avoid memcpy from NULL

Ben Kallus 137 January 03, 2024 07:00PM

Re: Core: Avoid memcpy from NULL

Maxim Dounin 141 January 04, 2024 11:12AM

Re: Core: Avoid memcpy from NULL

Ben Kallus 135 January 09, 2024 11:20AM

Re: Core: Avoid memcpy from NULL

Maxim Dounin 119 January 09, 2024 02:26PM

Re: Core: Avoid memcpy from NULL

Ben Kallus 143 January 23, 2024 07:10PM

Re: Core: Avoid memcpy from NULL

Maxim Dounin 118 January 24, 2024 03:58AM

Re: Core: Avoid memcpy from NULL

karton 131 January 24, 2024 10:54PM



Online Users

Guests: 128
Record Number of Users: 8 on April 13, 2023
Record Number of Guests: 500 on July 15, 2024
Powered by nginx      Powered by FreeBSD      PHP Powered      Powered by MariaDB      ipv6 ready