Project

General

Profile

Actions

Bug #5679

closed

be_sort_list(): Possible null pointer dereference

Added by Marcel Telka about 7 years ago. Updated almost 7 years ago.

Status:
Closed
Priority:
Low
Assignee:
Category:
lib - userland libraries
Start date:
2015-03-02
Due date:
% Done:

100%

Estimated time:
Difficulty:
Medium
Tags:
needs-triage
Gerrit CR:

Description

In a case the realloc() fails, we will crash at line 649 in be_sort_list():

646    for (p = *pstart, nbe = 0; p != NULL; nbe++, p = p->be_next_node) {
647        ptrlist = realloc(ptrlist,
648            sizeof (be_node_list_t *) * (nbe + 2));
649        ptrlist[nbe] = p;
650    }

Related issues

Is duplicate of illumos gate - Bug #5970: Many source files don't handle realloc() failureNew2015-05-29

Actions
Actions #1

Updated by Gary Mills almost 7 years ago

Is there any reason to fix this bug? The realloc() call on that line is unlikely to fail because it extends the space by only one pointer. If it did fail, the subsequent malloc() would also fail. I can devise a change to prevent a core dump if realloc() failed, but is there any reason to do so?

Actions #2

Updated by Marcel Telka almost 7 years ago

Gary Mills wrote:

Is there any reason to fix this bug? The realloc() call on that line is unlikely to fail because it extends the space by only one pointer. If it did fail, the subsequent malloc() would also fail. I can devise a change to prevent a core dump if realloc() failed, but is there any reason to do so?

I see the main reason to fix this in a possibility to kill the bad habits with the realloc() usage. I believe we have a lot of similar pieces of the code through the gate, so it would be great to have some kind of project "review/fix all the realloc() callers" so the new code writers are not inspired by them.

Actions #3

Updated by Gary Mills almost 7 years ago

As a start, I've reviewed all the illumos source, looking for instances where a realloc() failure is not handled by the code. I found 45 such files. These are listed in bug #5970 . One of them is the file in question here.

Actions #4

Updated by Marcel Telka almost 7 years ago

  • Is duplicate of Bug #5970: Many source files don't handle realloc() failure added
Actions #5

Updated by Marcel Telka almost 7 years ago

  • Status changed from New to Closed
Actions #6

Updated by Gary Mills almost 7 years ago

I can confirm that beadm does dump core in be_sort_list() whenever realloc() returns NULL. I tested this in Solaris 11.2, oi_151a9, and a recent build of illumos-gate. All of them dumped core.

Actions #7

Updated by Marcel Telka almost 7 years ago

  • Status changed from Closed to In Progress
  • Assignee set to Gary Mills
Actions #8

Updated by Electric Monk almost 7 years ago

  • Status changed from In Progress to Closed
  • % Done changed from 0 to 100

git commit 0afb687bf3724077cd5f304f176b5ee2b92aa2c2

commit  0afb687bf3724077cd5f304f176b5ee2b92aa2c2
Author: Gary Mills <gary_mills@fastmail.fm>
Date:   2015-06-12T03:15:33.000Z

    5679 be_sort_list(): Possible null pointer dereference
    Reviewed by: Garrett D'Amore <garrett@damore.org>
    Reviewed by: Marcel Telka <marcel@telka.sk>
    Reviewed by: Toomas Soome <tsoome@me.com>
    Approved by: Gordon Ross <gwr@nexenta.com>

Actions

Also available in: Atom PDF