|
|
|
|
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;
|
|
|
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;
|
|
|
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);
|
|
|
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);
>
|
|
|
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;
>
|
|
Locked
