Yes, we already discussed this possibile race condition. Usually there is only one system administrator operating on the semanage store, nevertheless it's worth having a robust locking mechanism... This patch either needs further work to avoid using flock() and instead using a simpler file lock mechanism with the added benefit of having a cleaner filesystem without confusing stale files around or we just drop the patch given it is not essential to keep things working. Regards, Guido On the 24th of April 2017 14:08:22 CEST, Alan Jenkins <alan.christopher.jenkins@xxxxxxxxx> wrote: >*expands thread > >Sorry, I see this has already been addressed. > > >On 24/04/17 13:06, Alan Jenkins wrote: >> On 20/04/17 15:38, Guido Trentalancia wrote: >>> Remove semanage read and transaction lock files upon releasing >>> them. >> >> What prevents this sequence? >> >> A release lock >> B acquire lock >> A unlink lock file >> C create lock file >> C acquire lock >> >>> Signed-off-by: Guido Trentalancia >>> <guido-D1bseh+SzQhuxeB9wqlrNw@xxxxxxxxxxxxxxxx> >>> --- >>> src/semanage_store.c | 2 ++ >>> 1 file changed, 2 insertions(+) >>> >>> diff -pruN a/src/semanage_store.c b/src/semanage_store.c >>> --- a/src/semanage_store.c 2016-10-14 17:31:26.000000000 +0200 >>> +++ b/src/semanage_store.c 2017-04-03 09:32:24.093627962 +0200 >>> @@ -1904,6 +1904,7 @@ void semanage_release_trans_lock(semanag >>> close(sh->u.direct.translock_file_fd); >>> sh->u.direct.translock_file_fd = -1; >>> } >>> + unlink(semanage_files[SEMANAGE_TRANS_LOCK]); >>> errno = errsv; >>> } >>> @@ -1917,6 +1918,7 @@ void semanage_release_active_lock(semana >>> close(sh->u.direct.activelock_file_fd); >>> sh->u.direct.activelock_file_fd = -1; >>> } >>> + unlink(semanage_files[SEMANAGE_READ_LOCK]); >>> errno = errsv; >>> } >> >> >>
