Another malloc without NULL check

[Claudia Moroder]

Hello Tim,

it is effectively
if (!cli->outbuf || !cli->inbuf)
{
return NULL;

but IMHO your cleanup continues to leave around memory.
it should be, because maybe that cli was also malloced a few lines before
and gets lost if we return NULL.
But... the problem is that cli is passed and is malloced only if NULL is
passed.
If we want to free wat we created we must remember if we created it so this
function should be.

struct cli_state *cli_initialise(struct cli_state *cli)
{
BOOL created;

created=FALSE;
 if (!cli) {
  cli = (struct cli_state *)malloc(sizeof(*cli));
  if (!cli)
   return NULL;
  ZERO_STRUCTP(cli);
  created=TRUE;
 }

 if (cli->initialised) {
  cli_shutdown(cli);
 }

 ZERO_STRUCTP(cli);

 cli->port = 0;
 cli->fd = -1;
 cli->cnum = -1;
 cli->pid = (uint16)sys_getpid();
 cli->mid = 1;
 cli->vuid = UID_FIELD_INVALID;
 cli->protocol = PROTOCOL_NT1;
 cli->timeout = 20000; /* Timeout is in milliseconds. */
 cli->bufsize = CLI_BUFFER_SIZE+4;
 cli->max_xmit = cli->bufsize;

 cli->outbuf = (char *)malloc(cli->bufsize);
if (cli->outbuf ==NULL) {
    if(created==TRUE)
       free(cli);
    }
    return NULL;
}
cli->inbuf = (char *)malloc(cli->bufsize);
if (cli->inbuf  ==NULL) {
    free(cli->outbuf);
    if(created==TRUE)
       free(cli);
    }
    return NULL;
}

 if ((cli->mem_ctx = talloc_init()) == NULL) {
    if(created==TRUE)
       free(cli);
    }
  free(cli->outbuf);
  free(cli->inbuf);
  return NULL;
 }

 memset(cli->outbuf, '\0', cli->bufsize);
 memset(cli->inbuf, '\0', cli->bufsize);

 cli->initialised = 1;

 return cli;
}

Bye

Andreas

P.S. If there is a little confusion if I am Andreas or Claudia, I am
Andreas. Claudia is my lovely wife that own this e-mail account. I have my
e-mail account at my workplace.



-----Ursprüngliche Nachricht-----
Von: "Tim Potter" <tpot at valinux.com>
An: "Claudia Moroder" <claudiamoroder at st-ulrich.suedtirol.net>
Cc: <samba-technical at lists.samba.org>
Gesendet: Montag, 6. August 2001 04:20
Betreff: Re: Another malloc without NULL check


> Claudia Moroder writes:
>
> > Hello,
> >
> > i found two problems with malloc/free
> >
> > 1. cli_spoolss.c
> >
> > decode_printer_info_0
> >
> > inf = malloc(...)
> >
> > and inf is used without a NULL check.
> > It is also strange that all other allocations in this files are made
through
> > talloc.
>
> Fixed.  Thanks for spotting that!
>
> > 2. in clientgen.c
> >
> > I don't remember the exact position, because now I am in windows , but
..
> >
> > cli is allocatd with malloc
> > few lines later the buffers are allocated with malloc and are tested.
> > If one of the buffer allocations fails the function return NULL without
> > freeing the first buffer ( if the second failed ) and without releasing
cli.
>
> It wouldn't be this would it?
>
> if (!cli->outbuf || !cli->inbuf)
> {
> return NULL;
> }
>
> The correct code should probably be
>
> if (!cli->outbuf)
>                 return NULL;
>
>         if (!cli->inbuf) {
>                 free(cli->inbuf);
> return NULL;
> }
>
>
> Tim.