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 <[email protected]> > > > --- > > > 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
