On 03/10/14 at 07:32pm, Sören Brinkmann wrote:
Hi Andrew,
On Mon, 2014-03-10 at 10:00AM -0400, Andrew Gregory wrote:
On 03/09/14 at 03:47pm, Sören Brinkmann wrote:
Check the return value of malloc() before dereferencing the returned pointer.
Signed-off-by: Sören Brinkmann <soeren.brinkmann@gmail.com> --- v2: - add print statement to log the error --- src/pacman/upgrade.c | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/src/pacman/upgrade.c b/src/pacman/upgrade.c index 5416f6180b39..11da4dcb5e53 100644 --- a/src/pacman/upgrade.c +++ b/src/pacman/upgrade.c @@ -51,6 +51,10 @@ int pacman_upgrade(alpm_list_t *targets) */ for(i = targets; i; i = alpm_list_next(i)) { int *r = malloc(sizeof(int)); + if(r == NULL) { + pm_printf(ALPM_LOG_ERROR, _("memory exhausted\n")); + return 1; + }
If you move the malloc outside the loop and make it an array we have a better chance of hitting this early enough that we still have a little memory left for cleanup.
I think that is not done that easily. Actually not without some significant rewrite of this code. Those allocated rs go into an alpm_list_t which is eventually freed using FREELIST. Doing the memory management differently here is not that straight forward.
Sören
The resulting list is only used in one place and isn't passed to any other functions. It would be trivial to convert it to a standard C array. Leaving it as-is with a list instead of an array leaves this function still vulnerable to a malloc failure from alpm_list_add. The only thing the list is used for is to set the siglevel, so you could even store the siglevels directly in the array instead of integer flags. apg