[PATCH 2 of 2] lock: include Linux pid namespace identifier in prefix

Gregory Szorc gregory.szorc at gmail.com
Mon Feb 13 19:22:59 EST 2017



> On Feb 13, 2017, at 12:55, Augie Fackler <raf at durin42.com> wrote:
> 
>> On Fri, Feb 10, 2017 at 04:50:55PM -0800, Jun Wu wrote:
>> I'd like to note that although this patch prevents repo corruption when
>> running hg inside different containers (which has different pid namespaces),
>> it does not prevent deadlock - if an hg process is SIGKILL-ed, every other
>> process will not able to take or remove the lock.
> 
> Sigh. Thanks for the fix, queued (and a fist shaken at this weird/poor
> choice from linux containers.)

Process namespaces (see clone(2) man page) are a really nifty security feature. Unfortunately they do have the side effect of invalidating assumptions made since the beginning of UNIX.

This patch is a step in the right direction. However, it's worth calling out that not all containers have a proc filesystem. There have been security vulns due to containers having access to procfs. So it is common to reduce attack surface area by not mounting it.

> 
>> 
>> I think if we do know the repo is not on NFS, and the system supports
>> flock(), flock() is way more robust and solves all kinds of pain here.
>> 
>> I hereby propose a new repo requirement "flock", once set, use flock instead
>> of the traditional lock file. It's off by default. Thoughts?
> 
> I'm...not categorically opposed to it, though it feels pretty risky. I
> know git doesn't use flock() either - presumably there's a good reason
> I don't know about that neither tool relies on it?
> 
>> 
>> Excerpts from Jun Wu's message of 2017-02-10 14:09:35 -0800:
>>> # HG changeset patch
>>> # User Jun Wu <quark at fb.com>
>>> # Date 1486763791 28800
>>> #      Fri Feb 10 13:56:31 2017 -0800
>>> # Node ID 1884652829f6e68bde6ab3e9b0e9ca1da9bed40c
>>> # Parent  f8eb762559b242ec12b199db1ce27c2930881c75
>>> # Available At https://bitbucket.org/quark-zju/hg-draft
>>> #              hg pull https://bitbucket.org/quark-zju/hg-draft  -r 1884652829f6
>>> lock: include Linux pid namespace identifier in prefix
>>> 
>>> Previously, the lock only contains a hostname as an attempt to detect pid
>>> namespace difference. However, that's not enough on modern Linux - a single
>>> hostname could have different pid namespaces.
>>> 
>>> That means if people run hg inside different PID namespaces with a same UTS
>>> namespae, the lock would be broken - an hg proccess in pid namespace A will
>>> think the lock having a "random" pid in pid namespace B is "dead" and remove
>>> it.
>>> 
>>> This patch solves the above issue by appending an PID namespace identifier of
>>> the current process to the lock prefix ("hostname"). It depends on /proc
>>> being mounted properly. But I don't think there is a better way to get pid
>>> namespace identifier reliably.
>>> 
>>> diff --git a/mercurial/lock.py b/mercurial/lock.py
>>> --- a/mercurial/lock.py
>>> +++ b/mercurial/lock.py
>>> @@ -10,4 +10,5 @@ from __future__ import absolute_import
>>> import contextlib
>>> import errno
>>> +import os
>>> import socket
>>> import time
>>> @@ -16,4 +17,5 @@ import warnings
>>> from . import (
>>>     error,
>>> +    pycompat,
>>>     util,
>>> )
>>> @@ -23,7 +25,15 @@ def _getlockprefix():
>>> 
>>>     It's useful to detect "dead" processes and remove stale locks with
>>> -    confidence. Typically it's just hostname.
>>> +    confidence. Typically it's just hostname. On modern linux, we include an
>>> +    extra Linux-specific pid namespace identifier.
>>>     """
>>> -    return socket.gethostname()
>>> +    result = socket.gethostname()
>>> +    if pycompat.sysplatform.startswith('linux'):
>>> +        try:
>>> +            result += '/%x' % os.stat('/proc/self/ns/pid').st_ino
>>> +        except OSError as ex:
>>> +            if ex.errno not in (errno.ENOENT, errno.EACCES, errno.ENOTDIR):
>>> +                raise
>>> +    return result
>>> 
>>> class lock(object):
>> _______________________________________________
>> Mercurial-devel mailing list
>> Mercurial-devel at mercurial-scm.org
>> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel at mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel


More information about the Mercurial-devel mailing list