[Midnightbsd-cvs] src [7744] trunk/lib/libmport: fix some potential thread safety issues with sqlite access

laffer1 at midnightbsd.org laffer1 at midnightbsd.org
Tue Aug 16 22:42:15 EDT 2016


Revision: 7744
          http://svnweb.midnightbsd.org/src/?rev=7744
Author:   laffer1
Date:     2016-08-16 22:42:15 -0400 (Tue, 16 Aug 2016)
Log Message:
-----------
fix some potential thread safety issues with sqlite access

Modified Paths:
--------------
    trunk/lib/libmport/bundle_read_install_pkg.c
    trunk/lib/libmport/instance.c

Modified: trunk/lib/libmport/bundle_read_install_pkg.c
===================================================================
--- trunk/lib/libmport/bundle_read_install_pkg.c	2016-08-15 11:40:03 UTC (rev 7743)
+++ trunk/lib/libmport/bundle_read_install_pkg.c	2016-08-17 02:42:15 UTC (rev 7744)
@@ -332,7 +332,7 @@
 do_actual_install(mportInstance *mport, mportBundleRead *bundle, mportPackageMeta *pkg)
 {
 	mportAssetList *alist = NULL;
-	mportAssetListEntry *e = NULL;
+	__block mportAssetListEntry *e = NULL;
     int file_total;
     int file_count = 0;
     struct archive_entry *entry;
@@ -347,7 +347,7 @@
     char *mkdirp = NULL;
     struct stat sb;
     char file[FILENAME_MAX], cwd[FILENAME_MAX], dir[FILENAME_MAX];
-    sqlite3_stmt *insert = NULL;
+    __block sqlite3_stmt *insert = NULL;
 
     /* sadly, we can't just use abs pathnames, because it will break hardlinks */
     orig_cwd = getcwd(NULL, 0);
@@ -557,122 +557,152 @@
                 break;
         }
 
-        /* insert this asset into the master database */
-        if (sqlite3_bind_int(insert, 1, (int) e->type) != SQLITE_OK) {
-            SET_ERROR(MPORT_ERR_FATAL, sqlite3_errmsg(mport->db));
-            goto ERROR;
-        }
-        if (e->type == ASSET_FILE || e->type == ASSET_SAMPLE || e->type == ASSET_SHELL || e->type == ASSET_FILE_OWNER_MODE) {
-            /* don't put the root in the database! */
-            if (sqlite3_bind_text(insert, 2, file + strlen(mport->root), -1, SQLITE_STATIC) != SQLITE_OK) {
-                SET_ERROR(MPORT_ERR_FATAL, sqlite3_errmsg(mport->db));
-                goto ERROR;
-            }
+		/* insert this asset into the master database */
+		__block int code = MPORT_OK;
+		__block const char *err;
+		dispatch_sync(mportSQLSerial, ^{
+			if (sqlite3_bind_int(insert, 1, (int) e->type) != SQLITE_OK) {
+				code = MPORT_ERR_FATAL;
+				err = sqlite3_errmsg(mport->db);
+				return;
+			}
+			if (e->type == ASSET_FILE || e->type == ASSET_SAMPLE || e->type == ASSET_SHELL ||
+				e->type == ASSET_FILE_OWNER_MODE) {
+				/* don't put the root in the database! */
+				if (sqlite3_bind_text(insert, 2, file + strlen(mport->root), -1, SQLITE_STATIC) != SQLITE_OK) {
+					code = MPORT_ERR_FATAL;
+					err = sqlite3_errmsg(mport->db);
+					return;
+				}
 
-            if (sqlite3_bind_text(insert, 3, e->checksum, -1, SQLITE_STATIC) != SQLITE_OK) {
-                SET_ERROR(MPORT_ERR_FATAL, sqlite3_errmsg(mport->db));
-                goto ERROR;
-            }
+				if (sqlite3_bind_text(insert, 3, e->checksum, -1, SQLITE_STATIC) != SQLITE_OK) {
+					code = MPORT_ERR_FATAL;
+					err = sqlite3_errmsg(mport->db);
+					return;
+				}
 
-		if (e->owner != NULL) {
-	            if (sqlite3_bind_text(insert, 4, e->owner, -1, SQLITE_STATIC) != SQLITE_OK) {
-			SET_ERROR(MPORT_ERR_FATAL, sqlite3_errmsg(mport->db));
-			goto ERROR;
-			}
-		} else {
-			if (sqlite3_bind_null(insert, 4) != SQLITE_OK) {
-				SET_ERROR(MPORT_ERR_FATAL, sqlite3_errmsg(mport->db));
-				goto ERROR;
-			}
-		}
+				if (e->owner != NULL) {
+					if (sqlite3_bind_text(insert, 4, e->owner, -1, SQLITE_STATIC) != SQLITE_OK) {
+						code = MPORT_ERR_FATAL;
+						err = sqlite3_errmsg(mport->db);
+						return;
+					}
+				} else {
+					if (sqlite3_bind_null(insert, 4) != SQLITE_OK) {
+						code = MPORT_ERR_FATAL;
+						err = sqlite3_errmsg(mport->db);
+						return;
+					}
+				}
 
-		if (e->group != NULL) {
-			if (sqlite3_bind_text(insert, 5, e->group, -1, SQLITE_STATIC) != SQLITE_OK) {
-				SET_ERROR(MPORT_ERR_FATAL, sqlite3_errmsg(mport->db));
-				goto ERROR;
-			}
-		} else {
-			if (sqlite3_bind_null(insert, 5) != SQLITE_OK) {
-				SET_ERROR(MPORT_ERR_FATAL, sqlite3_errmsg(mport->db));
-				goto ERROR;
-			}
-		}
+				if (e->group != NULL) {
+					if (sqlite3_bind_text(insert, 5, e->group, -1, SQLITE_STATIC) != SQLITE_OK) {
+						code = MPORT_ERR_FATAL;
+						err = sqlite3_errmsg(mport->db);
+						return;
+					}
+				} else {
+					if (sqlite3_bind_null(insert, 5) != SQLITE_OK) {
+						code = MPORT_ERR_FATAL;
+						err = sqlite3_errmsg(mport->db);
+						return;
+					}
+				}
 
-	    if (e->mode != NULL) {
-	            if (sqlite3_bind_text(insert, 6, e->mode, -1, SQLITE_STATIC) != SQLITE_OK) {
-       		         SET_ERROR(MPORT_ERR_FATAL, sqlite3_errmsg(mport->db));
-               		 goto ERROR;
-            	    }
-            } else {
-	            if (sqlite3_bind_null(insert, 6) != SQLITE_OK) {
-	                SET_ERROR(MPORT_ERR_FATAL, sqlite3_errmsg(mport->db));
-	                goto ERROR;
-	            }
-	   }
-        } else if (e->type == ASSET_DIR || e->type == ASSET_DIRRM || e->type == ASSET_DIRRMTRY) {
-		/* if data starts with /, it's most likely an absolute path. Don't prepend cwd */
-			if (e->data != NULL && e->data[0] == '/')
-				(void) snprintf(dir, FILENAME_MAX, "%s", e->data);
-			else
-				(void) snprintf(dir, FILENAME_MAX, "%s/%s", cwd, e->data);
+				if (e->mode != NULL) {
+					if (sqlite3_bind_text(insert, 6, e->mode, -1, SQLITE_STATIC) != SQLITE_OK) {
+						code = MPORT_ERR_FATAL;
+						err = sqlite3_errmsg(mport->db);
+						return;
+					}
+				} else {
+					if (sqlite3_bind_null(insert, 6) != SQLITE_OK) {
+						code = MPORT_ERR_FATAL;
+						err = sqlite3_errmsg(mport->db);
+						return;
+					}
+				}
+			} else if (e->type == ASSET_DIR || e->type == ASSET_DIRRM || e->type == ASSET_DIRRMTRY) {
+				/* if data starts with /, it's most likely an absolute path. Don't prepend cwd */
+				if (e->data != NULL && e->data[0] == '/')
+					(void) snprintf(dir, FILENAME_MAX, "%s", e->data);
+				else
+					(void) snprintf(dir, FILENAME_MAX, "%s/%s", cwd, e->data);
 
-            if (sqlite3_bind_text(insert, 2, dir, -1, SQLITE_STATIC) != SQLITE_OK) {
-                SET_ERROR(MPORT_ERR_FATAL, sqlite3_errmsg(mport->db));
-                goto ERROR;
-            }
+				if (sqlite3_bind_text(insert, 2, dir, -1, SQLITE_STATIC) != SQLITE_OK) {
+					code = MPORT_ERR_FATAL;
+					err = sqlite3_errmsg(mport->db);
+					return;
+				}
 
-            if (sqlite3_bind_null(insert, 3) != SQLITE_OK) {
-                SET_ERROR(MPORT_ERR_FATAL, sqlite3_errmsg(mport->db));
-                goto ERROR;
-            }
+				if (sqlite3_bind_null(insert, 3) != SQLITE_OK) {
+					code = MPORT_ERR_FATAL;
+					err = sqlite3_errmsg(mport->db);
+					return;
+				}
 
-            if (sqlite3_bind_null(insert, 4) != SQLITE_OK) {
-                SET_ERROR(MPORT_ERR_FATAL, sqlite3_errmsg(mport->db));
-                goto ERROR;
-            }
+				if (sqlite3_bind_null(insert, 4) != SQLITE_OK) {
+					code = MPORT_ERR_FATAL;
+					err = sqlite3_errmsg(mport->db);
+					return;
+				}
 
-            if (sqlite3_bind_null(insert, 5) != SQLITE_OK) {
-                SET_ERROR(MPORT_ERR_FATAL, sqlite3_errmsg(mport->db));
-                goto ERROR;
-            }
+				if (sqlite3_bind_null(insert, 5) != SQLITE_OK) {
+					code = MPORT_ERR_FATAL;
+					err = sqlite3_errmsg(mport->db);
+					return;
+				}
 
-            if (sqlite3_bind_null(insert, 6) != SQLITE_OK) {
-                SET_ERROR(MPORT_ERR_FATAL, sqlite3_errmsg(mport->db));
-                goto ERROR;
-            }
-        } else {
-            if (sqlite3_bind_text(insert, 2, e->data, -1, SQLITE_STATIC) != SQLITE_OK) {
-                SET_ERROR(MPORT_ERR_FATAL, sqlite3_errmsg(mport->db));
-                goto ERROR;
-            }
+				if (sqlite3_bind_null(insert, 6) != SQLITE_OK) {
+					code = MPORT_ERR_FATAL;
+					err = sqlite3_errmsg(mport->db);
+					return;
+				}
+			} else {
+				if (sqlite3_bind_text(insert, 2, e->data, -1, SQLITE_STATIC) != SQLITE_OK) {
+					code = MPORT_ERR_FATAL;
+					err = sqlite3_errmsg(mport->db);
+					return;
+				}
 
-            if (sqlite3_bind_null(insert, 3) != SQLITE_OK) {
-                SET_ERROR(MPORT_ERR_FATAL, sqlite3_errmsg(mport->db));
-                goto ERROR;
-            }
+				if (sqlite3_bind_null(insert, 3) != SQLITE_OK) {
+					code = MPORT_ERR_FATAL;
+					err = sqlite3_errmsg(mport->db);
+					return;
+				}
 
-            if (sqlite3_bind_null(insert, 4) != SQLITE_OK) {
-                SET_ERROR(MPORT_ERR_FATAL, sqlite3_errmsg(mport->db));
-                goto ERROR;
-            }
+				if (sqlite3_bind_null(insert, 4) != SQLITE_OK) {
+					code = MPORT_ERR_FATAL;
+					err = sqlite3_errmsg(mport->db);
+					return;
+				}
 
-            if (sqlite3_bind_null(insert, 5) != SQLITE_OK) {
-                SET_ERROR(MPORT_ERR_FATAL, sqlite3_errmsg(mport->db));
-                goto ERROR;
-            }
+				if (sqlite3_bind_null(insert, 5) != SQLITE_OK) {
+					code = MPORT_ERR_FATAL;
+					err = sqlite3_errmsg(mport->db);
+					return;
+				}
 
-            if (sqlite3_bind_null(insert, 6) != SQLITE_OK) {
-                SET_ERROR(MPORT_ERR_FATAL, sqlite3_errmsg(mport->db));
-                goto ERROR;
-            }
-        }
+				if (sqlite3_bind_null(insert, 6) != SQLITE_OK) {
+					code = MPORT_ERR_FATAL;
+					err = sqlite3_errmsg(mport->db);
+					return;
+				}
+			}
 
-        if (sqlite3_step(insert) != SQLITE_DONE) {
-            SET_ERROR(MPORT_ERR_FATAL, sqlite3_errmsg(mport->db));
-            goto ERROR;
-        }
+			if (sqlite3_step(insert) != SQLITE_DONE) {
+				code = MPORT_ERR_FATAL;
+				err = sqlite3_errmsg(mport->db);
+				return;
+			}
 
-        sqlite3_reset(insert);
+			sqlite3_reset(insert);
+		});
+
+		if (code != MPORT_OK) {
+			SET_ERROR(code, err);
+			goto ERROR;
+		}
     }
 
     sqlite3_finalize(insert);

Modified: trunk/lib/libmport/instance.c
===================================================================
--- trunk/lib/libmport/instance.c	2016-08-15 11:40:03 UTC (rev 7743)
+++ trunk/lib/libmport/instance.c	2016-08-17 02:42:15 UTC (rev 7744)
@@ -55,51 +55,50 @@
 mport_instance_init(mportInstance *mport, const char *root) {
 
 	char dir[FILENAME_MAX];
-        mport->flags = 0;
+	mport->flags = 0;
 
-        if (root != NULL) {
-            mport->root = strdup(root);
-        } else {
-            mport->root = strdup("");
-        }
+	if (root != NULL) {
+		mport->root = strdup(root);
+	} else {
+		mport->root = strdup("");
+	}
 
-        mport_init_queues();
+	mport_init_queues();
 
-        (void) snprintf(dir, FILENAME_MAX, "%s/%s", mport->root, MPORT_INST_DIR);
+	(void) snprintf(dir, FILENAME_MAX, "%s/%s", mport->root, MPORT_INST_DIR);
 
-        if (mport_mkdir(dir) != MPORT_OK) {
+	if (mport_mkdir(dir) != MPORT_OK) {
 		RETURN_CURRENT_ERROR;
 	}
 
-        (void) snprintf(dir, FILENAME_MAX, "%s/%s", mport->root, MPORT_INST_INFRA_DIR);
+	(void) snprintf(dir, FILENAME_MAX, "%s/%s", mport->root, MPORT_INST_INFRA_DIR);
 
-        if (mport_mkdir(dir) != MPORT_OK) {
+	if (mport_mkdir(dir) != MPORT_OK) {
 		RETURN_CURRENT_ERROR;
 	}
 
+	/* dir is a file here, just trying to save memory */
+	(void) snprintf(dir, FILENAME_MAX, "%s/%s", mport->root, MPORT_MASTER_DB_FILE);
+	if (sqlite3_open(dir, &(mport->db)) != 0) {
+		sqlite3_close(mport->db);
+		RETURN_ERROR(MPORT_ERR_FATAL, sqlite3_errmsg(mport->db));
+	}
 
-        /* dir is a file here, just trying to save memory */
-        (void) snprintf(dir, FILENAME_MAX, "%s/%s", mport->root, MPORT_MASTER_DB_FILE);
-        if (sqlite3_open(dir, &(mport->db)) != 0) {
-            sqlite3_close(mport->db);
-            RETURN_ERROR(MPORT_ERR_FATAL, sqlite3_errmsg(mport->db));
-        }
+	if (sqlite3_create_function(mport->db, "mport_version_cmp", 2, SQLITE_ANY, NULL, &mport_version_cmp_sqlite,
+								NULL,
+								NULL) != SQLITE_OK) {
+		sqlite3_close(mport->db);
+		RETURN_ERROR(MPORT_ERR_FATAL, sqlite3_errmsg(mport->db));
+	}
 
-        if (sqlite3_create_function(mport->db, "mport_version_cmp", 2, SQLITE_ANY, NULL, &mport_version_cmp_sqlite,
-                                    NULL,
-                                    NULL) != SQLITE_OK) {
-            sqlite3_close(mport->db);
-            RETURN_ERROR(MPORT_ERR_FATAL, sqlite3_errmsg(mport->db));
-        }
 
+	/* set the default UI callbacks */
+	mport->msg_cb = &mport_default_msg_cb;
+	mport->progress_init_cb = &mport_default_progress_init_cb;
+	mport->progress_step_cb = &mport_default_progress_step_cb;
+	mport->progress_free_cb = &mport_default_progress_free_cb;
+	mport->confirm_cb = &mport_default_confirm_cb;
 
-        /* set the default UI callbacks */
-        mport->msg_cb = &mport_default_msg_cb;
-        mport->progress_init_cb = &mport_default_progress_init_cb;
-        mport->progress_step_cb = &mport_default_progress_step_cb;
-        mport->progress_free_cb = &mport_default_progress_free_cb;
-        mport->confirm_cb = &mport_default_confirm_cb;
-
 	int db_version = mport_get_database_version(mport->db);
 	if (db_version < 1) {
 		/* new, create tables */



More information about the Midnightbsd-cvs mailing list