Project

General

Profile

Bug #4308

svc_userfds reallocation in svc_add_input() is fragile

Added by Marcel Telka over 6 years ago. Updated over 6 years ago.

Status:
Resolved
Priority:
Normal
Assignee:
Category:
lib - userland libraries
Start date:
2013-11-08
Due date:
% Done:

100%

Estimated time:
Difficulty:
Medium
Tags:
Gerrit CR:

Description

Here is how the svc_userfds realloc is implemented in svc_add_input():

446        svc_userfds = (_svc_user_fd_head *)
447            realloc(svc_userfds,
448            svc_nuserfds * sizeof (_svc_user_fd_head));
449
450        if (svc_userfds == NULL) {
451            syslog(LOG_ERR, "svc_add_input: out of memory");
452            errno = ENOMEM;
453            (void) mutex_unlock(&svc_userfds_lock);
454            return ((svc_input_id_t)-1);
455        }

The problem with this code is that after the reallocation failure we will lost the previously allocated svc_userfds. This is both memory leak, and it also could lead to null pointer dereference later (because svc_nuserfds does not reflect what we have now in svc_userfds) - see __is_a_userfd() for an example of the possible null pointer dereference.


Related issues

Related to illumos gate - Bug #4575: Single threaded rpcbind is not scalableResolved2014-02-05

Actions

History

#1

Updated by Marcel Telka over 6 years ago

  • Assignee set to Marcel Telka

The similar issue is in _svc_attribute_new_id() in the same source file - usr/src/lib/libnsl/rpc/svc.c.

#2

Updated by Marcel Telka over 6 years ago

In addition to the issue in Description, there is another possible memory corruption in svc_add_input():

Every entry in svc_userfds contains a linked list of _svc_user_fd_node nodes. If the list is empty, it is initialized using the LIST_CLR() macro. This macro makes the first entry to point to itself. Once the svc_userfds is reallocated in svc_add_input() the pointers will start to point to already freed memory. Here is an example:

> svc_nuserfds::print
0x15
> svc_userfds::print|::array _svc_user_fd_head 0x15|::print -a _svc_user_fd_head
80da150 {
    80da150 lnk = {
        80da150 next = 0x8070980
        80da154 previous = 0x8070980
    }
    80da158 mask = 0
}
80da15c {
    80da15c lnk = {
        80da15c next = 0x807098c
        80da160 previous = 0x807098c
    }
    80da164 mask = 0
}

...

80da234 {
    80da234 lnk = {
        80da234 next = 0x80da234
        80da238 previous = 0x80da234
    }
    80da23c mask = 0
}
80da240 {
    80da240 lnk = {
        80da240 next = 0x80da240
        80da244 previous = 0x80da240
    }
    80da248 mask = 0
}
>

As we can see, the beginning of the svc_userfds array points to now-random (and likely freed) memory (these entries were initialized before the last realloc() call), while the end of the svc_userfds array points correctly back to the entry addresses (these entries were initialized after the last realloc() call).

#3

Updated by Marcel Telka over 6 years ago

  • Status changed from New to In Progress
#4

Updated by Marcel Telka over 6 years ago

  • Status changed from In Progress to Pending RTI
#5

Updated by Electric Monk over 6 years ago

git commit 29d91154fb99ee8e353a5f89cd59be53f2ed8e64

Author: Marcel Telka <marcel.telka@nexenta.com>

4308 svc_userfds reallocation in svc_add_input() is fragile
Reviewed by: Ilya Usvyatsky <ilya.usvyatsky@nexenta.com>
Reviewed by: Jan Kryl <jan.kryl@nexenta.com>
Approved by: Richard Lowe <richlowe@richlowe.net>

#6

Updated by Rich Lowe over 6 years ago

  • Status changed from Pending RTI to Resolved
  • % Done changed from 0 to 100
  • Tags deleted (needs-triage)

Also available in: Atom PDF