Fix a problem in os_unix.c where a malloc failure could lead to a leaked file descriptor.

FossilOrigin-Name: aa6acfa8caa2ef59b4c16dfe42c4b5644da96905
This commit is contained in:
dan 2009-08-22 11:39:46 +00:00
parent 08da86a68e
commit e946c3966a
4 changed files with 149 additions and 106 deletions

View File

@ -1,5 +1,5 @@
C When\sa\sdatabase\sfile\sis\sopened,\stry\sto\sfind\san\sunused\sfile\sdescriptor\sto\sreuse.\s\sThis\schange\saffects\sunix\s(and\sother\ssystems\sthat\suse\sos_unix.c)\sonly.\sFix\sfor\scvstrac\sticket\s[http://www.sqlite.org/cvstrac/tktview?tn=4018|#4018].
D 2009-08-21T17:18:03
C Fix\sa\sproblem\sin\sos_unix.c\swhere\sa\smalloc\sfailure\scould\slead\sto\sa\sleaked\sfile\sdescriptor.
D 2009-08-22T11:39:47
F Makefile.arm-wince-mingw32ce-gcc fcd5e9cd67fe88836360bb4f9ef4cb7f8e2fb5a0
F Makefile.in 73ddeec9dd10b85876c5c2ce1fdce627e1dcc7f8
F Makefile.linux-gcc d53183f4aa6a9192d249731c90dbdffbd2c68654
@ -145,7 +145,7 @@ F src/os.c 9fea283e336ee31caa4654d6cb05a129a1c42d2f
F src/os.h 00a1334a4eecee7f7bef79ac606b88d325119f21
F src/os_common.h 8c61457df58f1a4bd5f5adc3e90e01b37bf7afbc
F src/os_os2.c bed77dc26e3a95ce4a204936b9a1ca6fe612fcc5
F src/os_unix.c 1546de71b888c9a2bb0589d04e7e4267d40ef944
F src/os_unix.c f151f27d353bf4240896d9a1386e5d59102a3cba
F src/os_win.c 58bb163f327e79726dd119344d908e4d98483c3f
F src/pager.c a47be286477ed6c7b9a342dd53d4e4043f29d8c2
F src/pager.h 11852d044c86cf5a9d6e34171fb0c4fcf1f6265f
@ -664,7 +664,7 @@ F test/tkt3929.test 6a4c3baefb4e75127356b7d675b5df42c35c00d1
F test/tkt3935.test e15261fedb9e30a4305a311da614a5d8e693c767
F test/tkt3992.test c193b9643b1c25d020c503a986d5e4089e65c530
F test/tkt3997.test a335fa41ca3985660a139df7b734a26ef53284bd
F test/tkt4018.test f581cf52dc359171875cb649bdc38b525d7b9309
F test/tkt4018.test 7c2c9ba4df489c676a0a7a0e809a1fb9b2185bd1
F test/tokenize.test ce430a7aed48fc98301611429595883fdfcab5d7
F test/trace.test 19ffbc09885c3321d56358a5738feae8587fb377
F test/trans.test d887cb07630dc39879a322d958ad8b006137485c
@ -748,7 +748,7 @@ F tool/speedtest2.tcl ee2149167303ba8e95af97873c575c3e0fab58ff
F tool/speedtest8.c 2902c46588c40b55661e471d7a86e4dd71a18224
F tool/speedtest8inst1.c 293327bc76823f473684d589a8160bde1f52c14e
F tool/vdbe-compress.tcl 672f81d693a03f80f5ae60bfefacd8a349e76746
P 75f596a04a74eb3a538c7be5b41756c970a21a1b
R a2800e2c5932867798528426b058f0c9
P 9b4d9ab62d687289837b13b07885e72cc3abe8a9
R cdd26bf118eaa2a9465bbbd01490d6db
U dan
Z 0551703c5a252ef4437a61cdae64c5ea
Z 37f9fc37ad160151ec2e7b69ade6b18c

View File

@ -1 +1 @@
9b4d9ab62d687289837b13b07885e72cc3abe8a9
aa6acfa8caa2ef59b4c16dfe42c4b5644da96905

View File

@ -167,6 +167,19 @@
#define IS_LOCK_ERROR(x) ((x != SQLITE_OK) && (x != SQLITE_BUSY))
/*
** Sometimes, after a file handle is closed by SQLite, the file descriptor
** cannot be closed immediately. In these cases, instances of the following
** structure are used to store the file descriptor while waiting for an
** opportunity to either close or reuse it.
*/
typedef struct UnixUnusedFd UnixUnusedFd;
struct UnixUnusedFd {
int fd; /* File descriptor to close */
int flags; /* Flags this file descriptor was opened with */
UnixUnusedFd *pNext; /* Next unused file descriptor on same file */
};
/*
** The unixFile structure is subclass of sqlite3_file specific to the unix
** VFS implementations.
@ -181,7 +194,7 @@ struct unixFile {
unsigned char locktype; /* The type of lock held on this fd */
int lastErrno; /* The unix errno from the last I/O error */
void *lockingContext; /* Locking style specific state */
int flags; /* Flags value returned by xOpen() */
UnixUnusedFd *pUnused; /* Pre-allocated UnixUnusedFd */
#if SQLITE_ENABLE_LOCKING_STYLE
int openFlags; /* The flags specified at open() */
#endif
@ -748,11 +761,7 @@ struct unixOpenCnt {
struct unixFileId fileId; /* The lookup key */
int nRef; /* Number of pointers to this structure */
int nLock; /* Number of outstanding locks */
int nPending; /* Number of pending close() operations */
struct PendingClose {
int fd; /* File descriptor to close */
int flags; /* Flags this file descriptor was opened with */
} *aPending; /* Malloced space holding fds awaiting close() */
UnixUnusedFd *pUnused; /* Unused file descriptors to close */
#if OS_VXWORKS
sem_t *pSem; /* Named POSIX semaphore */
char aSemName[MAX_PATHNAME+1]; /* Name of that semaphore */
@ -910,7 +919,7 @@ static void releaseOpenCnt(struct unixOpenCnt *pOpen){
assert( pOpen->pNext->pPrev==pOpen );
pOpen->pNext->pPrev = pOpen->pPrev;
}
sqlite3_free(pOpen->aPending);
assert( !pOpen->pUnused );
sqlite3_free(pOpen);
}
}
@ -1028,19 +1037,12 @@ static int findLockInfo(
rc = SQLITE_NOMEM;
goto exit_findlockinfo;
}
memset(pOpen, 0, sizeof(*pOpen));
pOpen->fileId = fileId;
pOpen->nRef = 1;
pOpen->nLock = 0;
pOpen->nPending = 0;
pOpen->aPending = 0;
pOpen->pNext = openList;
pOpen->pPrev = 0;
if( openList ) openList->pPrev = pOpen;
openList = pOpen;
#if OS_VXWORKS
pOpen->pSem = NULL;
pOpen->aSemName[0] = '\0';
#endif
}else{
pOpen->nRef++;
}
@ -1405,57 +1407,47 @@ end_lock:
}
/*
** Close all file descriptors accumuated in the p->aPending[] array. If
** all such file descriptors are closed without error, the aPending[]
** array is deleted and SQLITE_OK returned.
** Close all file descriptors accumuated in the unixOpenCnt->pUnused list.
** If all such file descriptors are closed without error, the list is
** cleared and SQLITE_OK returned.
**
** Otherwise, if an error occurs, then successfully closed file descriptor
** entries in the aPending[] array are set to -1, the aPending[] array
** entries are removed from the list, and SQLITE_IOERR_CLOSE returned.
** not deleted and SQLITE_IOERR_CLOSE returned.
*/
static int closePendingFds(unixFile *pFile){
struct unixOpenCnt *pOpen = pFile->pOpen;
struct PendingClose *aPending = pOpen->aPending;
int i;
int rc = SQLITE_OK;
assert( unixMutexHeld() );
for(i=0; i<pOpen->nPending; i++){
if( aPending[i].fd>=0 ){
if( close(aPending[i].fd) ){
pFile->lastErrno = errno;
rc = SQLITE_IOERR_CLOSE;
}else{
aPending[i].fd = -1;
}
struct unixOpenCnt *pOpen = pFile->pOpen;
UnixUnusedFd *pError = 0;
UnixUnusedFd *p;
UnixUnusedFd *pNext;
for(p=pOpen->pUnused; p; p=pNext){
pNext = p->pNext;
if( close(p->fd) ){
pFile->lastErrno = errno;
rc = SQLITE_IOERR_CLOSE;
p->pNext = pError;
pError = p;
assert(0);
}else{
sqlite3_free(p);
}
}
if( rc==SQLITE_OK ){
sqlite3_free(aPending);
pOpen->nPending = 0;
pOpen->aPending = 0;
}
pOpen->pUnused = pError;
return rc;
}
/*
** Add the file descriptor used by file handle pFile to the corresponding
** aPending[] array to be closed after some other connection releases
** a lock.
** pUnused list.
*/
static void setPendingFd(unixFile *pFile){
struct PendingClose *aNew;
struct unixOpenCnt *pOpen = pFile->pOpen;
int nByte = (pOpen->nPending+1)*sizeof(pOpen->aPending[0]);
aNew = sqlite3_realloc(pOpen->aPending, nByte);
if( aNew==0 ){
/* If a malloc fails, just leak the file descriptor */
}else{
pOpen->aPending = aNew;
pOpen->aPending[pOpen->nPending].fd = pFile->h;
pOpen->aPending[pOpen->nPending].flags = pFile->flags;
pOpen->nPending++;
pFile->h = -1;
}
UnixUnusedFd *p = pFile->pUnused;
p->pNext = pOpen->pUnused;
pOpen->pUnused = p;
pFile->h = -1;
pFile->pUnused = 0;
}
/*
@ -1573,7 +1565,7 @@ static int unixUnlock(sqlite3_file *id, int locktype){
pOpen = pFile->pOpen;
pOpen->nLock--;
assert( pOpen->nLock>=0 );
if( pOpen->nLock==0 && pOpen->nPending>0 ){
if( pOpen->nLock==0 ){
int rc2 = closePendingFds(pFile);
if( rc==SQLITE_OK ){
rc = rc2;
@ -1627,6 +1619,7 @@ static int closeUnixFile(sqlite3_file *id){
#endif
OSTRACE2("CLOSE %-3d\n", pFile->h);
OpenCounter(-1);
sqlite3_free(pFile->pUnused);
memset(pFile, 0, sizeof(unixFile));
}
return SQLITE_OK;
@ -1644,8 +1637,8 @@ static int unixClose(sqlite3_file *id){
if( pFile->pOpen && pFile->pOpen->nLock ){
/* If there are outstanding locks, do not actually close the file just
** yet because that would clear those locks. Instead, add the file
** descriptor to pOpen->aPending. It will be automatically closed when
** the last lock is cleared.
** descriptor to pOpen->pUnused list. It will be automatically closed
** when the last lock is cleared.
*/
setPendingFd(pFile);
}
@ -2734,7 +2727,7 @@ static int unixRead(
/* If this is a database file (not a journal, master-journal or temp
** file), the bytes in the locking range should never be read or written. */
assert( (pFile->flags&SQLITE_OPEN_MAIN_DB)==0
assert( pFile->pUnused==0
|| offset>=PENDING_BYTE+512
|| offset+amt<=PENDING_BYTE
);
@ -2807,7 +2800,7 @@ static int unixWrite(
/* If this is a database file (not a journal, master-journal or temp
** file), the bytes in the locking range should never be read or written. */
assert( (pFile->flags&SQLITE_OPEN_MAIN_DB)==0
assert( pFile->pUnused==0
|| offset>=PENDING_BYTE+512
|| offset+amt<=PENDING_BYTE
);
@ -3174,7 +3167,7 @@ static int unixDeviceCharacteristics(sqlite3_file *NotUsed){
**
** (1) The real finder-function named "FImpt()".
**
** (2) A constant pointer to this functio named just "F".
** (2) A constant pointer to this function named just "F".
**
**
** A pointer to the F pointer is used as the pAppData value for VFS
@ -3438,13 +3431,10 @@ static int fillInUnixFile(
assert( pNew->pLock==NULL );
assert( pNew->pOpen==NULL );
/* Parameter isDelete is only used on vxworks.
** Express this explicitly here to prevent compiler warnings
** about unused parameters.
/* Parameter isDelete is only used on vxworks. Express this explicitly
** here to prevent compiler warnings about unused parameters.
*/
#if !OS_VXWORKS
UNUSED_PARAMETER(isDelete);
#endif
OSTRACE3("OPEN %-3d %s\n", h, zFilename);
pNew->h = h;
@ -3474,6 +3464,28 @@ static int fillInUnixFile(
if( pLockingStyle == &posixIoMethods ){
unixEnterMutex();
rc = findLockInfo(pNew, &pNew->pLock, &pNew->pOpen);
if( rc!=SQLITE_OK ){
/* If an error occured in findLockInfo(), close the file descriptor
** immediately, before releasing the mutex. findLockInfo() may fail
** in two scenarios:
**
** (a) A call to fstat() failed.
** (b) A malloc failed.
**
** Scenario (b) may only occur if the process is holding no other
** file descriptors open on the same file. If there were other file
** descriptors on this file, then no malloc would be required by
** findLockInfo(). If this is the case, it is quite safe to close
** handle h - as it is guaranteed that no posix locks will be released
** by doing so.
**
** If scenario (a) caused the error then things are not so safe. The
** implicit assumption here is that if fstat() fails, things are in
** such bad shape that dropping a lock or two doesn't matter much.
*/
close(h);
h = -1;
}
unixLeaveMutex();
}
@ -3549,7 +3561,7 @@ static int fillInUnixFile(
#endif
if( rc!=SQLITE_OK ){
if( dirfd>=0 ) close(dirfd); /* silent leak if fail, already in error */
close(h);
if( h>=0 ) close(h);
}else{
pNew->pMethod = pLockingStyle;
OpenCounter(+1);
@ -3674,8 +3686,15 @@ static int proxyTransformUnixFile(unixFile*, const char*);
** If a suitable file descriptor is found, then it is returned. If no
** such file descriptor is located, -1 is returned.
*/
static int findReusableFd(const char *zPath, int flags){
int fd = -1; /* Return value */
static UnixUnusedFd *findReusableFd(const char *zPath, int flags){
UnixUnusedFd *pUnused = 0;
/* Do not search for an unused file descriptor on vxworks. Not because
** vxworks would not benefit from the change (it might, we're not sure),
** but because no way to test it is currently available. It is better
** not to risk breaking vxworks support for the sake of such an obscure
** feature. */
#if !OS_VXWORKS
struct stat sStat; /* Results of stat() call */
/* A stat() call may fail for various reasons. If this happens, it is
@ -3687,28 +3706,25 @@ static int findReusableFd(const char *zPath, int flags){
** Even if a subsequent open() call does succeed, the consequences of
** not searching for a resusable file descriptor are not dire. */
if( 0==stat(zPath, &sStat) ){
struct unixOpenCnt *p;
struct unixOpenCnt *pO;
struct unixFileId id;
id.dev = sStat.st_dev;
id.ino = sStat.st_ino;
unixEnterMutex();
for(p=openList; p&& memcmp(&id, &p->fileId, sizeof(id)); p=p->pNext);
if( p && p->aPending ){
int i;
struct PendingClose *aPending = p->aPending;
for(i=0; i<p->nPending; i++){
if( aPending[i].fd>=0 && flags==aPending[i].flags ){
fd = aPending[i].fd;
aPending[i].fd = -1;
break;
}
for(pO=openList; pO && memcmp(&id, &pO->fileId, sizeof(id)); pO=pO->pNext);
if( pO ){
UnixUnusedFd **pp;
for(pp=&pO->pUnused; *pp && (*pp)->flags!=flags; pp=&((*pp)->pNext));
pUnused = *pp;
if( pUnused ){
*pp = pUnused->pNext;
}
}
unixLeaveMutex();
}
return fd;
#endif /* if !OS_VXWORKS */
return pUnused;
}
/*
@ -3796,14 +3812,17 @@ static int unixOpen(
memset(p, 0, sizeof(unixFile));
if( eType==SQLITE_OPEN_MAIN_DB ){
/* Try to find an unused file descriptor to reuse. This is not done
** for vxworks. Not because vxworks would not benefit from the change
** (it might, we're not sure), but because no way to test it is
** currently available. It is better not to risk breaking vxworks for
** the sake of such an obscure feature. */
#if !OS_VXWORKS
fd = findReusableFd(zName, flags);
#endif
UnixUnusedFd *pUnused;
pUnused = findReusableFd(zName, flags);
if( pUnused ){
fd = pUnused->fd;
}else{
pUnused = sqlite3_malloc(sizeof(pUnused));
if( !pUnused ){
return SQLITE_NOMEM;
}
}
p->pUnused = pUnused;
}else if( !zName ){
/* If zName is NULL, the upper layer is requesting a temp file. */
assert(isDelete && !isOpenDirectory);
@ -3825,24 +3844,32 @@ static int unixOpen(
openFlags |= (O_LARGEFILE|O_BINARY);
if( fd<0 ){
fd = open(zName, openFlags, isDelete?0600:SQLITE_DEFAULT_FILE_PERMISSIONS);
mode_t openMode = (isDelete?0600:SQLITE_DEFAULT_FILE_PERMISSIONS);
fd = open(zName, openFlags, openMode);
OSTRACE4("OPENX %-3d %s 0%o\n", fd, zName, openFlags);
if( fd<0 && errno!=EISDIR && isReadWrite && !isExclusive ){
/* Failed to open the file for read/write access. Try read-only. */
flags &= ~(SQLITE_OPEN_READWRITE|SQLITE_OPEN_CREATE);
openFlags &= ~(O_RDWR|O_CREAT);
flags |= SQLITE_OPEN_READONLY;
return unixOpen(pVfs, zPath, pFile, flags, pOutFlags);
openFlags |= O_RDONLY;
fd = open(zName, openFlags, openMode);
}
if( fd<0 ){
return SQLITE_CANTOPEN;
rc = SQLITE_CANTOPEN;
goto open_finished;
}
}
assert( fd>=0 );
p->flags = flags;
if( pOutFlags ){
*pOutFlags = flags;
}
if( p->pUnused ){
p->pUnused->fd = fd;
p->pUnused->flags = flags;
}
if( isDelete ){
#if OS_VXWORKS
zPath = zName;
@ -3861,11 +3888,11 @@ static int unixOpen(
if( rc!=SQLITE_OK ){
/* It is safe to close fd at this point, because it is guaranteed not
** to be open on a database file. If it were open on a database file,
** it would not be safe to close as this would cause any locks held
** on the file by this process to be released. */
** it would not be safe to close as this would release any locks held
** on the file by this process. */
assert( eType!=SQLITE_OPEN_MAIN_DB );
close(fd); /* silently leak if fail, already in error */
return rc;
goto open_finished;
}
}
@ -3887,10 +3914,20 @@ static int unixOpen(
}else{
struct statfs fsInfo;
if( statfs(zPath, &fsInfo) == -1 ){
((unixFile*)pFile)->lastErrno = errno;
if( dirfd>=0 ) close(dirfd); /* silently leak if fail, in error */
/* In theory, the close(fd) call is sub-optimal. If the file opened
** with fd is a database file, and there are other connections open
** on that file that are currently holding advisory locks on it,
** then the call to close() will cancel those locks. In practice,
** we're assuming that statfs() doesn't fail very often. At least
** not while other file descriptors opened by the same process on
** the same file are working. */
p->lastErrno = errno;
if( dirfd>=0 ){
close(dirfd); /* silently leak if fail, in error */
}
close(fd); /* silently leak if fail, in error */
return SQLITE_IOERR_ACCESS;
rc = SQLITE_IOERR_ACCESS;
goto open_finished;
}
useProxy = !(fsInfo.f_flags&MNT_LOCAL);
}
@ -3899,14 +3936,20 @@ static int unixOpen(
if( rc==SQLITE_OK ){
rc = proxyTransformUnixFile((unixFile*)pFile, ":auto:");
}
return rc;
goto open_finished;
}
}
#endif
return fillInUnixFile(pVfs, fd, dirfd, pFile, zPath, noLock, isDelete);
rc = fillInUnixFile(pVfs, fd, dirfd, pFile, zPath, noLock, isDelete);
open_finished:
if( rc!=SQLITE_OK ){
sqlite3_free(p->pUnused);
}
return rc;
}
/*
** Delete the file at zPath. If the dirSync argument is true, fsync()
** the directory after deleting the file.

View File

@ -43,7 +43,7 @@ do_test tkt4018-1.1 {
} {}
# The database is locked by connection [db]. Open and close a second
# connection to test.db 20000 times. If file-descriptors are not being
# connection to test.db 10000 times. If file-descriptors are not being
# reused, then the process will quickly exceed its maximum number of
# file descriptors (1024 by default on linux).
do_test tkt4018-1.2 {