On Mon, 2018-01-22 at 16:38 +0000, Richard Haines wrote:
> Allow the tmp build files to be kept for debugging when a policy
> build fails.
>
> Signed-off-by: Richard Haines <richard_c_haines@xxxxxxxxxxxxxx>
> ---
> V2 Changes:
> Remove the retain-tmp flag and just keep tmp files on build errors.
>
> libsemanage/src/direct_api.c | 54 ++++++++++++++++++++++++++++++--
> ------------
> libsemanage/src/handle.c | 2 ++
> libsemanage/src/handle.h | 1 +
> 3 files changed, 40 insertions(+), 17 deletions(-)
>
> diff --git a/libsemanage/src/direct_api.c
> b/libsemanage/src/direct_api.c
> index a455612f..3d1cf1fe 100644
> --- a/libsemanage/src/direct_api.c
> +++ b/libsemanage/src/direct_api.c
> @@ -323,26 +323,44 @@ static void
> semanage_direct_destroy(semanage_handle_t * sh
> /* do nothing */
> }
>
> -static int semanage_direct_disconnect(semanage_handle_t * sh)
> +static int semanage_remove_tmps(semanage_handle_t *sh)
> {
> - /* destroy transaction */
> - if (sh->is_in_transaction) {
> - /* destroy sandbox */
> - if (semanage_remove_directory
> - (semanage_path(SEMANAGE_TMP, SEMANAGE_TOPLEVEL))
> < 0) {
> + if (sh->commit_err)
> + return 0;
> +
> + /* destroy sandbox if it exists */
> + if (semanage_remove_directory
> + (semanage_path(SEMANAGE_TMP, SEMANAGE_TOPLEVEL)) < 0) {
> + if (errno != ENOENT) {
> ERR(sh, "Could not cleanly remove sandbox
> %s.",
> semanage_path(SEMANAGE_TMP,
> SEMANAGE_TOPLEVEL));
> return -1;
> }
> - if (semanage_remove_directory
> - (semanage_final_path(SEMANAGE_FINAL_TMP,
> - SEMANAGE_FINAL_TOPLEVEL)) <
> 0) {
> + }
> +
> + /* destroy tmp policy if it exists */
> + if (semanage_remove_directory
> + (semanage_final_path(SEMANAGE_FINAL_TMP,
> + SEMANAGE_FINAL_TOPLEVEL)) < 0) {
> + if (errno != ENOENT) {
> ERR(sh, "Could not cleanly remove tmp %s.",
> semanage_final_path(SEMANAGE_FINAL_TMP,
> SEMANAGE_FINAL_TOPLE
> VEL));
> return -1;
> }
> + }
> +
> + return 0;
> +}
> +
> +static int semanage_direct_disconnect(semanage_handle_t *sh)
> +{
> + int retval = 0;
> +
> + /* destroy transaction and remove tmp files if no commit
> error */
> + if (sh->is_in_transaction) {
> semanage_release_trans_lock(sh);
Don't we need to wait until after we have deleted it to release the
trans lock? Otherwise, what prevents another process from taking the
trans lock, creating the tmp directory, and having it deleted
underneath it by this process?
> + retval = semanage_remove_tmps(sh);
> }
>
> /* Release object databases: local modifications */
> @@ -375,7 +393,7 @@ static int
> semanage_direct_disconnect(semanage_handle_t * sh)
> /* Release object databases: active kernel policy */
> bool_activedb_dbase_release(semanage_bool_dbase_active(sh));
>
> - return 0;
> + return retval;
> }
>
> static int semanage_direct_begintrans(semanage_handle_t * sh)
> @@ -1639,13 +1657,15 @@ cleanup:
>
> free(fc_buffer);
>
> - /* regardless if the commit was successful or not, remove
> the
> - sandbox if it is still there */
> - semanage_remove_directory(semanage_path
> - (SEMANAGE_TMP,
> SEMANAGE_TOPLEVEL));
> - semanage_remove_directory(semanage_final_path
> - (SEMANAGE_FINAL_TMP,
> - SEMANAGE_FINAL_TOPLEVEL));
> + /* Set commit_err so other functions can detect any errors.
> Note that
> + * retval > 0 will be the commit number.
> + */
> + if (retval < 0)
> + sh->commit_err = retval;
> +
> + if (semanage_remove_tmps(sh) != 0)
> + retval = -1;
> +
> umask(mask);
>
> return retval;
> diff --git a/libsemanage/src/handle.c b/libsemanage/src/handle.c
> index 4ce1df03..a6567bd4 100644
> --- a/libsemanage/src/handle.c
> +++ b/libsemanage/src/handle.c
> @@ -86,6 +86,8 @@ semanage_handle_t *semanage_handle_create(void)
> * If any changes are made, this flag is ignored */
> sh->do_rebuild = 0;
>
> + sh->commit_err = 0;
> +
> /* By default always reload policy after commit if SELinux
> is enabled. */
> sh->do_reload = (is_selinux_enabled() > 0);
>
> diff --git a/libsemanage/src/handle.h b/libsemanage/src/handle.h
> index 1780ac81..65b15600 100644
> --- a/libsemanage/src/handle.h
> +++ b/libsemanage/src/handle.h
> @@ -62,6 +62,7 @@ struct semanage_handle {
> int is_in_transaction;
> int do_reload; /* whether to reload policy
> after commit */
> int do_rebuild; /* whether to rebuild policy
> if there were no changes */
> + int commit_err; /* set by
> semanage_direct_commit() */
> int modules_modified;
> int create_store; /* whether to create the store if
> it does not exist
> * this will only have an effect on
> direct connections */
