Actions
Bug #7394
openlibapr computes incorrect size for dirent structure
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.
Updated by Alexander Pyhalov over 5 years ago
Updated by Denis Kozadaev over 5 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.
Updated by Peter Tribble over 5 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.
Updated by Denis Kozadaev over 5 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
Actions