On Mon, Jan 19, 2015 at 02:00:24PM +0100, Pablo Neira Ayuso wrote:
> On Mon, Jan 19, 2015 at 12:51:19PM +0000, Patrick McHardy wrote:
> > On 19.01, Pablo Neira Ayuso wrote:
> > > On Sun, Jan 18, 2015 at 10:28:32PM +0100, Jan Engelhardt wrote:
> > > > On Sunday 2015-01-18 22:13, Pablo Neira Ayuso wrote:
> > > >
> > > > >This patch introduces a semaphore
> > > > >index b18022e..80eed2c 100644
> > > > >--- a/iptables/xshared.c
> > > > >+++ b/iptables/xshared.c
> > > > >+static int xtables_check_owner(int semid)
> > > > >+{
> > > > >+ int ret;
> > > > >+ struct semid_ds ds;
> > > > >+
> > > > >+ ret = semctl(semid, 0, IPC_STAT, &ds);
> > > >
> > > > Is there a particular reason you are not using the POSIX semaphores?
> > > > [sem_open/shm_open as per sem_overview(7)].
> > >
> > > Please, read the patch description:
> > >
> > > "This patch introduces a semaphore that is identified by the path to
> > > the iptables binary, it also relies on SEM_UNDO so the kernel performs
> > > the up() operation at process exit to avoid races with signals. This
> > > also avoids file locks that require a writable filesystem."
> >
> > Is it wise to use the path? Not that its very common, but multiple
> > binaries would still race. Any reason you chose not to use something
> > globally unique?
>
> What kind of race are you worrying about?
Oh, I get it. Several different iptables binaries located in different
paths. This patch cannot address that situation, we can select a
hardcoded key but we may conflict with other applications.
Regarding the use of posix semaphores, there is no SEM_UNDO feature,
so we can have problem if this receives a kill signal or it
abort/crash somewhere in the code.
I think the best solution is to use to flock() as others do but then
we need a writable filesystem() which is what Phil was trying to skip.
Question is if we should really care. I mean, this locking solution
was introduced as a workaround given we couldn't solve this in the
kernel.
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
