svn commit: r367813 - head/lib/libutil

Stefan Esser se at freebsd.org
Wed Nov 18 21:52:49 UTC 2020



Am 18.11.20 um 22:15 schrieb Jessica Clarke:
> On 18 Nov 2020, at 19:44, Stefan Eßer <se at freebsd.org> wrote:
>> +		/*
>> +		 * Check for some other thread already having
>> +		 * set localbase - this should use atomic ops.
>> +		 * The amount of memory allocated above may leak,
>> +		 * if a parallel update in another thread is not
>> +		 * detected and the non-NULL pointer is overwritten.
>> +		 */
> 
> Why was this committed with a known racy/leaky implementation?

Because the alternatives that I offered for discussion were
less acceptable.

> What happens if I set the value with a sysctl and call this?

You'll get the value set with sysctl, unless overridden by the
environment variable. There is a window of a few nano-seconds
where a thread executing in parallel on another core might be
able to set the localbase variable (between the test for NULL
in this function and the assignment to it). The value that will
be returned by either thread will be identical, so there is no
risk of corruption of the result.

This unlikely case may actually leak a heap allocated string
of typically tens of bytes (but with negligible probability).

But this really is a non-issue, since there should never be a
reason to invoke this function in a multi-threaded context.

The result should be constant for the duration of execution
of the process (expect severe inconsistencies if that was not
the case) and all programs in base that are candidates for the
use of this function are non-threaded (and if they were multi-
threaded, then I'd expect this call to occur during start-up
of the program before any further threads are created).

So, this is a non-issue and the comment tries to explain it.
Did I fail to make this clear in the comment? Maybe I should
have written "could use atomic ops" instead?

Use of atomics or locks could prevent the race-condition. But
since I do not expect this function to be called from within
threads (it just doesn't make sense), the tiny time window of
a few nano-seconds which might lead to a double assignment to
the target variable (with one pointer value being lost), and
the worst case loss of 1KB of heap space in that case (more
likely 10 to 20 bytes rounded up to a 16 or 32 byte chunk), I
do not consider the complexities of either a lock or atomic ops
to be justified.

Regards, STefan

-------------- next part --------------
A non-text attachment was scrubbed...
Name: OpenPGP_signature
Type: application/pgp-signature
Size: 495 bytes
Desc: OpenPGP digital signature
URL: <http://lists.freebsd.org/pipermail/svn-src-head/attachments/20201118/43c78736/attachment.sig>


More information about the svn-src-head mailing list