Skip site navigation (1)Skip section navigation (2)
Date:      Sun, 26 Jul 2009 08:50:15 -0600
From:      Jamie Gritton <jamie@FreeBSD.org>
To:        Kostik Belousov <kostikbel@gmail.com>
Cc:        "Bjoern A. Zeeb" <bzeeb-lists@lists.zabbadoz.net>, freebsd-stable <freebsd-stable@FreeBSD.org>
Subject:   Re: regression with jexec?
Message-ID:  <4A6C6D27.2030500@FreeBSD.org>
In-Reply-To: <20090726144227.GK55190@deviant.kiev.zoral.com.ua>
References:  <4A6B0BD3.6040206@protected-networks.net> <4A6B9A60.90302@FreeBSD.org> <4A6BAC1A.5080303@protected-networks.net> <20090726120608.GE55190@deviant.kiev.zoral.com.ua> <20090726122230.E245@maildrop.int.zabbadoz.net> <4A6C67F5.8080408@FreeBSD.org> <20090726144227.GK55190@deviant.kiev.zoral.com.ua>

next in thread | previous in thread | raw e-mail | index | archive | help
Kostik Belousov wrote:
> On Sun, Jul 26, 2009 at 08:28:05AM -0600, Jamie Gritton wrote:
>> Bjoern A. Zeeb wrote:
>>> On Sun, 26 Jul 2009, Kostik Belousov wrote:
>>>
>>>> On Sat, Jul 25, 2009 at 09:06:34PM -0400, Michael Butler wrote:
>>>>> -----BEGIN PGP SIGNED MESSAGE-----
>>>>> Hash: SHA1
>>>>>
>>>>> Jamie Gritton wrote:
>>>>>> Michael Butler wrote:
>>>>>>> imb@aaron:/home/imb> sudo jexec 5 tcsh
>>>>>>> jexec: Unable to parse jail ID.: No such file or directory
>>>>>> The symptom in jexec can be fixed by this little patch:
>>>>>>
>>>>>> Index: usr.sbin/jexec/jexec.c
>>>>>> ===================================================================
>>>>>> --- usr.sbin/jexec/jexec.c    (revision 195879)
>>>>>> +++ usr.sbin/jexec/jexec.c    (working copy)
>>>>>> @@ -248,6 +248,7 @@
>>>>>>     if (argc < 2)
>>>>>>         usage();
>>>>>>     if (strlen(argv[0]) > 0) {
>>>>>> +        errno = 0;
>>>>>>         jid = (int)strtol(argv[0], NULL, 10);
>>>>>>         if (errno)
>>>>>>             err(1, "Unable to parse jail ID.");
>>>>> Thanks - this certainly cures the effect.
>>>>>
>>>>>> But the broader problem is malloc.  It's leaving errno set to
>>>>>> ENOENT when /etc/malloc.conf doesn't exist.  This seems like
>>>>>> wrong behavior to me.
>>>>> Seems like a POLA violation to me,
>>>> No, this is how errno generally work, it is not changed if no error
>>>> happens.
>>> I haven't really understood which part, when and why would set the errno in
>>> first place so that it would still be there?  Is it something in jexec
>>> that gets the errno in first place or is it something internal to
>>> malloc that sets it returns successfully and doesn't clear it?
>> The POLA violation is in malloc - it sets errno even when there was no
>> error.  The allocation succeeded and a pointer was returned, yet errno
>> was set to ENOENT (not even an error malloc should be able to return).
>> The fact that malloc looks for an optional config file and doesn't find
>> one shouldn't be relayed back to the caller in errno.  If
>> /etc/malloc.conf doesn't exist, it should either clear errno after that,
>> or perhaps restore its previous value.  There's also a
>> getenv("MALLOC_OPTIONS") that can similarly set errno.
>>
>> Perhaps this has all been gone over before and I missed it (this is from
>> code that's been stable since 2006), so I wouldn't want to just rush in
>> and fix malloc.  Maybe this general principle has already been discussed
>> and my viewpoint lost.  But if not, it's my opinion that malloc is
>> acting badly and needs a change.
>>
>> In the meantime, I have no problem with errno not being cleared in
>> strtol, and the patch to jexec is correct (though strictly speaking it
>> shouldn't be necessary since we "know" errno has not yet been set).
> Let me restate that errno is never cleared when no error occured.
> It is only set when error took place.
> 
> E.g., for syscalls, you should look into errno only if syscall returned
> -1, in most cases.
> 
> For non-syscall libc functions, you need to clear errno before the call,
> then check for the error return (for strtoul that would be ULONG_MAX
> or 0), that seems to be missed from your patch, and only then look for
> errno.
> 
> See RETURN VALUES section of the strtoul(3).

All true - and I'll add the check you mention to my patch.

But what about the malloc case?  Is it equally valid to say that errno
should not be set when no error occurred?  Or are non-syscall libc
functions generally given free reign to overwrite errno in non-error
situations?

- Jamie



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?4A6C6D27.2030500>