On 02/28/2018 05:15 AM, Vit Mojzis wrote: > F_OK access checks only work properly as long as all directories along > the path are accessible to real user running the program. > Replace F_OK access checks by testing return value of open, write, etc. > > Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1186431 > > Signed-off-by: Vit Mojzis <vmojzis@xxxxxxxxxx> > --- > libsemanage/src/direct_api.c | 39 +++++++++++++++------------------------ > 1 file changed, 15 insertions(+), 24 deletions(-) > > diff --git a/libsemanage/src/direct_api.c b/libsemanage/src/direct_api.c > index b7899d68..f4d57cf8 100644 > --- a/libsemanage/src/direct_api.c > +++ b/libsemanage/src/direct_api.c > @@ -1563,34 +1563,25 @@ rebuild: > goto cleanup; > } > > - path = semanage_path(SEMANAGE_TMP, SEMANAGE_STORE_FC_LOCAL); > - if (access(path, F_OK) == 0) { > - retval = semanage_copy_file(semanage_path(SEMANAGE_TMP, SEMANAGE_STORE_FC_LOCAL), > - semanage_final_path(SEMANAGE_FINAL_TMP, SEMANAGE_FC_LOCAL), > - sh->conf->file_mode); > - if (retval < 0) { > - goto cleanup; > - } > + retval = semanage_copy_file(semanage_path(SEMANAGE_TMP, SEMANAGE_STORE_FC_LOCAL), > + semanage_final_path(SEMANAGE_FINAL_TMP, SEMANAGE_FC_LOCAL), > + sh->conf->file_mode); > + if (retval < 0 && errno != ENOENT) { > + goto cleanup; > } > > - path = semanage_path(SEMANAGE_TMP, SEMANAGE_STORE_FC); > - if (access(path, F_OK) == 0) { > - retval = semanage_copy_file(semanage_path(SEMANAGE_TMP, SEMANAGE_STORE_FC), > - semanage_final_path(SEMANAGE_FINAL_TMP, SEMANAGE_FC), > - sh->conf->file_mode); > - if (retval < 0) { > - goto cleanup; > - } > + retval = semanage_copy_file(semanage_path(SEMANAGE_TMP, SEMANAGE_STORE_FC), > + semanage_final_path(SEMANAGE_FINAL_TMP, SEMANAGE_FC), > + sh->conf->file_mode); > + if (retval < 0 && errno != ENOENT) { > + goto cleanup; > } > > - path = semanage_path(SEMANAGE_TMP, SEMANAGE_STORE_SEUSERS); > - if (access(path, F_OK) == 0) { > - retval = semanage_copy_file(semanage_path(SEMANAGE_TMP, SEMANAGE_STORE_SEUSERS), > - semanage_final_path(SEMANAGE_FINAL_TMP, SEMANAGE_SEUSERS), > - sh->conf->file_mode); > - if (retval < 0) { > - goto cleanup; > - } > + retval = semanage_copy_file(semanage_path(SEMANAGE_TMP, SEMANAGE_STORE_SEUSERS), > + semanage_final_path(SEMANAGE_FINAL_TMP, SEMANAGE_SEUSERS), > + sh->conf->file_mode); > + if (retval < 0 && errno != ENOENT) { > + goto cleanup; > } I think we need to clear retval here to ensure that we don't later misinterpret a negative retval as an error. Otherwise, if disable_genhomedircon && !do_install, we'll fall through to the end of the function without ever setting retval again, so the mere absence of a seusers file will cause a fatal error from semanage_direct_commit. > > /* run genhomedircon if its enabled, this should be the last operation
