Re: [PATCH iptables] iptables: use IPC semaphore instead of abstract unix sockets

[Pablo Neira Ayuso <pablo@netfilter.org>]

On Mon, Jan 19, 2015 at 01:08:33PM +0000, Patrick McHardy wrote:
> On 19.01, Pablo Neira Ayuso wrote:
> > 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:
> > > > > "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.
> 
> Sure, but that risk also exists with using the path.
> 
> > 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.
> 
> I think your patch is fine, just wanted to point out that we might
> want to choose a hardcoded name. I think the risk of clashes with
> other applications is absolutely minimal.

Makes sense to me. This is how ftok() calculates the key based on the
path:

  key = ((st.st_ino & 0xffff) | ((st.st_dev & 0xff) << 16)
         | ((proj_id & 0xff) << 24));

We can a select a magic number for that key, any candidates?

Thanks!