Project

General

Profile

Bug #7394

libapr computes incorrect size for dirent structure

Added by Denis Kozadaev about 4 years ago. Updated about 4 years ago.

Status:
New
Priority:
Normal
Assignee:
-
Category:
-
Target version:
-
Start date:
2016-09-20
Due date:
% Done:

0%

Estimated time:
Difficulty:
Medium
Tags:
needs-triage

Description

The library has a function to open a directory:

apr_status_t apr_dir_open(apr_dir_t **new, const char *dirname, 
                          apr_pool_t *pool)
{
    /* On some platforms (e.g., Linux+GNU libc), d_name[] in struct 
     * dirent is declared with enough storage for the name.  On other
     * platforms (e.g., Solaris 8 for Intel), d_name is declared as a
     * one-byte array.  Note: gcc evaluates this at compile time.
     */
    apr_size_t dirent_size = 
        sizeof(*(*new)->entry) +
        (sizeof((*new)->entry->d_name) > 1 ? 0 : 255);
    DIR *dir = opendir(dirname);

    if (!dir) {
        return errno;
    }

    (*new) = (apr_dir_t *)apr_palloc(pool, sizeof(apr_dir_t));

    (*new)->pool = pool;
    (*new)->dirname = apr_pstrdup(pool, dirname);
    (*new)->dirstruct = dir;
    (*new)->entry = apr_pcalloc(pool, dirent_size);

    apr_pool_cleanup_register((*new)->pool, *new, dir_cleanup,
                              apr_pool_cleanup_null);
    return APR_SUCCESS;
}

The problem is dirent_size computed incorrectly, as the result apache
crashes on SPARC:
Starting program: /usr/sbin/apache2 
[Thread debugging using libthread_db enabled]
[Tue Sep 20 12:09:22.992753 2016] [core:warn] [pid 94546] AH00111: Config variable ${APACHE_LOCK_DIR} is not defined
[Tue Sep 20 12:09:22.992996 2016] [core:warn] [pid 94546] AH00111: Config variable ${APACHE_PID_FILE} is not defined
[Tue Sep 20 12:09:22.993188 2016] [core:warn] [pid 94546] AH00111: Config variable ${APACHE_RUN_USER} is not defined
[Tue Sep 20 12:09:22.993248 2016] [core:warn] [pid 94546] AH00111: Config variable ${APACHE_RUN_GROUP} is not defined
[Tue Sep 20 12:09:22.993411 2016] [core:warn] [pid 94546] AH00111: Config variable ${APACHE_LOG_DIR} is not defined
[New Thread 1 (LWP 1)]

Thread 2 received signal SIGSEGV, Segmentation fault.
[Switching to Thread 1 (LWP 1)]
apr_pool_cleanup_kill (p=p@entry=0x1001d6b98, data=data@entry=0x1001d6fc8, cleanup_fn=cleanup_fn@entry=0xffffffff01319b2c <dir_cleanup>)
    at /ws/jenkins/ws/du/components/apr/build/apr-1.5.2-4/memory/unix/apr_pools.c:2276
2276            if (c->data == data && c->plain_cleanup_fn == cleanup_fn) {
(gdb) p *p->cleanups
$2 = {next = 0x617369632e6c6f61, data = 0x6400000000000000, plain_cleanup_fn = 0xffffffff01319b2c <dir_cleanup>, 
  child_cleanup_fn = 0x10018cce0 <apr_pool_cleanup_null@plt>}
(gdb) p (char *)p->cleanups
$3 = 0x1001d7020 "asic.load"

As you see it is a tail of a string.
X86 Illumos defined d_name in dirent structure as char d_name1, but on SPARC
this field defined as char d_name3 (why?)
The expression from the function (sizeof((*new)->entry->d_name) > 1 ? 0 : 255)
returns 255 for x86 (because size of d_name is 1) and 0 for SPARC.
I suggest replace it by PATH_MAX from limits.h or something like that.

#2

Updated by Denis Kozadaev about 4 years ago

sizeof(struct dirent) includes d_name array in the result,
it already has an extra space (minimum one byte) for terminating '\0',
but the second part of this expression is incorrect if d_name array
has size more than one byte. On SPARC is has 3 bytes and this part
of the expression returns 0, so, we have for names only 3 bytes
including '\0' which is incorrect.

#3

Updated by Peter Tribble about 4 years ago

Where does the d_name being length 3 on SPARC come from? It's defined as 1 on both SPARC and X86.

I've tested this on x86 and sparc, and the original apr code works correctly for me.

#4

Updated by Denis Kozadaev about 4 years ago 

bld10% uname -a
SunOS bld10 5.11 1.3.7.151 sun4v sparc SUNW,SPARC-Enterprise-T5220
bld10% sha256sum /usr/include/sys/dirent.h
156fff8b26590fb577cc5515d20ee51d0c67060c862027ceb542f78aa9372980  /usr/include/sys/dirent.h


/*
 * File-system independent directory entry.
 */
typedef struct dirent {
        ino_t           d_ino;          /* "inode number" of entry */
        off_t           d_off;          /* offset of disk directory entry */
        unsigned short  d_reclen;       /* length of this record */
        char            d_name[3];      /* name of file */
} dirent_t;

#if defined(_SYSCALL32)

/* kernel's view of user ILP32 dirent */

typedef struct dirent32 {
        ino32_t         d_ino;          /* "inode number" of entry */
        off32_t         d_off;          /* offset of disk directory entry */
        uint16_t        d_reclen;       /* length of this record */
        char            d_name[3];      /* name of file */
} dirent32_t;

#endif  /* _SYSCALL32 */

DilOS (based on illumos) Version 1.3.7.151 2016-10-03

Also available in: Atom PDF