Skip site navigation (1)Skip section navigation (2)
Date:      Sun, 01 Dec 2013 15:46:34 +0100
From:      Alex <alex@kaworu.ch>
To:        freebsd-pkg@freebsd.org
Subject:   Re: [dev] about unchecked functions calls
Message-ID:  <529B4BCA.2090305@kaworu.ch>
In-Reply-To: <5299B7AC.1000603@infracaninophile.co.uk>
References:  <529909B9.2050007@kaworu.ch> <5299B7AC.1000603@infracaninophile.co.uk>

next in thread | previous in thread | raw e-mail | index | archive | help
On 11/30/2013 11:02 AM, Matthew Seaman wrote:
> On 29/11/2013 21:40, Alex wrote:
>> About solutions, the alternatives I have in mind are:
>> 
>> 1. assume that ENOMEM is fatal. We just abort() as soon as we see
>> it. In that case we should have some kind of xmalloc(3) & Co. and
>> use them consistently accross all the project. On the pros side
>> it's very easy to do, on the cons it could leave the system in a
>> unclean state (half done install / uninstall, corrupted pkg
>> database etc.).
> 
> abort(3) is almost certainly the wrong answer.  If malloc(3) et al
> fail, then, yes pkg(8) should terminate itself.  But it should
> rollback any pending database transactions and perform other
> cleanup and generally take care to leave the system in a consistent
> state.
> 
> [...]

I fully agree. As pointed later, I suggest it as a temporary
"transition" solution, because the actual codebase contains a lot of
unchecked malloc(3) which result inevitably to deference NULL at some
point (the problem being that it would be hard to reproduce and the
error might not be obviously related to ENOMEM).

>> steps could (and should) also be applied for other other calls
>> like strlcpy(3). However, in order to succeed cooperation is
>> required at
> 
> Huh?  What's wrong with strlcpy(3)?  strlcpy is a *good thing* and 
> should be used in preference to strcpy(3) or strncpy(3).  Which we 
> generally do already.  Similarly snprintf(3) rather than sprintf(3)
> and similarly bounded functions which won't overrun memory
> buffers.
> 
> Matthew

The issue is not strlcpy(3) itself (nor it is the case with malloc(3))
but unchecked calls. Unchecked strlcpy(3) can lead to C-string
truncation which is a very vicious type of bug often very hard to
debug. Again, I agree with you that strlcpy(3) is superior to the
standard functions it aim to replace, but unchecked it just means you
know it won't overflow and then it's just a slow memcpy(3). Of course,
there are legitimate cases but then the return value should explicitly
be discarded for clarity's sake by casting to `void' (as suggested by
style(9)).

Regards,
Alex.



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?529B4BCA.2090305>