[elbe-devel] [PATCH] elbepack: imgutils: protect against race between losetup() and udev

[Thomas Weißschuh]

On Wed, Feb 19, 2025 at 10:23:01AM +0106, John Ogness wrote:
> On 2025-02-19, Thomas Weißschuh <thomas.weissschuh at linutronix.de> wrote:
> > Udev or on of its plugins reconfigures new loop devices.
> > Partitions are rescanned, leading to the removal and readding of
> > their dev nodes.
> > Depending on the timing, ELBE tries to access the partion devnode
> > while it has been removed but not yet recreated.
> >
> > Use a file lock to synchronize the accesses between udev and ELBE.
> > It's not necessary to wait for the udev updates as devtmpfs will create
> > the device nodes on its own.
> 
> You do not need udev for the device nodes to be created, but you want to
> synchronize with udev? I do not understand the reasoning here. Either
> the device nodes exist or they do not. If udev is not involved in their
> creation, then why even care about udev?

I neither need nor want udev to be involved. Udev involves itself automatically.
It reconfigures the loop device which means the scanned partitions are removed
and then added again. This introduces the race window.
I'm not sure why it does this, but it does.

> > Reported-by: John Ogness <jogness at linutronix.de>
> > Link: https://systemd.io/BLOCK_DEVICE_LOCKING
> > Signed-off-by: Thomas Weißschuh <thomas.weissschuh at linutronix.de>
> > ---
> >  elbepack/imgutils.py | 6 +++++-
> >  1 file changed, 5 insertions(+), 1 deletion(-)
> >
> > diff --git a/elbepack/imgutils.py b/elbepack/imgutils.py
> > index 635b3e68e7a906e1acc2b5e4f145f7aadcc9dcff..b060121c4590d4235a0fbc65c97c0b0c82cd95ce 100644
> > --- a/elbepack/imgutils.py
> > +++ b/elbepack/imgutils.py
> > @@ -3,6 +3,7 @@
> >  # SPDX-FileCopyrightText: 2024 Linutronix GmbH
> >  
> >  import contextlib
> > +import fcntl
> >  import subprocess
> >  
> >  from elbepack.shellhelper import ELBE_LOGGING, do, run
> > @@ -16,7 +17,10 @@ def losetup(dev, extra_args=[]):
> >      ).stdout.decode('ascii').rstrip('\n')
> >  
> >      try:
> > -        yield loopdev
> > +        with open(loopdev) as f:
> > +            # protect against races with udev
> > +            fcntl.flock(f, fcntl.LOCK_EX)
> > +            yield loopdev
> 
> And where is the unlock?

It's implicit when the opened file is closed at the end of the contextmanager.

>From flock(2):

	Locks  created  by flock() are associated with an open file description
	(see open(2)).  This means that duplicate file descriptors (created by, for
	example, fork(2) or dup(2)) refer to the same lock, and this lock may be
	modified or released using any of these file descriptors.  Furthermore, the
	lock is released either by an explicit LOCK_UN operation on any of these
	duplicate file descriptors, or when all such file descriptors have been
	closed.