Re: nm(1): return on malloc error
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 - 1.53 > --- nm.c 20 Feb 2019 17:34:01 - > *** 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; >
Re: nm(1): return on malloc error
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 - 1.53 > +++ nm.c 3 Mar 2019 15:12:28 - > @@ -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); >
Re: nm(1): return on malloc error
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.c27 Oct 2017 16:47:08 - 1.53 +++ nm.c3 Mar 2019 15:12:28 - @@ -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);
Re: nm(1): return on malloc error
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 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.c27 Oct 2017 16:47:08 - 1.53 --- nm.c20 Feb 2019 17:34:01 - *** 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;
nm(1): return on malloc error
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.c27 Oct 2017 16:47:08 - 1.53 --- nm.c20 Feb 2019 17:34:01 - *** 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;