Re: [PHP-DEV] [PATCH] Zend/zend_alloc.c
Stanislav Malyshev [EMAIL PROTECTED] writes: JG I disagree with this patch. The scenario of not being able to JG allocate memory is a fatal error, and the only appropriate JG response for php is to exit. If you need other behavior use JG pemalloc( which calls malloc if set persistant ). Actually, there's a lot of code in PHP that is based on the assumption that emalloc never fails. If emalloc can return NULLs, all this code should be rewritten. I do not see any point in it and agree with Jason. You are right, but this feature (emalloc never fails) is undocumented and both php and zend are full of code that just check the emalloc return value. You can find some example also in zend_alloc.c. Changing emalloc to return NULLs does not modify the program behaviour: you (probably) receive a SIGSEGV from the emalloc caller and the execution terminate. I can't see a big difference in this case, but if you check for NULL you can fail gracefully. Ciao -- Walter Franzini, e-mail: [EMAIL PROTECTED] SysNet, Via Digione 8, 27100 Pavia - Italy -- PHP Development Mailing List http://www.php.net/ To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED] To contact the list administrators, e-mail: [EMAIL PROTECTED]
Re: [PHP-DEV] [PATCH] Zend/zend_alloc.c
There's a big difference between an undefined behavior which may result in a crash, and an organized shutdown. The former is dangerous, the latter is not. There are some parts of code that don't rely on this, from the times when it wasn't true, but it doesn't cause any problem, so there's no reason to change it urgently. Zeev At 09:54 29-08-01, Walter Franzini wrote: Stanislav Malyshev [EMAIL PROTECTED] writes: JG I disagree with this patch. The scenario of not being able to JG allocate memory is a fatal error, and the only appropriate JG response for php is to exit. If you need other behavior use JG pemalloc( which calls malloc if set persistant ). Actually, there's a lot of code in PHP that is based on the assumption that emalloc never fails. If emalloc can return NULLs, all this code should be rewritten. I do not see any point in it and agree with Jason. You are right, but this feature (emalloc never fails) is undocumented and both php and zend are full of code that just check the emalloc return value. You can find some example also in zend_alloc.c. Changing emalloc to return NULLs does not modify the program behaviour: you (probably) receive a SIGSEGV from the emalloc caller and the execution terminate. I can't see a big difference in this case, but if you check for NULL you can fail gracefully. Ciao -- Walter Franzini, e-mail: [EMAIL PROTECTED] SysNet, Via Digione 8, 27100 Pavia - Italy -- PHP Development Mailing List http://www.php.net/ To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED] To contact the list administrators, e-mail: [EMAIL PROTECTED] -- Zeev Suraski [EMAIL PROTECTED] CTO co-founder, Zend Technologies Ltd. http://www.zend.com/ -- PHP Development Mailing List http://www.php.net/ To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED] To contact the list administrators, e-mail: [EMAIL PROTECTED]
Re: [PHP-DEV] [PATCH] Zend/zend_alloc.c
Zeev Suraski [EMAIL PROTECTED] writes:
There's a big difference between an undefined behavior which may
result in a crash, and an organized shutdown. The former is
dangerous, the latter is not.
[Sorry poor English follows]
I agree, but *who* is in charge to decide *what* an organized shutdown is?
When emalloc fails to allocate ZEND INTERNAL MEMORY the current
behaviour is correct, but what if the author of an EXTENSION has a
different idea?
I faced this problem using odbc with Solid. SQLColAttributes can
return very big number (2147483647) when you ask for
SQL_COLUMN_DISPLAY_SIZE of a LONG VARCHAR and obviously my computer
does not have enough virtual memory.
The patch below try to fix the problem, can some odbc guru confirm if
my solution is right?
The patch still contains the emalloc check, feel free to remove it :-)
There are some parts of code that don't rely on this, from the times
when it wasn't true, but it doesn't cause any problem, so there's no
reason to change it urgently.
Ok, I hope my current patch can live longer :-)
Zeev
Ciao
--
Walter Franzini, e-mail: [EMAIL PROTECTED]
SysNet, Via Digione 8, 27100 Pavia - Italy
diff -ur php-4.0.6.ORIG/ext/odbc/php_odbc.c php-4.0.6/ext/odbc/php_odbc.c
--- php-4.0.6.ORIG/ext/odbc/php_odbc.c Tue Jun 19 19:55:00 2001
+++ php-4.0.6/ext/odbc/php_odbc.c Thu Aug 2 09:13:00 2001
@@ -565,6 +565,7 @@
{
RETCODE rc;
int i;
+ long _size=0;
SWORD colnamelen; /* Not used */
SDWORD displaysize;
ODBCLS_FETCH();
@@ -610,11 +611,18 @@
break;
#endif /* HAVE_ADABAS */
default:
+_size = result-longreadlen;
rc = SQLColAttributes(result-stmt, (UWORD)(i+1),
SQL_COLUMN_DISPLAY_SIZE,
NULL, 0, NULL,
displaysize);
- result-values[i].value = (char *)emalloc(displaysize
+ 1);
- rc = SQLBindCol(result-stmt, (UWORD)(i+1),
SQL_C_CHAR, result-values[i].value,
- displaysize + 1,
result-values[i].vallen);
+_size = (_size = displaysize ? _size : displaysize);
+result-values[i].value = (char *)emalloc(_size + 1);
+if (result-values[i].value) {
+rc = SQLBindCol(result-stmt, (UWORD)(i+1), SQL_C_CHAR,
+result-values[i].value, _size + 1,
+result-values[i].vallen);
+} else {
+return 0;
+}
break;
}
}
Only in php-4.0.6/ext/odbc: php_odbc.c~
Only in php-4.0.6.ORIG/: mod_build
--
PHP Development Mailing List http://www.php.net/
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
To contact the list administrators, e-mail: [EMAIL PROTECTED]
Re: [PHP-DEV] [PATCH] Zend/zend_alloc.c
WF Changing emalloc to return NULLs does not modify the program WF behaviour: you (probably) receive a SIGSEGV from the emalloc WF caller and the execution terminate. I can't see a big Making such assumptions is a good way to security holes and obscure bugs. -- Stanislav Malyshev, Zend Products Engineer [EMAIL PROTECTED] http://www.zend.com/ +972-3-6139665 ext.115 -- PHP Development Mailing List http://www.php.net/ To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED] To contact the list administrators, e-mail: [EMAIL PROTECTED]
Re: [PHP-DEV] [PATCH] Zend/zend_alloc.c
Stanislav Malyshev [EMAIL PROTECTED] writes: [...] WF I faced this problem using odbc with Solid. SQLColAttributes can WF return very big number (2147483647) when you ask for WF SQL_COLUMN_DISPLAY_SIZE of a LONG VARCHAR and obviously my computer WF does not have enough virtual memory. It is not always a wise idea to use MAX_SIZE kind of constants to allocate memory. Some of them can indeed be huge. You should fix you code to not use such constants for memory allocation. If you look better at the patch, the current php implementation (php-4.0.6 and current cvs) uses the MAX_SIZE approach and I try to lower this number to a more reasonable value. Ciao -- Walter Franzini, e-mail: [EMAIL PROTECTED] SysNet, Via Digione 8, 27100 Pavia - Italy -- PHP Development Mailing List http://www.php.net/ To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED] To contact the list administrators, e-mail: [EMAIL PROTECTED]
Re: [PHP-DEV] [PATCH] Zend/zend_alloc.c
Zeev Suraski [EMAIL PROTECTED] writes: At 12:38 29-08-01, Walter Franzini wrote: Stanislav Malyshev [EMAIL PROTECTED] writes: [...] WF I faced this problem using odbc with Solid. SQLColAttributes can WF return very big number (2147483647) when you ask for WF SQL_COLUMN_DISPLAY_SIZE of a LONG VARCHAR and obviously my computer WF does not have enough virtual memory. It is not always a wise idea to use MAX_SIZE kind of constants to allocate memory. Some of them can indeed be huge. You should fix you code to not use such constants for memory allocation. If you look better at the patch, the current php implementation (php-4.0.6 and current cvs) uses the MAX_SIZE approach and I try to lower this number to a more reasonable value. The above sentence is referred to the odbc patch, not to the zend_alloc one. I'm not sure what you mean here. Several things: - A crash is not an organized shutdown, no matter how you look at it. Different operating systems treat it in different ways, and none of them is considered a good way of shutting down. On this point I totally agree :-) - By default, PHP builds without memory_limit enabled. A failure would only occur if we actually run out of memory, and then, simply put, we're screwed anyway, and the best thing we can do is try to shut down as nicely as possible. - If you enable memory_limit, when the program reaches it, the engine schedules a shutdown - a very organized one. The only situation in which an exit() would occur is if during the shutdown, the memory consumption continues to grow beyond 1MB of the memory limit. Zeev From the ZE (the kernel) point of view I can only agree with you. From the extension (the user of kernel services) pov I must disagree. But obviusly I'missing something :-) Ciao -- Walter Franzini, e-mail: [EMAIL PROTECTED] SysNet, Via Digione 8, 27100 Pavia - Italy -- PHP Development Mailing List http://www.php.net/ To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED] To contact the list administrators, e-mail: [EMAIL PROTECTED]
Re: [PHP-DEV] [PATCH] Zend/zend_alloc.c
At 14:57 29-08-01, Walter Franzini wrote: From the extension (the user of kernel services) pov I must disagree. But obviusly I'missing something :-) I don't see why there's a difference. There may be a point in giving extension developers an _ex way to try and allocate memory, and fail gracefully (return NULL) in case of failure. We already have this support in realloc(), we can add it to emalloc() if it's necessary. Do you have a case where you want to try and allocate a chunk of memory, and do something special in case it fails (e.g., allocate a smaller block)? If so, we can try to come up with a solution. My main point is that there's a very strong motivation to take away the responsibility of what happens in case of a memory failure away from the API user, and take care of it in a centralized way in the memory manager. It's safer (no possibility of memory allocation failures causing crashes) and it avoids redundant coding. If this behavior is unsuitable in certain cases, we should address these cases, rather than cut this safety net to pieces. Zeev -- PHP Development Mailing List http://www.php.net/ To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED] To contact the list administrators, e-mail: [EMAIL PROTECTED]
Re: [PHP-DEV] [PATCH] Zend/zend_alloc.c
Zeev Suraski [EMAIL PROTECTED] writes: At 14:57 29-08-01, Walter Franzini wrote: From the extension (the user of kernel services) pov I must disagree. But obviusly I'missing something :-) I don't see why there's a difference. Maybe the difference is not ZE vs. extension but internal vs. external data, where external means coming from the outside of PHP/Zend. If you try to allocate memory for data that come from the outside (the browser, a db) you should fail gracefully: a malicious user can send to your app a huge amount of data only to make it crash. From the user point of view an application that exit without a message is crashed, no matter how controlled the shutdown is. Is this a reasonable scenario? [...] Do you have a case where you want to try and allocate a chunk of memory, and do something special in case it fails (e.g., allocate a smaller block)? If so, we can try to come up with a solution. No, at the moment I use PHP to write webApp and I really hate the document contains non data message :-) My main point is that there's a very strong motivation to take away the responsibility of what happens in case of a memory failure away from the API user, and take care of it in a centralized way in the memory manager. It's safer (no possibility of memory allocation failures causing crashes) and it avoids redundant coding. If this behavior is unsuitable in certain cases, we should address these cases, rather than cut this safety net to pieces. I really understand, and appreciate, your position, but I think that: Ehi, this stupid program want 2GB of memory and I only have 600MB available. It's time to exit!! is not the best reaction for the ZE. But Ehi, I can't allocate the memory needed to store a refcount. It's time to exit!! seems reasonable to me. Zeev Thank for your patience. Ciao -- Walter Franzini, e-mail: [EMAIL PROTECTED] SysNet, Via Digione 8, 27100 Pavia - Italy -- PHP Development Mailing List http://www.php.net/ To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED] To contact the list administrators, e-mail: [EMAIL PROTECTED]
Re: [PHP-DEV] [PATCH] Zend/zend_alloc.c
At 18:35 29-08-01, Walter Franzini wrote: Zeev Suraski [EMAIL PROTECTED] writes: At 14:57 29-08-01, Walter Franzini wrote: From the extension (the user of kernel services) pov I must disagree. But obviusly I'missing something :-) I don't see why there's a difference. Maybe the difference is not ZE vs. extension but internal vs. external data, where external means coming from the outside of PHP/Zend. If you try to allocate memory for data that come from the outside (the browser, a db) you should fail gracefully: a malicious user can send to your app a huge amount of data only to make it crash. But it does not crash. It exits, which is the safest thing you can do. From the user point of view an application that exit without a message is crashed, no matter how controlled the shutdown is. Is this a reasonable scenario? Given the fact that a failed malloc() can often mean that the situation is *really* bad, the best thing to do would be exiting. Doing anything else, such as printing things out, etc., is dangerous, as it may result in a crash. We can argue as to what to do in case we run out of memory - but what you were suggesting is not doing anything, and move the responsibility for the users of the memory manager. No matter how I look at it, that's a bad thing. But Ehi, I can't allocate the memory needed to store a refcount. It's time to exit!! seems reasonable to me. Due to the reasons I gave in my previous letter, I disagree. Zeev -- PHP Development Mailing List http://www.php.net/ To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED] To contact the list administrators, e-mail: [EMAIL PROTECTED]
Re: [PHP-DEV] [PATCH] Zend/zend_alloc.c
At 18:45 29-08-01, George Schlossnagle wrote: At 14:57 29-08-01, Walter Franzini wrote: From the extension (the user of kernel services) pov I must disagree. But obviusly I'missing something :-) I don't see why there's a difference. Maybe the difference is not ZE vs. extension but internal vs. external data, where external means coming from the outside of PHP/Zend. If you try to allocate memory for data that come from the outside (the browser, a db) you should fail gracefully: a malicious user can send to your app a huge amount of data only to make it crash. This sort of checking should be done by the extension before you make the allocation, no? You shoulnd't even be -trying- to allocate 2G of memory for an object in your extension (unless you really want to be, of course). Amongst other things allowing for this sloppiness only encourages people to further sloppiness, like not checking the return values of their malloc/emalloc call, which will almost certainly result in crashes when they return null and that pointer is sloppily passed to another routine without proper checking. Failure to allocate should be a fatal error. Sometimes it makes sense (but these cases are very rare). For instance, some hashes that over-populate try to increase their lookup table size, so that they stay efficient - but if you try to allocate a bigger block and fail, it's quite alright to stay with the existing table size. That's the only reason we added this ability to erealloc(), so they're really rare cases. Zeev -- PHP Development Mailing List http://www.php.net/ To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED] To contact the list administrators, e-mail: [EMAIL PROTECTED]
[PHP-DEV] [PATCH] Zend/zend_alloc.c
Hi,
is ok to send to this list patch for Zend?
I think _emalloc should not exit if unable to allocate enough memory,
malloc behave differently :-)
the patch below fix also a small problem with the format string.
--- php-4.0.6.ORIG/Zend/zend_alloc.cTue Jun 19 20:04:53 2001
+++ php-4.0.6/Zend/zend_alloc.c Tue Jul 31 10:32:39 2001
@@ -158,12 +158,11 @@
HANDLE_BLOCK_INTERRUPTIONS();
if (!p) {
- fprintf(stderr,FATAL: emalloc(): Unable to allocate %ld bytes\n,
(long) size);
+ fprintf(stderr,FATAL: emalloc(): Unable to allocate %lu bytes\n,
+(long unsigned) size);
#if ZEND_DEBUG defined(HAVE_KILL) defined(HAVE_GETPID)
kill(getpid(), SIGSEGV);
-#else
- exit(1);
#endif
+
HANDLE_UNBLOCK_INTERRUPTIONS();
return (void *)p;
}
Ciao
--
Walter Franzini, e-mail: [EMAIL PROTECTED]
SysNet, Via Digione 8, 27100 Pavia - Italy
--
PHP Development Mailing List http://www.php.net/
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
To contact the list administrators, e-mail: [EMAIL PROTECTED]
Re: [PHP-DEV] [PATCH] Zend/zend_alloc.c
I disagree with this patch. The scenario of not being able to allocate memory is a
fatal error, and the only appropriate response for php is to exit. If you need other
behavior use pemalloc( which calls malloc if set persistant ).
-Jason
- Original Message -
From: Walter Franzini [EMAIL PROTECTED]
To: [EMAIL PROTECTED]
Sent: Tuesday, August 28, 2001 8:50 AM
Subject: [PHP-DEV] [PATCH] Zend/zend_alloc.c
Hi,
is ok to send to this list patch for Zend?
I think _emalloc should not exit if unable to allocate enough memory,
malloc behave differently :-)
the patch below fix also a small problem with the format string.
--- php-4.0.6.ORIG/Zend/zend_alloc.c Tue Jun 19 20:04:53 2001
+++ php-4.0.6/Zend/zend_alloc.c Tue Jul 31 10:32:39 2001
@@ -158,12 +158,11 @@
HANDLE_BLOCK_INTERRUPTIONS();
if (!p) {
- fprintf(stderr,FATAL: emalloc(): Unable to allocate %ld bytes\n, (long) size);
+ fprintf(stderr,FATAL: emalloc(): Unable to allocate %lu bytes\n, (long
unsigned) size);
#if ZEND_DEBUG defined(HAVE_KILL) defined(HAVE_GETPID)
kill(getpid(), SIGSEGV);
-#else
- exit(1);
#endif
+
HANDLE_UNBLOCK_INTERRUPTIONS();
return (void *)p;
}
Ciao
--
Walter Franzini, e-mail: [EMAIL PROTECTED]
SysNet, Via Digione 8, 27100 Pavia - Italy
--
PHP Development Mailing List http://www.php.net/
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
To contact the list administrators, e-mail: [EMAIL PROTECTED]
--
PHP Development Mailing List http://www.php.net/
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
To contact the list administrators, e-mail: [EMAIL PROTECTED]
Re: [PHP-DEV] [PATCH] Zend/zend_alloc.c
JG I disagree with this patch. The scenario of not being able to JG allocate memory is a fatal error, and the only appropriate JG response for php is to exit. If you need other behavior use JG pemalloc( which calls malloc if set persistant ). Actually, there's a lot of code in PHP that is based on the assumption that emalloc never fails. If emalloc can return NULLs, all this code should be rewritten. I do not see any point in it and agree with Jason. -- Stanislav Malyshev, Zend Products Engineer [EMAIL PROTECTED] http://www.zend.com/ +972-3-6139665 ext.115 -- PHP Development Mailing List http://www.php.net/ To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED] To contact the list administrators, e-mail: [EMAIL PROTECTED]
[PHP-DEV] [PATCH] Zend/zend_alloc.c
--- php-4.0.6.ORIG/Zend/zend_alloc.c Tue Jun 19 20:04:53 2001
+++ php-4.0.6/Zend/zend_alloc.c Tue Jul 31 10:32:39 2001
@@ -158,12 +158,11 @@
HANDLE_BLOCK_INTERRUPTIONS();
if (!p) {
- fprintf(stderr,FATAL: emalloc(): Unable to allocate %ld bytes\n, (long) size);
+ fprintf(stderr,FATAL: emalloc(): Unable to allocate %lu bytes\n, (long unsigned) size);
#if ZEND_DEBUG defined(HAVE_KILL) defined(HAVE_GETPID)
kill(getpid(), SIGSEGV);
-#else
- exit(1);
#endif
+
HANDLE_UNBLOCK_INTERRUPTIONS();
return (void *)p;
}
--
PHP Development Mailing List http://www.php.net/
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
To contact the list administrators, e-mail: [EMAIL PROTECTED]
