Date: Wed, 18 Nov 2020 22:52:45 +0100 From: Stefan Esser <se@freebsd.org> To: Jessica Clarke <jrtc27@freebsd.org> Cc: src-committers <src-committers@freebsd.org>, svn-src-all <svn-src-all@freebsd.org>, svn-src-head@freebsd.org Subject: Re: svn commit: r367813 - head/lib/libutil Message-ID: <92a87ed8-4b22-4d03-9481-b02f92dcaaa0@freebsd.org> In-Reply-To: <25465269-5497-4981-A1E4-CC1FFAB68CF4@freebsd.org> References: <202011181944.0AIJiUU3003699@repo.freebsd.org> <25465269-5497-4981-A1E4-CC1FFAB68CF4@freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --CiFDVtct0WzIbzaXzTqH2tzXmzhIreK1E Content-Type: multipart/mixed; boundary="bMrkz2KlwWJC6Iq4vIixFPeZAagHYNYR2"; protected-headers="v1" From: Stefan Esser <se@freebsd.org> To: Jessica Clarke <jrtc27@freebsd.org> Cc: src-committers <src-committers@freebsd.org>, svn-src-all <svn-src-all@freebsd.org>, svn-src-head@freebsd.org Message-ID: <92a87ed8-4b22-4d03-9481-b02f92dcaaa0@freebsd.org> Subject: Re: svn commit: r367813 - head/lib/libutil References: <202011181944.0AIJiUU3003699@repo.freebsd.org> <25465269-5497-4981-A1E4-CC1FFAB68CF4@freebsd.org> In-Reply-To: <25465269-5497-4981-A1E4-CC1FFAB68CF4@freebsd.org> --bMrkz2KlwWJC6Iq4vIixFPeZAagHYNYR2 Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: quoted-printable Am 18.11.20 um 22:15 schrieb Jessica Clarke: > On 18 Nov 2020, at 19:44, Stefan E=C3=9Fer <se@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. >> + */ >=20 > 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 --bMrkz2KlwWJC6Iq4vIixFPeZAagHYNYR2-- --CiFDVtct0WzIbzaXzTqH2tzXmzhIreK1E Content-Type: application/pgp-signature; name="OpenPGP_signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="OpenPGP_signature" -----BEGIN PGP SIGNATURE----- wsB5BAABCAAjFiEEo3HqZZwL7MgrcVMTR+u171r99UQFAl+1l60FAwAAAAAACgkQR+u171r99UT2 sQf9Hsal77Y3ZKWZj6rSm3lWzux2NDffFfSREpb+KPjetq2GHicAkG9jUQWal7TsqzTZvvYocx2j hiexE2qlyVhSyQ82VDgsNshVEYgZUhP0MsNyFdZt+xHfWeWdfaHDSSMZ0JRjGOt2/UkfdzAGPBkz /WUQ9Dc86ANY0SEYuG/RmBmhhV1JcwtDIAI637BWJwZwhwYB7AWlKdUBnaQ7MsRepLfNil9Gf+BU unFZWFutsDMR4fitw2odTJiF/f8AOv2hBAkhZHF8khHfEJdjo8vrCUzTxb7FCM+sscmwy8G5gBKx xNuOt1MHrTzeVy3MuGQboKvs7INEUXsccIFopwDiew== =4vAC -----END PGP SIGNATURE----- --CiFDVtct0WzIbzaXzTqH2tzXmzhIreK1E--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?92a87ed8-4b22-4d03-9481-b02f92dcaaa0>