<div dir="auto"><div><br><br><div class="gmail_quote gmail_quote_container"><div dir="ltr" class="gmail_attr">On Sun, Jan 12, 2025, 5:21 AM James Browning via devel <<a href="mailto:devel@ntpsec.org">devel@ntpsec.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">On Saturday, January 11, 2025 10:43:47 PM Pacific Standard Time Hal Murray via <br>
devel wrote:<br>
> The Linux man page says<br>
>        If ptr is NULL, no operation is performed.<br>
> <br>
> I'd expect a wrapper if there really was a problem.<br>
> <br>
> This code is in ntpd/ntp_config.c<br>
>         if (NULL == s) {                        /* free() hates NULL */<br>
>                 s = estrdup("");<br>
>         }<br>
> <br>
> Any reason not to nuke that chunk of code?<br>
<br>
Have we looked beyond the sound bite? A bush league check by this armchair <br>
asshole suggest that a NULL that slips through here can go into estrdup, <br>
strlen, get_logmask, stats_config, or other places I haven't found yet. Just be <br>
aware that sucker is a stopper; it would be prudent to check everything <br>
downstream.<br></blockquote></div></div><div dir="auto"><br></div><div dir="auto">If you don't want to pass a NULL into a libc or POSIX call, you can't depend on any of them checking. There is an assumption that dereferencing a NULL pointer will cause a fault so in the interest of performance (e.g. micro optimization), the checks are often left out </div><div dir="auto"><br></div><div dir="auto">Some prototypes in header files will specify that an argument cannot be null. If you are lucky, that will catch some null cases at compile time.</div><div dir="auto"><br></div><div dir="auto">This assumption is invalid in environments without an MMU. </div><div dir="auto"><br></div><div dir="auto">In practice, you should check every return value and argument.</div><div dir="auto"><br></div><div dir="auto">I've taken to recommending a set of precondition and post condition debug asserts for most of what we write. Use project specific macros so you can tweak the information and have options beyond what assert does.</div><div dir="auto"><br></div><div dir="auto">But you have to have robust tests inside the project for this to catch all the cases. Otherwise, you leave the checks and turn the action into something appropriate to be fielded.</div><div dir="auto"><br></div><div dir="auto"><div class="gmail_quote gmail_quote_container"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
Ah, it looks like estrdup and strlen at least would react poorly[1] to being <br>
fed a null from the wrapping code. huh.<br>
<br>
[1] <a href="https://stackoverflow.com/questions/5796103/strlen-not-checking-for-null" rel="noreferrer noreferrer" target="_blank">https://stackoverflow.com/questions/5796103/strlen-not-checking-for-null</a></blockquote></div></div><div dir="auto"><br></div><div dir="auto">--joel</div><div dir="auto"><div class="gmail_quote gmail_quote_container"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><br>
<br>
<br>
_______________________________________________<br>
devel mailing list<br>
<a href="mailto:devel@ntpsec.org" target="_blank" rel="noreferrer">devel@ntpsec.org</a><br>
<a href="https://lists.ntpsec.org/mailman/listinfo/devel" rel="noreferrer noreferrer" target="_blank">https://lists.ntpsec.org/mailman/listinfo/devel</a><br>
</blockquote></div></div></div>