Bug #5533

LOADPRIVDATA macro could return NULL on memory stress

Added by Serghei Samsi almost 6 years ago. Updated over 4 years ago.

In Progress
lib - userland libraries
Start date:
Due date:
% Done:


Estimated time:
Gerrit CR:


During memory stress condition,
cron service is dumping core multiple times.

mdb ./cron.webmail_mtc.20110.1413125400.core 
Loading modules: [ ]
> $C
fb5ac7d8`ucred_size+0x1b(8d0bad0, f6173000, fb5ac7f8, f6088733, 8d0bad0, f5fd2000)
fb5ac7e8`_ucred_alloc+0x19(8d0bad0, f5fd2000, fb5ac828, f5fae7ba, ffffffff, f5f90018)
fb5ac7f8`ucred_get+0x19(ffffffff, f5f90018, f60d8df5, f5fae79f)
fb5ac828`adt_init+0x8e(8d0bad0, 1, 8ce83b8, f5facfd5)
fb5ac868`adt_start_session+0x62(fb5ac8b8, 0, 1, f5d21a18)
fb5b2178`pam_sm_setcred+0x211(8d0a828, 1, 0, 0)
fb5b21e8`run_stack+0x5e5(8d0a828, 0, 1, 1, 10, 2)
fb5b2218`pam_setcred+0x31(8d0a828, 1, 806b430, 806e04c)
fb5b2238 set_user_cred+0xa1(8ce6098, 0, fb5b4338, 8056fda)
fb5b4338 ex+0x3d6(8cea720, fb5b4380, fb5b4428, 8053ab7)
fb5b4428 main+0x3a6(1, fb5b4460, fb5b4468, fb5b441c)
fb5b4454 _start+0x7d(1, fb5b4524, 0, fb5b4533, fb5b454a, fb5b4551)

> ucred_size+0x1b::dis`ucred_size:    pushl  %ebp`ucred_size+1:  movl   %esp,%ebp`ucred_size+3:  pushl  %ebx`ucred_size+4:  subl   $0x4,%esp`ucred_size+7:  andl   $0xfffffff0,%esp`ucred_size+0xa:call   +0x0     <`ucred_size+0xf>`ucred_size+0xf:popl   %ebx`ucred_size+0x10:       addl   $0xea386,%ebx`ucred_size+0x16:       call   -0xa2c8  <`__priv_getdata>`ucred_size+0x1b:       movl   0xc(%eax),%eax`ucred_size+0x1e:       leal   -0x4(%ebp),%esp`ucred_size+0x21:       popl   %ebx`ucred_size+0x22:       leave`ucred_size+0x23:       ret    
0xf6088c8f:                     nop    
0xf6088c90:                     nop    
0xf6088c91:                     nop    
0xf6088c92:                     nop    
0xf6088c93:                     nop    
0xf6088c94:                     nop    
> $r
%cs = 0x0043            %eax = 0x00000000 
%ds = 0x004b            %ebx = 0xf6173000 
%ss = 0x004b            %ecx = 0xf6174ce0`pd_lock
%es = 0x004b            %edx = 0x00000000 
%fs = 0x0000            %esi = 0x08d0bad0 
%gs = 0x01c3            %edi = 0x00000001 

 %eip = 0xf6088c86`ucred_size+0x1b
 %ebp = 0xfb5ac7d8
%kesp = 0x00000000

%eflags = 0x00010286
  id=0 vip=0 vif=0 ac=0 vm=0 rf=1 nt=0 iopl=0x0

   %esp = 0xfb5ac7d0
%trapno = 0xe
   %err = 0x4

Problem piece of code is here (file /illumos-gate/usr/src/lib/libc/port/gen/ucred.c):

372    priv_data_t *d;
376    return (d->pd_ucredsize);

We should add NULL test for priv_data_t pointer after line 376 like that:

376 return ((d != NULL) ? d->pd_ucredsize : 0);

Then ucred_alloc() will fail as expected (due to malloc(0)).

Another place in the same file:

213const priv_set_t *
214ucred_getprivset(const ucred_t *uc, priv_ptype_t set)
216    /* LINTED: alignment */
217    const prpriv_t *pr = UCPRIV(uc);
218    int pset = priv_getsetbyname(set);
219    priv_data_t *d;
221    if (pr == NULL || pset == -1) {
222        errno = EINVAL;
223        return (NULL);
224    }
228    return ((const priv_set_t *)
229        &pr->pr_sets[d->pd_pinfo->priv_setsize * pset]);

It seems that all places with LOADPRIVDATA macro should be fixed too.



Updated by Serghei Samsi over 5 years ago

  • Assignee changed from Serghei Samsi to Marcel Telka

Updated by Marcel Telka over 5 years ago

  • Category set to lib - userland libraries
  • Status changed from New to In Progress

Updated by Marcel Telka over 5 years ago

The similar issue is with GETPRIVDATA macro too.


Updated by Marcel Telka over 5 years ago

Usually, the LOADPRIVDATA/GETPRIVDATA macros are used directly or indirectly in various API functions. There are two sorts of LOADPRIVDATA/GETPRIVDATA users:

I) those who could fail, because they have defined the failure modes of operation, for example getppriv(2)/setppriv(2)

To fix these, we will just fail in a case either the LOADPRIVDATA or GETPRIVDATA fails.

II) those who cannot fail, for example priv_union(3c)

In this case the LOADPRIVDATA/GETPRIVDATA calls cannot fail. The LOADPRIVDATA/GETPRIVDATA implementation is simple: in a case the privdata global variable is not set, the implementation attempts to initialize it. Once the privdata is set for the first time, it cannot change, so all subsequent LOADPRIVDATA/GETPRIVDATA calls will just succeed.

To make sure the LOADPRIVDATA/GETPRIVDATA calls won't fail for these cases we will just preload the privdata global variable during the application start in libc_init().


Updated by Marcel Telka over 5 years ago

Testing of the fix

First, I checked that the fix works as expected and the privdata global
variable is always set during the application start. To confirm this I used
the following script named privtest.mdb:


cat <<EOF | mdb $1
::bp main

Here is an example of results on a system without the fix:

$ ./privtest.mdb /bin/ls
debugging executable file (32-bit)
file: /usr/bin/ls
status: idle
mdb: stop at main
mdb: target stopped at:
main:           leal   0x4(%esp),%ecx

Here is the output on a system with the fix:

$ ./privtest.mdb /bin/ls
debugging executable file (32-bit)
file: /usr/bin/ls
status: idle
mdb: stop at main
mdb: target stopped at:
main:           leal   0x4(%esp),%ecx

Next, I ran the onu-ed system for a while to make sure that everything is
working stable and there are no random core files generated due to the code
added to libc_init().

Finally, I tried to exhaust the memory to see how an application start would
fail. I ran the following 'executor' application:

#include <unistd.h>


    execlp("/bin/bash", "bash", (char *)0);

    return 0;

During the sleep(10) period I started this 'allocator':

#include <stdlib.h>
#include <unistd.h>
#include <stdio.h>

    int s = 1024*1024;

    for (;;) {
        void *p = malloc(s);

        if (p == NULL && s == 1024*1024) {
            s = 1024;

        if (p == NULL) {
            printf("malloc failed\\\\n");

    return 0;

And this happened:

# ./executor &
[1] 103755
# ./allocator 
*** libc thread failure: cannot preload the priv data
malloc failed
[1]+  Abort                   (core dumped) ./executor
# echo ::status | mdb core
debugging core file of bash (32-bit) from openindiana
file: /usr/bin/bash
initial argv: bash
threading model: native threads
status: process terminated by SIGABRT (Abort), pid=103755 uid=0 code=-1
panic message: *** libc thread failure: cannot preload the priv data

So it worked as expected.


Updated by Marcel Telka over 5 years ago

  • Status changed from In Progress to Pending RTI

Updated by Gordon Ross over 5 years ago

  • Status changed from Pending RTI to In Progress

Kicked back (from "pending rti" to "in progress") for questions and possibly more work.

I have a general design question about this fix.

Would it be feasible to change the implementation so that LOADPRIVDATA will never fail?
For example, if we know the maximum size of the data that would be loaded by this, perhaps we could initialize it to point to storage that's reserved at compile time? (i.e. BSS space, or initialized data if we know what the content should be at compile time).
That's a solution I've seen elsewhere for problems of this sort, i.e. when you can't load locale data, you're left with the compiled-in C locale defaults. I'm thinking of something like that.

Could something like that work here?


Updated by Marcel Telka over 4 years ago

For reference I attached my version of the fix.

Also available in: Atom PDF