Re: rational behind not checking the return value of apr_palloc and apr_pcalloc
On 03 Sep 2010, at 5:31 AM, dave b wrote: Sure ok :) You have no complains from me really here. Just this could be an issue on some platform with some mods potentially :) In order to understand why it isn't an issue for httpd, you need to understand how httpd works. httpd has a thin parent process, which is responsible for spawning children as required to do the actual work. Those children doing the actual work are expendable, and if the child process dies for any reason, the parent process will spawn new child processes to replace them. This is the fundamental reason why it is so difficult to crash an httpd server, because your module code only has the power to crash one child process. If a single child process goes bananas and tries to allocate all the RAM, that child will be terminated and replaced. I only asked this list because the mod_wsgi guy wasn't checking the result of memory allocation. The rational as I see it is: there is only a few cases where this can happen 1: and 2: first the attacker has to find a way to reduce system memory to an almost oom condition by the looks of it. If the attacker has found a way to create an OOM condition, that child process will terminate and be replaced, and the attacker would have caused no lasting damage. Regards, Graham --
Re: rational behind not checking the return value of apr_palloc and apr_pcalloc
On Fri, Sep 3, 2010 at 03:49, Graham Leggett minf...@sharp.fm wrote: On 03 Sep 2010, at 5:31 AM, dave b wrote: Sure ok :) You have no complains from me really here. Just this could be an issue on some platform with some mods potentially :) In order to understand why it isn't an issue for httpd, you need to understand how httpd works. httpd has a thin parent process, which is responsible for spawning children as required to do the actual work. Those children doing the actual work are expendable, and if the child process dies for any reason, the parent process will spawn new child processes to replace them. This is the fundamental reason why it is so difficult to crash an httpd server, because your module code only has the power to crash one child process. If a single child process goes bananas and tries to allocate all the RAM, that child will be terminated and replaced. I only asked this list because the mod_wsgi guy wasn't checking the result of memory allocation. The rational as I see it is: there is only a few cases where this can happen 1: and 2: first the attacker has to find a way to reduce system memory to an almost oom condition by the looks of it. If the attacker has found a way to create an OOM condition, that child process will terminate and be replaced, and the attacker would have caused no lasting damage. Regards, Graham -- ...assuming he attacks a single httpd thread, as opposed to say a distributed attack or attack on an unrelated process. -- Sent from my toaster.
Re: rational behind not checking the return value of apr_palloc and apr_pcalloc
On Fri, Sep 3, 2010 at 07:12, Graham Leggett minf...@sharp.fm wrote: On 03 Sep 2010, at 2:37 PM, HyperHacker wrote: ...assuming he attacks a single httpd thread, as opposed to say a distributed attack or attack on an unrelated process. How would a distributed attack be different? Obviously an attack on an unrelated process would have nothing to do with checking the return value of apr_pcalloc(). Regards, Graham -- first the attacker has to find a way to reduce system memory to an almost oom condition Say, by attacking several httpd threads and/or unrelated processes to get them to eat up memory. -- Sent from my toaster.
Re: rational behind not checking the return value of apr_palloc and apr_pcalloc
On 03 Sep 2010, at 3:58 PM, HyperHacker wrote: first the attacker has to find a way to reduce system memory to an almost oom condition Say, by attacking several httpd threads and/or unrelated processes to get them to eat up memory. At which point the child processes are terminated, and httpd spawns new children to replace them. If an attacker has a way to trigger an OOM condition, that is a separate problem completely unrelated to the behavior of apr_pcalloc(). Regards, Graham --
Re: rational behind not checking the return value of apr_palloc and apr_pcalloc
first the attacker has to find a way to reduce system memory to an almost oom condition Say, by attacking several httpd threads and/or unrelated processes to get them to eat up memory. -- Sent from my toaster. If you know something why not share it ;) ? imho Apache is pretty good - so perhaps you could find a commonly used module that leaks memory? Also, I hope your toaster is running netbsd with apache ^^ -- As flies to wanton boys are we to the gods; they kill us for their sport. -- Shakespeare, King Lear
Re: rational behind not checking the return value of apr_palloc and apr_pcalloc
On Fri, Sep 3, 2010 at 13:24, dave b db.pub.m...@gmail.com wrote: first the attacker has to find a way to reduce system memory to an almost oom condition Say, by attacking several httpd threads and/or unrelated processes to get them to eat up memory. -- Sent from my toaster. If you know something why not share it ;) ? imho Apache is pretty good - so perhaps you could find a commonly used module that leaks memory? Also, I hope your toaster is running netbsd with apache ^^ -- As flies to wanton boys are we to the gods; they kill us for their sport. -- Shakespeare, King Lear Just tossing around ideas. What's the threshold for killing these child processes? What prevents someone from bringing several to just below that threshold? -- Sent from my toaster.
Re: rational behind not checking the return value of apr_palloc and apr_pcalloc
On 2 September 2010 13:29, William A. Rowe Jr. wr...@rowe-clan.net wrote: On 9/1/2010 10:17 PM, dave b wrote: Why not just fix it now and not worry? ... It will help if you can provide a specific use case for graceful failure. A segfault/dereference of NULL pointer provides a very specific exception for this general case, and 'recovers' neatly from a process which has simply consumed far too much memory. There are very few alloc exceptions which are recoverable a multi-client application. But if you can illustrate a few, the community is happy to evaluate your examples, which is what Jeff has politely suggested to you. http://blog.ksplice.com/2010/03/null-pointers-part-i/ So what if NULL doesn't crash the app ^^ ? https://www.securecoding.cert.org/confluence/display/seccode/EXP34-C.+Do+not+dereference+null+pointers I can't see MAP_FIXED anywhere in APR or apache code though. I am just suggesting it would be wise to prevent this from occurring. Not every platform / linux has /proc/sys/vm/mmap_min_addr . Sure it is not likely to be null, but it could be :) -- It is a wise father that knows his own child. -- William Shakespeare, The Merchant of Venice
Re: rational behind not checking the return value of apr_palloc and apr_pcalloc
On 9/1/2010 10:29 PM, William A. Rowe Jr. wrote: On 9/1/2010 10:17 PM, dave b wrote: Why not just fix it now and not worry? ... But if you can illustrate a few, the community is happy to evaluate your examples, which is what Jeff has politely suggested to you. And if you can't illustrate a few explicit cases, further abstract arguments are likely to be politely, but firmly, ignored. There are good C language forums for folks to carry on such religious arguments. Or to put it another way, the dev@ group here is most certainly not worried about the general case, as the current design is effective at terminating httpd when faced with runaway allocations.
Re: rational behind not checking the return value of apr_palloc and apr_pcalloc
And if you can't illustrate a few explicit cases, further abstract arguments are likely to be politely, but firmly, ignored. There are good C language forums for folks to carry on such religious arguments. Or to put it another way, the dev@ group here is most certainly not worried about the general case, as the current design is effective at terminating httpd when faced with runaway allocations. Sure ok :) You have no complains from me really here. Just this could be an issue on some platform with some mods potentially :) I only asked this list because the mod_wsgi guy wasn't checking the result of memory allocation. The rational as I see it is: there is only a few cases where this can happen 1: and 2: first the attacker has to find a way to reduce system memory to an almost oom condition by the looks of it. -- I dote on his very absence. -- William Shakespeare, The Merchant of Venice
Re: rational behind not checking the return value of apr_palloc and apr_pcalloc
On 01 Sep 2010, at 6:07 AM, dave b wrote: What is the rational behind not checking the return value of apr_palloc and apr_pcalloc? The rationale is to not be forced to check for and handle hundreds of potential failure cases when you're probably doomed anyway. The APR pools API gives you the apr_pool_abort_set() function, which specifies a function to call if the memory allocation fails. In the case of httpd, a function is registered which gracefully shuts down that particular server process if the allocation fails, and apr_palloc() is in the process guaranteed to never return NULL. Obviously if you're not using APR from httpd, or if you're writing a library that depends on APR, and you haven't set an abort function, NULL will potentially be returned and you should check for and handle that case. Regards, Graham --
Re: rational behind not checking the return value of apr_palloc and apr_pcalloc
On Wed, Sep 1, 2010 at 6:15 AM, Graham Leggett minf...@sharp.fm wrote: On 01 Sep 2010, at 6:07 AM, dave b wrote: What is the rational behind not checking the return value of apr_palloc and apr_pcalloc? The rationale is to not be forced to check for and handle hundreds of potential failure cases when you're probably doomed anyway. probably more than hundreds ;) If there's a real world scenario where allocation failures can occur and must be dealt with more gracefully than segfaulting, I suspect that you can find a pragmatic way to deal with it much more reliably than relying on each individual memory allocation to be checked (that will never be implemented perfectly, and those paths will almost never be exercised anyway). For example, a plug-in module might be able to confirm (or fail gracefully) in an early request hook that enough memory is available to handle the expected types of requests. Another way to look at it: If somebody had the time to add all those checks/error paths, their time would be better spent looking for situations where httpd would use a lot more memory than normal because of the way external input was received. If there's not a repeatable real world scenario to address -- IOW you think they should be checked just because -- there probably won't be any sympathy here. With a particular scenario in hand there may be ideas forthcoming to deal with the situation, whether internal to the web server or external. HTH!
Re: rational behind not checking the return value of apr_palloc and apr_pcalloc
On 1 September 2010 20:15, Graham Leggett minf...@sharp.fm wrote: On 01 Sep 2010, at 6:07 AM, dave b wrote: What is the rational behind not checking the return value of apr_palloc and apr_pcalloc? The rationale is to not be forced to check for and handle hundreds of potential failure cases when you're probably doomed anyway. The APR pools API gives you the apr_pool_abort_set() function, which specifies a function to call if the memory allocation fails. In the case of httpd, a function is registered which gracefully shuts down that particular server process if the allocation fails, and apr_palloc() is in the process guaranteed to never return NULL. Noting that apr_pool_abort_set() is only setup in Apache 2.3 and not in Apache 2.2.16 or earlier. Not being in 2.X explains why I couldn't find it before. Any reason why setting up apr_pool_abort_set() wasn't back ported to Apache 2.2? Graham Obviously if you're not using APR from httpd, or if you're writing a library that depends on APR, and you haven't set an abort function, NULL will potentially be returned and you should check for and handle that case. Regards, Graham --
Re: rational behind not checking the return value of apr_palloc and apr_pcalloc
On Wed, Sep 1, 2010 at 6:37 AM, Graham Dumpleton graham.dumple...@gmail.com wrote: On 1 September 2010 20:15, Graham Leggett minf...@sharp.fm wrote: On 01 Sep 2010, at 6:07 AM, dave b wrote: What is the rational behind not checking the return value of apr_palloc and apr_pcalloc? The rationale is to not be forced to check for and handle hundreds of potential failure cases when you're probably doomed anyway. The APR pools API gives you the apr_pool_abort_set() function, which specifies a function to call if the memory allocation fails. In the case of httpd, a function is registered which gracefully shuts down that particular server process if the allocation fails, and apr_palloc() is in the process guaranteed to never return NULL. Noting that apr_pool_abort_set() is only setup in Apache 2.3 and not in Apache 2.2.16 or earlier. Not being in 2.X explains why I couldn't find it before. Any reason why setting up apr_pool_abort_set() wasn't back ported to Apache 2.2? dunno, but here's the commit and original discussion in case anyone wants to read http://svn.apache.org/viewvc?view=revisionrevision=406953 (dunno if it was subsequently fixed) http://www.mail-archive.com/dev@httpd.apache.org/msg32257.html Graham Obviously if you're not using APR from httpd, or if you're writing a library that depends on APR, and you haven't set an abort function, NULL will potentially be returned and you should check for and handle that case. Regards, Graham -- -- Born in Roswell... married an alien...
Re: rational behind not checking the return value of apr_palloc and apr_pcalloc
On 1 September 2010 22:08, Jeff Trawick traw...@gmail.com wrote:
On Wed, Sep 1, 2010 at 6:37 AM, Graham Dumpleton
graham.dumple...@gmail.com wrote:
On 1 September 2010 20:15, Graham Leggett minf...@sharp.fm wrote:
On 01 Sep 2010, at 6:07 AM, dave b wrote:
What is the rational behind not checking the return value of
apr_palloc and apr_pcalloc?
The rationale is to not be forced to check for and handle hundreds of
potential failure cases when you're probably doomed anyway.
Yes I agree :)
Hence, why the patch I put in the bug report used assert :)
The APR pools API gives you the apr_pool_abort_set() function, which
specifies a function to call if the memory allocation fails. In the case
of
httpd, a function is registered which gracefully shuts down that
particular
server process if the allocation fails, and apr_palloc() is in the
process
guaranteed to never return NULL.
Noting that apr_pool_abort_set() is only setup in Apache 2.3 and not
in Apache 2.2.16 or earlier. Not being in 2.X explains why I couldn't
find it before.
Any reason why setting up apr_pool_abort_set() wasn't back ported to
Apache 2.2?
dunno, but here's the commit and original discussion in case anyone wants to
read
http://svn.apache.org/viewvc?view=revisionrevision=406953 (dunno if it was
subsequently fixed)
http://www.mail-archive.com/dev@httpd.apache.org/msg32257.html
Interesting patch, um but... I think there maybe a problem here:
from apr :)
include/apr_pools.h
#if defined(DOXYGEN)
APR_DECLARE(apr_status_t) apr_pool_create(apr_pool_t **newpool,
apr_pool_t *parent);
#else
#if APR_POOL_DEBUG
#define apr_pool_create(newpool, parent) \
apr_pool_create_ex_debug(newpool, parent, NULL, NULL, \
APR_POOL__FILE_LINE__)
#else
#define apr_pool_create(newpool, parent) \
apr_pool_create_ex(newpool, parent, NULL, NULL)
#endif
#endif
And apr_pool_create_ex is:
APR_DECLARE(apr_status_t) apr_pool_create_ex(apr_pool_t **newpool,
apr_pool_t *parent,
apr_abortfunc_t abort_fn,
apr_allocator_t *allocator);
First thing to note is that in main apr_app_initialize[1] in
server/main.c via init_process. When this happens the global_pool will
not have a abort_fn. *However*, cntx will (it is set :) ),
process-pconf will.
Now we look in the actual code :)
See [0]
Right so if apr_pool_create gets called with null as the parent from
now on - there will be no default abort_fn.
Lets go hunting for this happens ;) see [2]
Ok so lets pull out the scoreboard now ?
/* ToDo: This function should be made to handle setting up
* a scoreboard shared between processes using any IPC technique,
* not just a shared memory segment
*/
static apr_status_t open_scoreboard(apr_pool_t *pconf)
{
#if APR_HAS_SHARED_MEMORY
apr_status_t rv;
char *fname = NULL;
apr_pool_t *global_pool;
/* We don't want to have to recreate the scoreboard after
* restarts, so we'll create a global pool and never clean it.
*/
rv = apr_pool_create(global_pool, NULL);
MMM I wonder where else httpd does these kind of things ;)
Remember my simple grep can *easily* miss some :)
Ok great just get the rest of world to use apache 2.3 which has had
this patch for over 4 years now. Just one problem: I don't see anyone
running apache 2.3 alpha 8 on their production servers.
So perhaps apache 2.2 should get a backport of this 'fix' :-)
Apparently my fix that I suggested on the bug referenced in my
original email is also not going to work :) (it will not be enough :-)
)
Also, note I hit that code path via apr testdir (in one of the tests
;) ) - and I was not out of memory - I haven't checked which test it
is yet (it would be nice if the apr tests told me this...).
If the caller has hit that code path and they haven't defined an
abort_fn, I think it best to exit / abort for them - If a user wants
not to have that happen then they can just simply ensure they are
providing their own abort_fn :)
A little less conversation, a little more action please.
[0]
APR_DECLARE(apr_status_t) apr_pool_create_ex(apr_pool_t **newpool,
apr_pool_t *parent,
apr_abortfunc_t abort_fn,
apr_allocator_t *allocator)
{
apr_pool_t *pool;
apr_memnode_t *node;
*newpool = NULL;
if (!parent)
parent = global_pool;
/* parent will always be non-NULL here except the first time a
* pool is created, in which case allocator is guaranteed to be
* non-NULL. */
if (!abort_fn parent)
abort_fn = parent-abort_fn;
if (allocator == NULL)
allocator = parent-allocator;
if ((node = allocator_alloc(allocator,
MIN_ALLOC -
Re: rational behind not checking the return value of apr_palloc and apr_pcalloc
On Wed, Sep 1, 2010 at 2:49 PM, dave b db.pub.m...@gmail.com wrote:
On 1 September 2010 22:08, Jeff Trawick traw...@gmail.com wrote:
On Wed, Sep 1, 2010 at 6:37 AM, Graham Dumpleton
graham.dumple...@gmail.com wrote:
On 1 September 2010 20:15, Graham Leggett minf...@sharp.fm wrote:
On 01 Sep 2010, at 6:07 AM, dave b wrote:
What is the rational behind not checking the return value of
apr_palloc and apr_pcalloc?
The rationale is to not be forced to check for and handle hundreds of
potential failure cases when you're probably doomed anyway.
Yes I agree :)
Hence, why the patch I put in the bug report used assert :)
The APR pools API gives you the apr_pool_abort_set() function, which
specifies a function to call if the memory allocation fails. In the
case
of
httpd, a function is registered which gracefully shuts down that
particular
server process if the allocation fails, and apr_palloc() is in the
process
guaranteed to never return NULL.
Noting that apr_pool_abort_set() is only setup in Apache 2.3 and not
in Apache 2.2.16 or earlier. Not being in 2.X explains why I couldn't
find it before.
Any reason why setting up apr_pool_abort_set() wasn't back ported to
Apache 2.2?
dunno, but here's the commit and original discussion in case anyone wants
to
read
http://svn.apache.org/viewvc?view=revisionrevision=406953 (dunno if it
was
subsequently fixed)
http://www.mail-archive.com/dev@httpd.apache.org/msg32257.html
Interesting patch, um but... I think there maybe a problem here:
from apr :)
include/apr_pools.h
#if defined(DOXYGEN)
APR_DECLARE(apr_status_t) apr_pool_create(apr_pool_t **newpool,
apr_pool_t *parent);
#else
#if APR_POOL_DEBUG
#define apr_pool_create(newpool, parent) \
apr_pool_create_ex_debug(newpool, parent, NULL, NULL, \
APR_POOL__FILE_LINE__)
#else
#define apr_pool_create(newpool, parent) \
apr_pool_create_ex(newpool, parent, NULL, NULL)
#endif
#endif
And apr_pool_create_ex is:
APR_DECLARE(apr_status_t) apr_pool_create_ex(apr_pool_t **newpool,
apr_pool_t *parent,
apr_abortfunc_t abort_fn,
apr_allocator_t *allocator);
First thing to note is that in main apr_app_initialize[1] in
server/main.c via init_process. When this happens the global_pool will
not have a abort_fn. *However*, cntx will (it is set :) ),
process-pconf will.
Now we look in the actual code :)
See [0]
Right so if apr_pool_create gets called with null as the parent from
now on - there will be no default abort_fn.
Lets go hunting for this happens ;) see [2]
Ok so lets pull out the scoreboard now ?
/* ToDo: This function should be made to handle setting up
* a scoreboard shared between processes using any IPC technique,
* not just a shared memory segment
*/
static apr_status_t open_scoreboard(apr_pool_t *pconf)
{
#if APR_HAS_SHARED_MEMORY
apr_status_t rv;
char *fname = NULL;
apr_pool_t *global_pool;
/* We don't want to have to recreate the scoreboard after
* restarts, so we'll create a global pool and never clean it.
*/
rv = apr_pool_create(global_pool, NULL);
MMM I wonder where else httpd does these kind of things ;)
Remember my simple grep can *easily* miss some :)
My 2 cents:
I doubt that any of the core devs are going to match you for devotion to
this topic, but I'm sure we will review patches to trunk to fix somewhat
practical scenarios, such as ensuring that memory allocation failures during
request processing go through the common abort function, if the existing
code doesn't already handle that. (OTOH, I wouldn't think many would find
addressing un-aborted alloc failures during initialization so interesting.)
That requires some investigation/understanding of the context of the code
that passes NULL for the parent pool, which hopefully you can make an
attempt at without dumping the grep output and selected source code to the
mailing list ;)
(I know that sounds rude, but its my best advice to pursuing that thread of
investigation.)
Ok great just get the rest of world to use apache 2.3 which has had
this patch for over 4 years now. Just one problem: I don't see anyone
running apache 2.3 alpha 8 on their production servers.
So perhaps apache 2.2 should get a backport of this 'fix' :-)
Apparently my fix that I suggested on the bug referenced in my
original email is also not going to work :) (it will not be enough :-)
)
What's the big picture here from your standpoint? Why is 2.2 noticeably
impaired without such changes? Are there really many users who need that
abort function to tell them that httpd can't alloc more heap? Anyway, if
2.3 doesn't really have this suitably addressed it would be better to get
that finished first.
Also, note
Re: rational behind not checking the return value of apr_palloc and apr_pcalloc
My 2 cents: I doubt that any of the core devs are going to match you for devotion to this topic, but I'm sure we will review patches to trunk to fix somewhat practical scenarios, such as ensuring that memory allocation failures during request processing go through the common abort function, if the existing code doesn't already handle that. (OTOH, I wouldn't think many would find addressing un-aborted alloc failures during initialization so interesting.) Sorry I have a habit of measuring things twice and cutting once ;) That requires some investigation/understanding of the context of the code that passes NULL for the parent pool, which hopefully you can make an attempt at without dumping the grep output and selected source code to the mailing list ;) (I know that sounds rude, but its my best advice to pursuing that thread of investigation.) It doesn't sound rude. It sounds like you want me to do the work for you. The code dump and grep was to give some examples. I think without the code dump some would have problems following that the global_pool abort_fn is not to abort when it would otherwise return NULL ( I believe this to be the case atm at least :) ). What's the big picture here from your standpoint? Why is 2.2 noticeably impaired without such changes? Are there really many users who need that abort function to tell them that httpd can't alloc more heap? Anyway, if 2.3 doesn't really have this suitably addressed it would be better to get that finished first. I would need to do a lot more testing and I don't have much time for that presently. I will have a poke around later in a vm after making some modifications to the code base to try and trigger somethings :) IMHO the apr init function, which inits the global_pool, could take in an argument for the default abort_fn. Also, note I hit that code path via apr testdir (in one of the tests ;) ) - and I was not out of memory - I haven't checked which test it is yet (it would be nice if the apr tests told me this...). If the caller has hit that code path and they haven't defined an abort_fn, I think it best to exit / abort for them - If a user wants not to have that happen then they can just simply ensure they are providing their own abort_fn :) d...@apr.apache.org, but first try gdb or testall -v or anything else that you need to find exactly what/where the failure is ah there is a -v ! -- The ripest fruit falls first. -- William Shakespeare, Richard II
Re: rational behind not checking the return value of apr_palloc and apr_pcalloc
On Wed, Sep 1, 2010 at 4:09 PM, dave b db.pub.m...@gmail.com wrote: My 2 cents: I doubt that any of the core devs are going to match you for devotion to this topic, but I'm sure we will review patches to trunk to fix somewhat practical scenarios, such as ensuring that memory allocation failures during request processing go through the common abort function, if the existing code doesn't already handle that. (OTOH, I wouldn't think many would find addressing un-aborted alloc failures during initialization so interesting.) Sorry I have a habit of measuring things twice and cutting once ;) That requires some investigation/understanding of the context of the code that passes NULL for the parent pool, which hopefully you can make an attempt at without dumping the grep output and selected source code to the mailing list ;) (I know that sounds rude, but its my best advice to pursuing that thread of investigation.) It doesn't sound rude. It sounds like you want me to do the work for you. no, I don't want you to do anything for me; I'm just sharing my educated guess at what it takes to make progress on this topic you're apparently very interested in with a little luck you'll be able to find somebody here to analyze the code you pointed out to see which cases actually matter, and to what extent The code dump and grep was to give some examples. I think without the code dump some would have problems following that the global_pool abort_fn is not to abort when it would otherwise return NULL ( I believe this to be the case atm at least :) ). What's the big picture here from your standpoint? Why is 2.2 noticeably impaired without such changes? Are there really many users who need that abort function to tell them that httpd can't alloc more heap? Anyway, if 2.3 doesn't really have this suitably addressed it would be better to get that finished first. I would need to do a lot more testing and I don't have much time for that presently. I will have a poke around later in a vm after making some modifications to the code base to try and trigger somethings :) IMHO the apr init function, which inits the global_pool, could take in an argument for the default abort_fn. Also, note I hit that code path via apr testdir (in one of the tests ;) ) - and I was not out of memory - I haven't checked which test it is yet (it would be nice if the apr tests told me this...). If the caller has hit that code path and they haven't defined an abort_fn, I think it best to exit / abort for them - If a user wants not to have that happen then they can just simply ensure they are providing their own abort_fn :) d...@apr.apache.org, but first try gdb or testall -v or anything else that you need to find exactly what/where the failure is ah there is a -v ! -- The ripest fruit falls first. -- William Shakespeare, Richard II -- Born in Roswell... married an alien...
Re: rational behind not checking the return value of apr_palloc and apr_pcalloc
no, I don't want you to do anything for me; I'm just sharing my educated guess at what it takes to make progress on this topic you're apparently very interested in with a little luck you'll be able to find somebody here to analyze the code you pointed out to see which cases actually matter, and to what extent Why not just fix it now and not worry? ... -- For a light heart lives long. -- Shakespeare, Love's Labour's Lost
Re: rational behind not checking the return value of apr_palloc and apr_pcalloc
On 9/1/2010 10:17 PM, dave b wrote: Why not just fix it now and not worry? ... It will help if you can provide a specific use case for graceful failure. A segfault/dereference of NULL pointer provides a very specific exception for this general case, and 'recovers' neatly from a process which has simply consumed far too much memory. There are very few alloc exceptions which are recoverable a multi-client application. But if you can illustrate a few, the community is happy to evaluate your examples, which is what Jeff has politely suggested to you.
Re: rational behind not checking the return value of apr_palloc and apr_pcalloc
On 1 September 2010 14:07, dave b db.pub.m...@gmail.com wrote:
What is the rational behind not checking the return value of
apr_palloc and apr_pcalloc?
Specifically here talking about why HTTPD code doesn't check. Ie.,
core server code and modules supplied with HTTPD.
I am clarifying this because he is hitting up on me as to why mod_wsgi
doesn't do it, yet the HTTPD code itself doesn't do it and I am just
following that precedent. So suggested he ask here why there is no
practice of checking for NULL values in HTTPD code when doing
allocations against pools. :-)
Graham
code memory/unix/apr_pools.c from apr-1.4.2
APR_DECLARE(void *) apr_pcalloc(apr_pool_t *pool, apr_size_t size);
APR_DECLARE(void *) apr_pcalloc(apr_pool_t *pool, apr_size_t size)
{
void *mem;
if ((mem = apr_palloc(pool, size)) != NULL) {
memset(mem, 0, size);
}
return mem;
}
and
apr_palloc can return NULL.
So I modified the code and the testdir test failed in one place -
node = active-next;
if (size = node_free_space(node)) {
list_remove(node);
}
else {
if ((node = allocator_alloc(pool-allocator, size)) == NULL) {
if (pool-abort_fn)
pool-abort_fn(APR_ENOMEM); /* HERE */
return NULL;
}
}
When you run the testdir (test). If you change the above to be:
.
if ((node = allocator_alloc(pool-allocator, size)) == NULL) {
if (! pool-abort_fn) /* note the ! added */
pool-abort_fn(APR_ENOMEM);
return NULL; /* you end up here */
}
}
and you will fail one of the tests. This to me suggests that this scenario is
possible if the pool is like that one failed test *but* pool-abort_fn is not
true :)
So what is the rational behind most users of these method *not*
checking the return code - because from what I have seen / know it is
possible return NULL.
Also see: https://issues.apache.org/bugzilla/show_bug.cgi?id=49847
