nm(1): return on malloc error

classic Classic list List threaded Threaded
5 messages Options
Reply | Threaded
Open this post in threaded view
|

nm(1): return on malloc error

Benjamin Baier
Hi.

When malloc fails we should return like the MMAP case does.

Greetings Ben

Index: nm.c
===================================================================
RCS file: /cvs/src/usr.bin/nm/nm.c,v
retrieving revision 1.53
diff -u -p -u -C10 -r1.53 nm.c
*** nm.c 27 Oct 2017 16:47:08 -0000 1.53
--- nm.c 20 Feb 2019 17:34:01 -0000
*************** show_symtab(off_t off, u_long len, const
*** 374,393 ****
--- 374,394 ----
  restore = ftello(fp);

  MMAP(symtab, len, PROT_READ, MAP_PRIVATE|MAP_FILE, fileno(fp), off);
  if (symtab == MAP_FAILED)
  return (1);

  namelen = sizeof(ar_head.ar_name);
  if ((p = malloc(sizeof(ar_head.ar_name))) == NULL) {
  warn("%s: malloc", name);
  MUNMAP(symtab, len);
+ return (1);
  }

  printf("\nArchive index:\n");
  num = betoh32(*symtab);
  strtab = (char *)(symtab + num + 1);
  for (ps = symtab + 1; num--; ps++, strtab += strlen(strtab) + 1) {
  if (fseeko(fp, betoh32(*ps), SEEK_SET)) {
  warn("%s: fseeko", name);
  rval = 1;
  break;

Reply | Threaded
Open this post in threaded view
|

Re: nm(1): return on malloc error

Benjamin Baier
Ping.

On malloc error symtab is unmapped, so proceeding on will lead to a NULL pointer
dereference.

On Wed, 20 Feb 2019 17:55:08 +0100
Benjamin Baier <[hidden email]> wrote:

> Hi.
>
> When malloc fails we should return like the MMAP case does.
>
> Greetings Ben
>
Index: nm.c
===================================================================
RCS file: /cvs/src/usr.bin/nm/nm.c,v
retrieving revision 1.53
diff -u -p -u -C10 -r1.53 nm.c
*** nm.c 27 Oct 2017 16:47:08 -0000 1.53
--- nm.c 20 Feb 2019 17:34:01 -0000
*************** show_symtab(off_t off, u_long len, const
*** 374,393 ****
--- 374,394 ----
  restore = ftello(fp);

  MMAP(symtab, len, PROT_READ, MAP_PRIVATE|MAP_FILE, fileno(fp), off);
  if (symtab == MAP_FAILED)
  return (1);

  namelen = sizeof(ar_head.ar_name);
  if ((p = malloc(sizeof(ar_head.ar_name))) == NULL) {
  warn("%s: malloc", name);
  MUNMAP(symtab, len);
+ return (1);
  }

  printf("\nArchive index:\n");
  num = betoh32(*symtab);
  strtab = (char *)(symtab + num + 1);
  for (ps = symtab + 1; num--; ps++, strtab += strlen(strtab) + 1) {
  if (fseeko(fp, betoh32(*ps), SEEK_SET)) {
  warn("%s: fseeko", name);
  rval = 1;
  break;

Reply | Threaded
Open this post in threaded view
|

Re: nm(1): return on malloc error

Ingo Schwarze
Hi,

Benjamin Baier wrote on Sat, Mar 02, 2019 at 10:10:40AM +0100:

> On malloc error symtab is unmapped, so proceeding on will lead
> to a NULL pointer dereference.
> When malloc fails we should return like the MMAP case does.

while i'm certainly not experienced with nm(1), this change looks
correct to me.  OK to commit?

Note that returning implies that the program might attempt to process
further files, which is of dubious value: it is likely to fail, too,
and in the unlikely case of success, that's maybe even worse: the
output from subsequent files might cause the user to miss the error
message about the malloc failure...  But given that mmap(2) failure
already behaves like that, switching to just err(3) out on resource
exhaustion looks like a larger change which i'm not planning to
push for, even though it would make sense to me.

Here is the patch again, in standard format.

Yours
  Ingo


Index: nm.c
===================================================================
RCS file: /cvs/src/usr.bin/nm/nm.c,v
retrieving revision 1.53
diff -p -U8 -r1.53 nm.c
--- nm.c 27 Oct 2017 16:47:08 -0000 1.53
+++ nm.c 3 Mar 2019 15:12:28 -0000
@@ -376,16 +376,17 @@ show_symtab(off_t off, u_long len, const
  MMAP(symtab, len, PROT_READ, MAP_PRIVATE|MAP_FILE, fileno(fp), off);
  if (symtab == MAP_FAILED)
  return (1);
 
  namelen = sizeof(ar_head.ar_name);
  if ((p = malloc(sizeof(ar_head.ar_name))) == NULL) {
  warn("%s: malloc", name);
  MUNMAP(symtab, len);
+ return (1);
  }
 
  printf("\nArchive index:\n");
  num = betoh32(*symtab);
  strtab = (char *)(symtab + num + 1);
  for (ps = symtab + 1; num--; ps++, strtab += strlen(strtab) + 1) {
  if (fseeko(fp, betoh32(*ps), SEEK_SET)) {
  warn("%s: fseeko", name);

Reply | Threaded
Open this post in threaded view
|

Re: nm(1): return on malloc error

Otto Moerbeek
On Sun, Mar 03, 2019 at 04:23:53PM +0100, Ingo Schwarze wrote:

> Hi,
>
> Benjamin Baier wrote on Sat, Mar 02, 2019 at 10:10:40AM +0100:
>
> > On malloc error symtab is unmapped, so proceeding on will lead
> > to a NULL pointer dereference.
> > When malloc fails we should return like the MMAP case does.
>
> while i'm certainly not experienced with nm(1), this change looks
> correct to me.  OK to commit?
>
> Note that returning implies that the program might attempt to process
> further files, which is of dubious value: it is likely to fail, too,
> and in the unlikely case of success, that's maybe even worse: the
> output from subsequent files might cause the user to miss the error
> message about the malloc failure...  But given that mmap(2) failure
> already behaves like that, switching to just err(3) out on resource
> exhaustion looks like a larger change which i'm not planning to
> push for, even though it would make sense to me.
>
> Here is the patch again, in standard format.

ok,

        -Otto

>
> Yours
>   Ingo
>
>
> Index: nm.c
> ===================================================================
> RCS file: /cvs/src/usr.bin/nm/nm.c,v
> retrieving revision 1.53
> diff -p -U8 -r1.53 nm.c
> --- nm.c 27 Oct 2017 16:47:08 -0000 1.53
> +++ nm.c 3 Mar 2019 15:12:28 -0000
> @@ -376,16 +376,17 @@ show_symtab(off_t off, u_long len, const
>   MMAP(symtab, len, PROT_READ, MAP_PRIVATE|MAP_FILE, fileno(fp), off);
>   if (symtab == MAP_FAILED)
>   return (1);
>  
>   namelen = sizeof(ar_head.ar_name);
>   if ((p = malloc(sizeof(ar_head.ar_name))) == NULL) {
>   warn("%s: malloc", name);
>   MUNMAP(symtab, len);
> + return (1);
>   }
>  
>   printf("\nArchive index:\n");
>   num = betoh32(*symtab);
>   strtab = (char *)(symtab + num + 1);
>   for (ps = symtab + 1; num--; ps++, strtab += strlen(strtab) + 1) {
>   if (fseeko(fp, betoh32(*ps), SEEK_SET)) {
>   warn("%s: fseeko", name);
>

Reply | Threaded
Open this post in threaded view
|

Re: nm(1): return on malloc error

Ingo Schwarze
In reply to this post by Benjamin Baier
Hi,

Benjamin Baier wrote on Sat, Mar 02, 2019 at 10:10:40AM +0100:

> On malloc error symtab is unmapped, so proceeding on will lead
> to a NULL pointer dereference.
> When malloc fails we should return like the MMAP case does.

Committed.

Thanks for the patch (and to those who checked it).
  Ingo


> Index: nm.c
> ===================================================================
> RCS file: /cvs/src/usr.bin/nm/nm.c,v
> retrieving revision 1.53
> diff -u -p -u -C10 -r1.53 nm.c
> *** nm.c 27 Oct 2017 16:47:08 -0000 1.53
> --- nm.c 20 Feb 2019 17:34:01 -0000
> *************** show_symtab(off_t off, u_long len, const
> *** 374,393 ****
> --- 374,394 ----
>   restore = ftello(fp);
>
>   MMAP(symtab, len, PROT_READ, MAP_PRIVATE|MAP_FILE, fileno(fp), off);
>   if (symtab == MAP_FAILED)
>   return (1);
>
>   namelen = sizeof(ar_head.ar_name);
>   if ((p = malloc(sizeof(ar_head.ar_name))) == NULL) {
>   warn("%s: malloc", name);
>   MUNMAP(symtab, len);
> + return (1);
>   }
>
>   printf("\nArchive index:\n");
>   num = betoh32(*symtab);
>   strtab = (char *)(symtab + num + 1);
>   for (ps = symtab + 1; num--; ps++, strtab += strlen(strtab) + 1) {
>   if (fseeko(fp, betoh32(*ps), SEEK_SET)) {
>   warn("%s: fseeko", name);
>   rval = 1;
>   break;
>