[PATCH V2 for-6.10/block 1/2] loop: Fix a race between loop detach and loop open

[PATCH V2 for-6.10/block 1/2] loop: Fix a race between loop detach and loop open

[Gulam Mohamed]

Description
===========

1. Userspace sends the command "losetup -d" which uses the open() call
   to open the device
2. Kernel receives the ioctl command "LOOP_CLR_FD" which calls the
   function loop_clr_fd()
3. If LOOP_CLR_FD is the first command received at the time, then the
   AUTOCLEAR flag is not set and deletion of the
   loop device proceeds ahead and scans the partitions (drop/add
   partitions)

	if (disk_openers(lo->lo_disk) > 1) {
		lo->lo_flags |= LO_FLAGS_AUTOCLEAR;
		loop_global_unlock(lo, true);
		return 0;
	}

 4. Before scanning partitions, it will check to see if any partition of
    the loop device is currently opened
 5. If any partition is opened, then it will return EBUSY:

    if (disk->open_partitions)
		return -EBUSY;
 6. So, after receiving the "LOOP_CLR_FD" command and just before the above
    check for open_partitions, if any other command
    (like blkid) opens any partition of the loop device, then the partition
    scan will not proceed and EBUSY is returned as shown in above code
 7. But in "__loop_clr_fd()", this EBUSY error is not propagated
 8. We have noticed that this is causing the partitions of the loop to
    remain stale even after the loop device is detached resulting in the
    IO errors on the partitions

Fix
---
Re-introduce the lo_open() call to restrict any process to open the loop
device when its being detached

Test case
=========
Test case involves the following two scripts:

script1.sh
----------
while [ 1 ];
do
	losetup -P -f /home/opt/looptest/test10.img
	blkid /dev/loop0p1
done

script2.sh
----------
while [ 1 ];
do
	losetup -d /dev/loop0
done

Without fix, the following IO errors have been observed:

kernel: __loop_clr_fd: partition scan of loop0 failed (rc=-16)
kernel: I/O error, dev loop0, sector 20971392 op 0x0:(READ) flags 0x80700
        phys_seg 1 prio class 0
kernel: I/O error, dev loop0, sector 108868 op 0x0:(READ) flags 0x0
        phys_seg 1 prio class 0
kernel: Buffer I/O error on dev loop0p1, logical block 27201, async page
        read

V1->V2:
	Added a test case, 010, in blktests in tests/loop/
Signed-off-by: Gulam Mohamed <gulam.mohamed@oracle.com>
---
 drivers/block/loop.c | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/drivers/block/loop.c b/drivers/block/loop.c
index 28a95fd366fe..9a235d8c062d 100644
--- a/drivers/block/loop.c
+++ b/drivers/block/loop.c
@@ -1717,6 +1717,24 @@ static int lo_compat_ioctl(struct block_device *bdev, blk_mode_t mode,
 }
 #endif
 
+static int lo_open(struct gendisk *disk, blk_mode_t mode)
+{
+        struct loop_device *lo = disk->private_data;
+        int err;
+
+        if (!lo)
+                return -ENXIO;
+
+        err = mutex_lock_killable(&lo->lo_mutex);
+        if (err)
+                return err;
+
+        if (lo->lo_state == Lo_rundown)
+                err = -ENXIO;
+        mutex_unlock(&lo->lo_mutex);
+	return err;
+}
+
 static void lo_release(struct gendisk *disk)
 {
 	struct loop_device *lo = disk->private_data;
@@ -1752,6 +1770,7 @@ static void lo_free_disk(struct gendisk *disk)
 
 static const struct block_device_operations lo_fops = {
 	.owner =	THIS_MODULE,
+	.open = 	lo_open,
 	.release =	lo_release,
 	.ioctl =	lo_ioctl,
 #ifdef CONFIG_COMPAT
-- 
2.39.3

[PATCH 2/2] loop: Test to detect a race condition between loop detach and open

[Gulam Mohamed]

When one process opens a loop device partition and another process detaches
it, there will be a race condition due to which stale loop partitions are
created causing IO errors. This test will detect the race

Signed-off-by: Gulam Mohamed <gulam.mohamed@oracle.com>
---
 tests/loop/010     | 90 ++++++++++++++++++++++++++++++++++++++++++++++
 tests/loop/010.out |  2 ++
 2 files changed, 92 insertions(+)
 create mode 100755 tests/loop/010
 create mode 100644 tests/loop/010.out

diff --git a/tests/loop/010 b/tests/loop/010
new file mode 100755
index 000000000000..ea93a120d51a
--- /dev/null
+++ b/tests/loop/010
@@ -0,0 +1,90 @@
+#!/bin/bash
+# SPDX-License-Identifier: GPL-3.0+
+# Copyright (C) 2024, Oracle and/or its affiliates.
+#
+# Test to detect a race between loop detach and loop open which creates
+# stale loop partitions when one process opens the loop partition and
+# another process detaches the loop device
+#
+. tests/loop/rc
+DESCRIPTION="check stale loop partition"
+TIMED=1
+
+requires() {
+        _have_program parted
+        _have_program mkfs.xfs
+}
+
+image_file="/tmp/loopImg"
+line1="partition scan of loop0 failed (rc=-16)"
+
+function create_loop()
+{
+        while [ 1 ]
+        do
+                loop_device="$(losetup -P -f --show "${image_file}")"
+                blkid /dev/loop0p1 >> /dev/null 2>&1
+        done
+}
+
+function detach_loop()
+{
+        while [ 1 ]
+        do
+                if [ -e /dev/loop0 ]; then
+                        losetup -d /dev/loop0 > /dev/null 2>&1
+                fi
+        done
+}
+
+test() {
+	echo "Running ${TEST_NAME}"
+	local failure="/tmp/failure"
+	touch $failure
+	echo 0 > $failure
+	dmesg -c > /dev/null 2>&1
+
+	truncate -s 2G "${image_file}"
+	parted -a none -s "${image_file}" mklabel gpt
+	loop_device="$(losetup -P -f --show "${image_file}")"
+	parted -a none -s "${loop_device}" mkpart primary 64s 109051s
+	
+	udevadm settle
+        if [ ! -e "${loop_device}" ]; then
+		return 1
+        fi
+
+        mkfs.xfs -f "${loop_device}p1" > /dev/null 2>&1
+
+        losetup -d "${loop_device}" >  /dev/null 2>&1
+
+        create_loop &
+	create_pid=$!
+        detach_loop &
+	detach_pid=$!
+
+	sleep "${TIMEOUT:-180}"
+        {
+                kill -9 $create_pid
+		kill -9 $detach_pid
+                wait
+                sleep 1
+        } 2>/dev/null
+
+        losetup -D > /dev/null 2>&1
+        dmesg  | while IFS= read -r line2
+	do
+        	match=$(echo "$line2" | grep -o "$line1")
+                if [ "$line1" == "$match" ]; then
+			echo 1 > "/tmp/failure"
+			break
+                fi
+        done
+	failed=$(cat $failure)
+	if [ $failed -eq 0 ]; then
+		echo "Test complete"
+	else
+		echo "Test failed"
+	fi
+	rm -f $failure
+}
diff --git a/tests/loop/010.out b/tests/loop/010.out
new file mode 100644
index 000000000000..64a6aee00b8a
--- /dev/null
+++ b/tests/loop/010.out
@@ -0,0 +1,2 @@
+Running loop/010
+Test complete
-- 
2.39.3

Re: [PATCH 2/2] loop: Test to detect a race condition between loop detach and open

[Christoph Hellwig]

On Tue, May 21, 2024 at 10:42:49PM +0000, Gulam Mohamed wrote:
> When one process opens a loop device partition and another process detaches
> it, there will be a race condition due to which stale loop partitions are
> created causing IO errors. This test will detect the race

This isn't really a 2/2, but a single patch for a separate repository.

Re: [PATCH 2/2] loop: Test to detect a race condition between loop detach and open

[Shinichiro Kawasaki]

On May 21, 2024 / 22:42, Gulam Mohamed wrote:
> When one process opens a loop device partition and another process detaches
> it, there will be a race condition due to which stale loop partitions are
> created causing IO errors. This test will detect the race

Hello Gulam, thanks for the patch. I ran the new test case on my system and
observed it failed. It looks working as expected.

This is the patch not for Linux kernel but for blktests, then it's the better to
separate it out. I suggest to add the subject title prefix "[PATCH blktests]"
to this patch so that reviewers can tell the target repository easily.

I suggest to take a look in the "new" script. It has some guides for blktests,
such as "Indent with tabs" or the global variable "TMPDIR". Also, please run
"make check" which runs shellcheck.

  $ make check
  shellcheck -x -e SC2119 -f gcc check common/* \
          tests/*/rc tests/*/[0-9]*[0-9] src/*.sh
  tests/loop/010:23:17: note: Instead of '[ 1 ]', use 'true'. [SC2161]
  tests/loop/010:32:17: note: Instead of '[ 1 ]', use 'true'. [SC2161]
  tests/loop/010:84:7: note: Double quote to prevent globbing and word splitting. [SC2086]
  make: *** [Makefile:21: check] Error 1

Please address the shellcheck warnings above. It will help us to keep script
quality at some level.

> 
> Signed-off-by: Gulam Mohamed <gulam.mohamed@oracle.com>
> ---
>  tests/loop/010     | 90 ++++++++++++++++++++++++++++++++++++++++++++++
>  tests/loop/010.out |  2 ++
>  2 files changed, 92 insertions(+)
>  create mode 100755 tests/loop/010
>  create mode 100644 tests/loop/010.out
> 
> diff --git a/tests/loop/010 b/tests/loop/010
> new file mode 100755
> index 000000000000..ea93a120d51a
> --- /dev/null
> +++ b/tests/loop/010
> @@ -0,0 +1,90 @@
> +#!/bin/bash
> +# SPDX-License-Identifier: GPL-3.0+
> +# Copyright (C) 2024, Oracle and/or its affiliates.
> +#
> +# Test to detect a race between loop detach and loop open which creates
> +# stale loop partitions when one process opens the loop partition and
> +# another process detaches the loop device
> +#
> +. tests/loop/rc
> +DESCRIPTION="check stale loop partition"
> +TIMED=1
> +
> +requires() {
> +        _have_program parted
> +        _have_program mkfs.xfs
> +}
> +
> +image_file="/tmp/loopImg"

$TMPDIR is recommended instead of /tmp. $TMPDIR is cleaned up after test run.

> +line1="partition scan of loop0 failed (rc=-16)"
> +
> +function create_loop()

Nit: the "function" keyword is not used in blktests (there is one exception
though).

> +{
> +        while [ 1 ]
> +        do
> +                loop_device="$(losetup -P -f --show "${image_file}")"
> +                blkid /dev/loop0p1 >> /dev/null 2>&1
> +        done
> +}
> +
> +function detach_loop()
> +{
> +        while [ 1 ]
> +        do
> +                if [ -e /dev/loop0 ]; then
> +                        losetup -d /dev/loop0 > /dev/null 2>&1
> +                fi
> +        done
> +}
> +
> +test() {
> +	echo "Running ${TEST_NAME}"
> +	local failure="/tmp/failure"

Nit: some local variables are declared with "local" keyword, and some other are
not.

> +	touch $failure
> +	echo 0 > $failure

Why the file is required to keep the fail/pass status? Just a bash variable
looks enough.

> +	dmesg -c > /dev/null 2>&1

This kernel ring buffer clear will likely affect test harnesses which run
blktests. To check the kernel message generated during each test case run,
please use _dmesg_since_test_start(): ref nbd/004, nvme/003.

> +
> +	truncate -s 2G "${image_file}"

Other test cases in loop group have 1G file size. Is 2G required for this test?
If not, 1G size would be the better to reduce disk shortage risk.

> +	parted -a none -s "${image_file}" mklabel gpt
> +	loop_device="$(losetup -P -f --show "${image_file}")"
> +	parted -a none -s "${loop_device}" mkpart primary 64s 109051s
> +	
> +	udevadm settle
> +        if [ ! -e "${loop_device}" ]; then
> +		return 1
> +        fi
> +
> +        mkfs.xfs -f "${loop_device}p1" > /dev/null 2>&1
> +
> +        losetup -d "${loop_device}" >  /dev/null 2>&1
> +
> +        create_loop &
> +	create_pid=$!
> +        detach_loop &
> +	detach_pid=$!
> +
> +	sleep "${TIMEOUT:-180}"

The default 180 seconds sounds a bit long. How about 30 seconds?

> +        {
> +                kill -9 $create_pid
> +		kill -9 $detach_pid
> +                wait
> +                sleep 1
> +        } 2>/dev/null
> +
> +        losetup -D > /dev/null 2>&1
> +        dmesg  | while IFS= read -r line2
> +	do
> +        	match=$(echo "$line2" | grep -o "$line1")
> +                if [ "$line1" == "$match" ]; then
> +			echo 1 > "/tmp/failure"
> +			break
> +                fi
> +        done
> +	failed=$(cat $failure)
> +	if [ $failed -eq 0 ]; then
> +		echo "Test complete"
> +	else
> +		echo "Test failed"
> +	fi
> +	rm -f $failure
> +}
> diff --git a/tests/loop/010.out b/tests/loop/010.out
> new file mode 100644
> index 000000000000..64a6aee00b8a
> --- /dev/null
> +++ b/tests/loop/010.out
> @@ -0,0 +1,2 @@
> +Running loop/010
> +Test complete
> -- 
> 2.39.3
> 

RE: [PATCH blktests] loop: Test to detect a race condition between loop detach and open

[Gulam Mohamed]

Hi,

> -----Original Message-----
> From: Shinichiro Kawasaki <shinichiro.kawasaki@wdc.com>
> Sent: Thursday, May 23, 2024 4:45 PM
> To: Gulam Mohamed <gulam.mohamed@oracle.com>
> Cc: linux-block@vger.kernel.org; linux-kernel@vger.kernel.org;
> axboe@kernel.dk; chaitanyak@nvidia.com; hch@lst.de
> Subject: Re: [PATCH 2/2] loop: Test to detect a race condition between loop
> detach and open
> 
> On May 21, 2024 / 22:42, Gulam Mohamed wrote:
> > When one process opens a loop device partition and another process
> > detaches it, there will be a race condition due to which stale loop
> > partitions are created causing IO errors. This test will detect the
> > race
> 
> Hello Gulam, thanks for the patch. I ran the new test case on my system and
> observed it failed. It looks working as expected.
> 
> This is the patch not for Linux kernel but for blktests, then it's the better to
> separate it out. I suggest to add the subject title prefix "[PATCH blktests]"
> to this patch so that reviewers can tell the target repository easily.
> 
Done. Changed the subject line prefix to "[PATCH blktests]"
 
> I suggest to take a look in the "new" script. It has some guides for blktests,
> such as "Indent with tabs" or the global variable "TMPDIR". Also, please run
> "make check" which runs shellcheck.
> 
>   $ make check
>   shellcheck -x -e SC2119 -f gcc check common/* \
>           tests/*/rc tests/*/[0-9]*[0-9] src/*.sh
>   tests/loop/010:23:17: note: Instead of '[ 1 ]', use 'true'. [SC2161]
>   tests/loop/010:32:17: note: Instead of '[ 1 ]', use 'true'. [SC2161]
>   tests/loop/010:84:7: note: Double quote to prevent globbing and word
> splitting. [SC2086]
>   make: *** [Makefile:21: check] Error 1
> 
> Please address the shellcheck warnings above. It will help us to keep script
> quality at some level.
Done. Resolved the warnings
> 
> >
> > Signed-off-by: Gulam Mohamed <gulam.mohamed@oracle.com>
> > ---
> >  tests/loop/010     | 90
> ++++++++++++++++++++++++++++++++++++++++++++++
> >  tests/loop/010.out |  2 ++
> >  2 files changed, 92 insertions(+)
> >  create mode 100755 tests/loop/010
> >  create mode 100644 tests/loop/010.out
> >
> > diff --git a/tests/loop/010 b/tests/loop/010 new file mode 100755
> > index 000000000000..ea93a120d51a
> > --- /dev/null
> > +++ b/tests/loop/010
> > @@ -0,0 +1,90 @@
> > +#!/bin/bash
> > +# SPDX-License-Identifier: GPL-3.0+
> > +# Copyright (C) 2024, Oracle and/or its affiliates.
> > +#
> > +# Test to detect a race between loop detach and loop open which
> > +creates # stale loop partitions when one process opens the loop
> > +partition and # another process detaches the loop device # .
> > +tests/loop/rc DESCRIPTION="check stale loop partition"
> > +TIMED=1
> > +
> > +requires() {
> > +        _have_program parted
> > +        _have_program mkfs.xfs
> > +}
> > +
> > +image_file="/tmp/loopImg"
> 
> $TMPDIR is recommended instead of /tmp. $TMPDIR is cleaned up after test
> run.
Done. Using $TMPDIR now.
> 
> > +line1="partition scan of loop0 failed (rc=-16)"
> > +
> > +function create_loop()
> 
> Nit: the "function" keyword is not used in blktests (there is one exception
> though).
> 
Done. Removed the keyword "function" now
> > +{
> > +        while [ 1 ]
> > +        do
> > +                loop_device="$(losetup -P -f --show "${image_file}")"
> > +                blkid /dev/loop0p1 >> /dev/null 2>&1
> > +        done
> > +}
> > +
> > +function detach_loop()
> > +{
> > +        while [ 1 ]
> > +        do
> > +                if [ -e /dev/loop0 ]; then
> > +                        losetup -d /dev/loop0 > /dev/null 2>&1
> > +                fi
> > +        done
> > +}
> > +
> > +test() {
> > +	echo "Running ${TEST_NAME}"
> > +	local failure="/tmp/failure"
> 
> Nit: some local variables are declared with "local" keyword, and some other
> are not.
Done. Using the "local" keyword for all local variables now
> 
> > +	touch $failure
> > +	echo 0 > $failure
> 
> Why the file is required to keep the fail/pass status? Just a bash variable looks
> enough.
In the beginning I tried with a bash variable but it was not working as the variable need to be accessed by other two child threads also.
> 
> > +	dmesg -c > /dev/null 2>&1
> 
> This kernel ring buffer clear will likely affect test harnesses which run blktests.
> To check the kernel message generated during each test case run, please use
> _dmesg_since_test_start(): ref nbd/004, nvme/003.
Done. Used the above function
> 
> > +
> > +	truncate -s 2G "${image_file}"
> 
> Other test cases in loop group have 1G file size. Is 2G required for this test?
> If not, 1G size would be the better to reduce disk shortage risk.
> 
Using 1G is not an issue. Done
> > +	parted -a none -s "${image_file}" mklabel gpt
> > +	loop_device="$(losetup -P -f --show "${image_file}")"
> > +	parted -a none -s "${loop_device}" mkpart primary 64s 109051s
> > +
> > +	udevadm settle
> > +        if [ ! -e "${loop_device}" ]; then
> > +		return 1
> > +        fi
> > +
> > +        mkfs.xfs -f "${loop_device}p1" > /dev/null 2>&1
> > +
> > +        losetup -d "${loop_device}" >  /dev/null 2>&1
> > +
> > +        create_loop &
> > +	create_pid=$!
> > +        detach_loop &
> > +	detach_pid=$!
> > +
> > +	sleep "${TIMEOUT:-180}"
> 
> The default 180 seconds sounds a bit long. How about 30 seconds?
Using 180 seconds is just to give more time to reproduce the issue as 30 seconds may not be sufficient for the race condition to occur

Will send the version 2 as soon as possible, with all these changes
> 
> > +        {
> > +                kill -9 $create_pid
> > +		kill -9 $detach_pid
> > +                wait
> > +                sleep 1
> > +        } 2>/dev/null
> > +
> > +        losetup -D > /dev/null 2>&1
> > +        dmesg  | while IFS= read -r line2
> > +	do
> > +        	match=$(echo "$line2" | grep -o "$line1")
> > +                if [ "$line1" == "$match" ]; then
> > +			echo 1 > "/tmp/failure"
> > +			break
> > +                fi
> > +        done
> > +	failed=$(cat $failure)
> > +	if [ $failed -eq 0 ]; then
> > +		echo "Test complete"
> > +	else
> > +		echo "Test failed"
> > +	fi
> > +	rm -f $failure
> > +}
> > diff --git a/tests/loop/010.out b/tests/loop/010.out new file mode
> > 100644 index 000000000000..64a6aee00b8a
> > --- /dev/null
> > +++ b/tests/loop/010.out
> > @@ -0,0 +1,2 @@
> > +Running loop/010
> > +Test complete
> > --
> > 2.39.3
> >

Re: [PATCH V2 for-6.10/block 1/2] loop: Fix a race between loop detach and loop open

[Jens Axboe]

On 5/21/24 4:42 PM, Gulam Mohamed wrote:
> Description
> ===========
> 
> 1. Userspace sends the command "losetup -d" which uses the open() call
>    to open the device
> 2. Kernel receives the ioctl command "LOOP_CLR_FD" which calls the
>    function loop_clr_fd()
> 3. If LOOP_CLR_FD is the first command received at the time, then the
>    AUTOCLEAR flag is not set and deletion of the
>    loop device proceeds ahead and scans the partitions (drop/add
>    partitions)
> 
> 	if (disk_openers(lo->lo_disk) > 1) {
> 		lo->lo_flags |= LO_FLAGS_AUTOCLEAR;
> 		loop_global_unlock(lo, true);
> 		return 0;
> 	}
> 
>  4. Before scanning partitions, it will check to see if any partition of
>     the loop device is currently opened
>  5. If any partition is opened, then it will return EBUSY:
> 
>     if (disk->open_partitions)
> 		return -EBUSY;
>  6. So, after receiving the "LOOP_CLR_FD" command and just before the above
>     check for open_partitions, if any other command
>     (like blkid) opens any partition of the loop device, then the partition
>     scan will not proceed and EBUSY is returned as shown in above code
>  7. But in "__loop_clr_fd()", this EBUSY error is not propagated
>  8. We have noticed that this is causing the partitions of the loop to
>     remain stale even after the loop device is detached resulting in the
>     IO errors on the partitions
> 
> Fix
> ---
> Re-introduce the lo_open() call to restrict any process to open the loop
> device when its being detached
> 
> Test case
> =========
> Test case involves the following two scripts:
> 
> script1.sh
> ----------
> while [ 1 ];
> do
> 	losetup -P -f /home/opt/looptest/test10.img
> 	blkid /dev/loop0p1
> done
> 
> script2.sh
> ----------
> while [ 1 ];
> do
> 	losetup -d /dev/loop0
> done
> 
> Without fix, the following IO errors have been observed:
> 
> kernel: __loop_clr_fd: partition scan of loop0 failed (rc=-16)
> kernel: I/O error, dev loop0, sector 20971392 op 0x0:(READ) flags 0x80700
>         phys_seg 1 prio class 0
> kernel: I/O error, dev loop0, sector 108868 op 0x0:(READ) flags 0x0
>         phys_seg 1 prio class 0
> kernel: Buffer I/O error on dev loop0p1, logical block 27201, async page
>         read
> 
> V1->V2:
> 	Added a test case, 010, in blktests in tests/loop/
> Signed-off-by: Gulam Mohamed <gulam.mohamed@oracle.com>
> ---
>  drivers/block/loop.c | 19 +++++++++++++++++++
>  1 file changed, 19 insertions(+)
> 
> diff --git a/drivers/block/loop.c b/drivers/block/loop.c
> index 28a95fd366fe..9a235d8c062d 100644
> --- a/drivers/block/loop.c
> +++ b/drivers/block/loop.c
> @@ -1717,6 +1717,24 @@ static int lo_compat_ioctl(struct block_device *bdev, blk_mode_t mode,
>  }
>  #endif
>  
> +static int lo_open(struct gendisk *disk, blk_mode_t mode)
> +{
> +        struct loop_device *lo = disk->private_data;
> +        int err;
> +
> +        if (!lo)
> +                return -ENXIO;
> +
> +        err = mutex_lock_killable(&lo->lo_mutex);
> +        if (err)
> +                return err;
> +
> +        if (lo->lo_state == Lo_rundown)
> +                err = -ENXIO;
> +        mutex_unlock(&lo->lo_mutex);
> +	return err;
> +}

Most of this function uses spaces rather than tabs.

-- 
Jens Axboe

Re: [PATCH V2 for-6.10/block 1/2] loop: Fix a race between loop detach and loop open

[Yu Kuai]

Hi,

在 2024/05/22 9:39, Jens Axboe 写道:
> On 5/21/24 4:42 PM, Gulam Mohamed wrote:
>> Description
>> ===========
>>
>> 1. Userspace sends the command "losetup -d" which uses the open() call
>>     to open the device
>> 2. Kernel receives the ioctl command "LOOP_CLR_FD" which calls the
>>     function loop_clr_fd()
>> 3. If LOOP_CLR_FD is the first command received at the time, then the
>>     AUTOCLEAR flag is not set and deletion of the
>>     loop device proceeds ahead and scans the partitions (drop/add
>>     partitions)
>>
>> 	if (disk_openers(lo->lo_disk) > 1) {
>> 		lo->lo_flags |= LO_FLAGS_AUTOCLEAR;
>> 		loop_global_unlock(lo, true);
>> 		return 0;
>> 	}
>>
>>   4. Before scanning partitions, it will check to see if any partition of
>>      the loop device is currently opened
>>   5. If any partition is opened, then it will return EBUSY:
>>
>>      if (disk->open_partitions)
>> 		return -EBUSY;
>>   6. So, after receiving the "LOOP_CLR_FD" command and just before the above
>>      check for open_partitions, if any other command
>>      (like blkid) opens any partition of the loop device, then the partition
>>      scan will not proceed and EBUSY is returned as shown in above code
>>   7. But in "__loop_clr_fd()", this EBUSY error is not propagated
>>   8. We have noticed that this is causing the partitions of the loop to
>>      remain stale even after the loop device is detached resulting in the
>>      IO errors on the partitions
>>
>> Fix
>> ---
>> Re-introduce the lo_open() call to restrict any process to open the loop
>> device when its being detached
>>
>> Test case
>> =========
>> Test case involves the following two scripts:
>>
>> script1.sh
>> ----------
>> while [ 1 ];
>> do
>> 	losetup -P -f /home/opt/looptest/test10.img
>> 	blkid /dev/loop0p1
>> done
>>
>> script2.sh
>> ----------
>> while [ 1 ];
>> do
>> 	losetup -d /dev/loop0
>> done
>>
>> Without fix, the following IO errors have been observed:
>>
>> kernel: __loop_clr_fd: partition scan of loop0 failed (rc=-16)
>> kernel: I/O error, dev loop0, sector 20971392 op 0x0:(READ) flags 0x80700
>>          phys_seg 1 prio class 0
>> kernel: I/O error, dev loop0, sector 108868 op 0x0:(READ) flags 0x0
>>          phys_seg 1 prio class 0
>> kernel: Buffer I/O error on dev loop0p1, logical block 27201, async page
>>          read
>>
>> V1->V2:
>> 	Added a test case, 010, in blktests in tests/loop/
>> Signed-off-by: Gulam Mohamed <gulam.mohamed@oracle.com>
>> ---
>>   drivers/block/loop.c | 19 +++++++++++++++++++
>>   1 file changed, 19 insertions(+)
>>
>> diff --git a/drivers/block/loop.c b/drivers/block/loop.c
>> index 28a95fd366fe..9a235d8c062d 100644
>> --- a/drivers/block/loop.c
>> +++ b/drivers/block/loop.c
>> @@ -1717,6 +1717,24 @@ static int lo_compat_ioctl(struct block_device *bdev, blk_mode_t mode,
>>   }
>>   #endif
>>   
>> +static int lo_open(struct gendisk *disk, blk_mode_t mode)
>> +{
>> +        struct loop_device *lo = disk->private_data;
>> +        int err;
>> +
>> +        if (!lo)
>> +                return -ENXIO;
>> +
>> +        err = mutex_lock_killable(&lo->lo_mutex);
>> +        if (err)
>> +                return err;
>> +
>> +        if (lo->lo_state == Lo_rundown)
>> +                err = -ENXIO;
>> +        mutex_unlock(&lo->lo_mutex);

This doesn't fix the problem completely, there is still a race window.

lo_release
  if (disk_openers(disk) > 0)
   reutrn
   -> no openers now
		lo_open
		 if (lo->lo_state == Lo_rundown)
		 -> no set yet
		 open succeed
  mutex_lock(lo_mutex)
  lo->lo_state = Lo_rundown
  mutex_unlock(lo_mutex)
  __loop_clr_fd

And with the respect, loop_clr_fd() has the same problem.

I think probably loop need a open counter for itself.

Thanks,
Kuai

>> +	return err;
>> +}
> 
> Most of this function uses spaces rather than tabs.
> 

RE: [PATCH V2 for-6.10/block 1/2] loop: Fix a race between loop detach and loop open

[Gulam Mohamed]

Hi Kuai,

> -----Original Message-----
> From: Yu Kuai <yukuai1@huaweicloud.com>
> Sent: Wednesday, May 22, 2024 8:01 AM
> To: Jens Axboe <axboe@kernel.dk>; Gulam Mohamed
> <gulam.mohamed@oracle.com>; linux-block@vger.kernel.org; linux-
> kernel@vger.kernel.org
> Cc: shinichiro.kawasaki@wdc.com; chaitanyak@nvidia.com; hch@lst.de;
> yukuai (C) <yukuai3@huawei.com>
> Subject: Re: [PATCH V2 for-6.10/block 1/2] loop: Fix a race between loop
> detach and loop open
> 
> Hi,
> 
> 在 2024/05/22 9:39, Jens Axboe 写道:
> > On 5/21/24 4:42 PM, Gulam Mohamed wrote:
> >> Description
> >> ===========
> >>
> >> 1. Userspace sends the command "losetup -d" which uses the open() call
> >>     to open the device
> >> 2. Kernel receives the ioctl command "LOOP_CLR_FD" which calls the
> >>     function loop_clr_fd()
> >> 3. If LOOP_CLR_FD is the first command received at the time, then the
> >>     AUTOCLEAR flag is not set and deletion of the
> >>     loop device proceeds ahead and scans the partitions (drop/add
> >>     partitions)
> >>
> >> 	if (disk_openers(lo->lo_disk) > 1) {
> >> 		lo->lo_flags |= LO_FLAGS_AUTOCLEAR;
> >> 		loop_global_unlock(lo, true);
> >> 		return 0;
> >> 	}
> >>
> >>   4. Before scanning partitions, it will check to see if any partition of
> >>      the loop device is currently opened
> >>   5. If any partition is opened, then it will return EBUSY:
> >>
> >>      if (disk->open_partitions)
> >> 		return -EBUSY;
> >>   6. So, after receiving the "LOOP_CLR_FD" command and just before the
> above
> >>      check for open_partitions, if any other command
> >>      (like blkid) opens any partition of the loop device, then the partition
> >>      scan will not proceed and EBUSY is returned as shown in above code
> >>   7. But in "__loop_clr_fd()", this EBUSY error is not propagated
> >>   8. We have noticed that this is causing the partitions of the loop to
> >>      remain stale even after the loop device is detached resulting in the
> >>      IO errors on the partitions
> >>
> >> Fix
> >> ---
> >> Re-introduce the lo_open() call to restrict any process to open the
> >> loop device when its being detached
> >>
> >> Test case
> >> =========
> >> Test case involves the following two scripts:
> >>
> >> script1.sh
> >> ----------
> >> while [ 1 ];
> >> do
> >> 	losetup -P -f /home/opt/looptest/test10.img
> >> 	blkid /dev/loop0p1
> >> done
> >>
> >> script2.sh
> >> ----------
> >> while [ 1 ];
> >> do
> >> 	losetup -d /dev/loop0
> >> done
> >>
> >> Without fix, the following IO errors have been observed:
> >>
> >> kernel: __loop_clr_fd: partition scan of loop0 failed (rc=-16)
> >> kernel: I/O error, dev loop0, sector 20971392 op 0x0:(READ) flags 0x80700
> >>          phys_seg 1 prio class 0
> >> kernel: I/O error, dev loop0, sector 108868 op 0x0:(READ) flags 0x0
> >>          phys_seg 1 prio class 0
> >> kernel: Buffer I/O error on dev loop0p1, logical block 27201, async page
> >>          read
> >>
> >> V1->V2:
> >> 	Added a test case, 010, in blktests in tests/loop/
> >> Signed-off-by: Gulam Mohamed <gulam.mohamed@oracle.com>
> >> ---
> >>   drivers/block/loop.c | 19 +++++++++++++++++++
> >>   1 file changed, 19 insertions(+)
> >>
> >> diff --git a/drivers/block/loop.c b/drivers/block/loop.c index
> >> 28a95fd366fe..9a235d8c062d 100644
> >> --- a/drivers/block/loop.c
> >> +++ b/drivers/block/loop.c
> >> @@ -1717,6 +1717,24 @@ static int lo_compat_ioctl(struct block_device
> *bdev, blk_mode_t mode,
> >>   }
> >>   #endif
> >>
> >> +static int lo_open(struct gendisk *disk, blk_mode_t mode) {
> >> +        struct loop_device *lo = disk->private_data;
> >> +        int err;
> >> +
> >> +        if (!lo)
> >> +                return -ENXIO;
> >> +
> >> +        err = mutex_lock_killable(&lo->lo_mutex);
> >> +        if (err)
> >> +                return err;
> >> +
> >> +        if (lo->lo_state == Lo_rundown)
> >> +                err = -ENXIO;
> >> +        mutex_unlock(&lo->lo_mutex);
> 
> This doesn't fix the problem completely, there is still a race window.
> 
> lo_release
>   if (disk_openers(disk) > 0)
>    reutrn
>    -> no openers now
> 		lo_open
> 		 if (lo->lo_state == Lo_rundown)
> 		 -> no set yet
> 		 open succeed
>   mutex_lock(lo_mutex)
>   lo->lo_state = Lo_rundown
>   mutex_unlock(lo_mutex)
>   __loop_clr_fd
We have noticed that, at block layer, both open() and release() are protected by gendisk->open_mutex.
So, this race may not happen. Can you please let me know if I understand correctly?
> 
> And with the respect, loop_clr_fd() has the same problem.
> 
> I think probably loop need a open counter for itself.
We are looking to see how to handle this case
> 
> Thanks,
> Kuai
> 
> >> +	return err;
> >> +}
> >
> > Most of this function uses spaces rather than tabs.
> >

Re: [PATCH V2 for-6.10/block 1/2] loop: Fix a race between loop detach and loop open

[Yu Kuai]

Hi,

在 2024/05/23 3:12, Gulam Mohamed 写道:
> Hi Kuai,
> 
>> -----Original Message-----
>> From: Yu Kuai <yukuai1@huaweicloud.com>
>> Sent: Wednesday, May 22, 2024 8:01 AM
>> To: Jens Axboe <axboe@kernel.dk>; Gulam Mohamed
>> <gulam.mohamed@oracle.com>; linux-block@vger.kernel.org; linux-
>> kernel@vger.kernel.org
>> Cc: shinichiro.kawasaki@wdc.com; chaitanyak@nvidia.com; hch@lst.de;
>> yukuai (C) <yukuai3@huawei.com>
>> Subject: Re: [PATCH V2 for-6.10/block 1/2] loop: Fix a race between loop
>> detach and loop open
>>
>> Hi,
>>
>> 在 2024/05/22 9:39, Jens Axboe 写道:
>>> On 5/21/24 4:42 PM, Gulam Mohamed wrote:
>>>> Description
>>>> ===========
>>>>
>>>> 1. Userspace sends the command "losetup -d" which uses the open() call
>>>>      to open the device
>>>> 2. Kernel receives the ioctl command "LOOP_CLR_FD" which calls the
>>>>      function loop_clr_fd()
>>>> 3. If LOOP_CLR_FD is the first command received at the time, then the
>>>>      AUTOCLEAR flag is not set and deletion of the
>>>>      loop device proceeds ahead and scans the partitions (drop/add
>>>>      partitions)
>>>>
>>>> 	if (disk_openers(lo->lo_disk) > 1) {
>>>> 		lo->lo_flags |= LO_FLAGS_AUTOCLEAR;
>>>> 		loop_global_unlock(lo, true);
>>>> 		return 0;
>>>> 	}
>>>>
>>>>    4. Before scanning partitions, it will check to see if any partition of
>>>>       the loop device is currently opened
>>>>    5. If any partition is opened, then it will return EBUSY:
>>>>
>>>>       if (disk->open_partitions)
>>>> 		return -EBUSY;
>>>>    6. So, after receiving the "LOOP_CLR_FD" command and just before the
>> above
>>>>       check for open_partitions, if any other command
>>>>       (like blkid) opens any partition of the loop device, then the partition
>>>>       scan will not proceed and EBUSY is returned as shown in above code
>>>>    7. But in "__loop_clr_fd()", this EBUSY error is not propagated
>>>>    8. We have noticed that this is causing the partitions of the loop to
>>>>       remain stale even after the loop device is detached resulting in the
>>>>       IO errors on the partitions
>>>>
>>>> Fix
>>>> ---
>>>> Re-introduce the lo_open() call to restrict any process to open the
>>>> loop device when its being detached
>>>>
>>>> Test case
>>>> =========
>>>> Test case involves the following two scripts:
>>>>
>>>> script1.sh
>>>> ----------
>>>> while [ 1 ];
>>>> do
>>>> 	losetup -P -f /home/opt/looptest/test10.img
>>>> 	blkid /dev/loop0p1
>>>> done
>>>>
>>>> script2.sh
>>>> ----------
>>>> while [ 1 ];
>>>> do
>>>> 	losetup -d /dev/loop0
>>>> done
>>>>
>>>> Without fix, the following IO errors have been observed:
>>>>
>>>> kernel: __loop_clr_fd: partition scan of loop0 failed (rc=-16)
>>>> kernel: I/O error, dev loop0, sector 20971392 op 0x0:(READ) flags 0x80700
>>>>           phys_seg 1 prio class 0
>>>> kernel: I/O error, dev loop0, sector 108868 op 0x0:(READ) flags 0x0
>>>>           phys_seg 1 prio class 0
>>>> kernel: Buffer I/O error on dev loop0p1, logical block 27201, async page
>>>>           read
>>>>
>>>> V1->V2:
>>>> 	Added a test case, 010, in blktests in tests/loop/
>>>> Signed-off-by: Gulam Mohamed <gulam.mohamed@oracle.com>
>>>> ---
>>>>    drivers/block/loop.c | 19 +++++++++++++++++++
>>>>    1 file changed, 19 insertions(+)
>>>>
>>>> diff --git a/drivers/block/loop.c b/drivers/block/loop.c index
>>>> 28a95fd366fe..9a235d8c062d 100644
>>>> --- a/drivers/block/loop.c
>>>> +++ b/drivers/block/loop.c
>>>> @@ -1717,6 +1717,24 @@ static int lo_compat_ioctl(struct block_device
>> *bdev, blk_mode_t mode,
>>>>    }
>>>>    #endif
>>>>
>>>> +static int lo_open(struct gendisk *disk, blk_mode_t mode) {
>>>> +        struct loop_device *lo = disk->private_data;
>>>> +        int err;
>>>> +
>>>> +        if (!lo)
>>>> +                return -ENXIO;
>>>> +
>>>> +        err = mutex_lock_killable(&lo->lo_mutex);
>>>> +        if (err)
>>>> +                return err;
>>>> +
>>>> +        if (lo->lo_state == Lo_rundown)
>>>> +                err = -ENXIO;
>>>> +        mutex_unlock(&lo->lo_mutex);
>>
>> This doesn't fix the problem completely, there is still a race window.
>>
>> lo_release
>>    if (disk_openers(disk) > 0)
>>     reutrn
>>     -> no openers now
>> 		lo_open
>> 		 if (lo->lo_state == Lo_rundown)
>> 		 -> no set yet
>> 		 open succeed
>>    mutex_lock(lo_mutex)
>>    lo->lo_state = Lo_rundown
>>    mutex_unlock(lo_mutex)
>>    __loop_clr_fd
> We have noticed that, at block layer, both open() and release() are protected by gendisk->open_mutex.
> So, this race may not happen. Can you please let me know if I understand correctly?

Yes, __loop_clr_fd from lo_release can't concurrent with lo_open.
>>
>> And with the respect, loop_clr_fd() has the same problem.

Did you check __loop_clr_fd from lo_ioctl?

Thanks,
Kuai

>>
>> I think probably loop need a open counter for itself.
> We are looking to see how to handle this case
>>
>> Thanks,
>> Kuai
>>
>>>> +	return err;
>>>> +}
>>>
>>> Most of this function uses spaces rather than tabs.
>>>
> 

RE: [PATCH V2 for-6.10/block 1/2] loop: Fix a race between loop detach and loop open

[Gulam Mohamed]

Hi Jens,

> -----Original Message-----
> From: Jens Axboe <axboe@kernel.dk>
> Sent: Wednesday, May 22, 2024 7:10 AM
> To: Gulam Mohamed <gulam.mohamed@oracle.com>; linux-
> block@vger.kernel.org; linux-kernel@vger.kernel.org
> Cc: shinichiro.kawasaki@wdc.com; chaitanyak@nvidia.com; hch@lst.de
> Subject: Re: [PATCH V2 for-6.10/block 1/2] loop: Fix a race between loop
> detach and loop open
> 
> On 5/21/24 4:42 PM, Gulam Mohamed wrote:
> > Description
> > ===========
> >
> > 1. Userspace sends the command "losetup -d" which uses the open() call
> >    to open the device
> > 2. Kernel receives the ioctl command "LOOP_CLR_FD" which calls the
> >    function loop_clr_fd()
> > 3. If LOOP_CLR_FD is the first command received at the time, then the
> >    AUTOCLEAR flag is not set and deletion of the
> >    loop device proceeds ahead and scans the partitions (drop/add
> >    partitions)
> >
> > 	if (disk_openers(lo->lo_disk) > 1) {
> > 		lo->lo_flags |= LO_FLAGS_AUTOCLEAR;
> > 		loop_global_unlock(lo, true);
> > 		return 0;
> > 	}
> >
> >  4. Before scanning partitions, it will check to see if any partition of
> >     the loop device is currently opened  5. If any partition is
> > opened, then it will return EBUSY:
> >
> >     if (disk->open_partitions)
> > 		return -EBUSY;
> >  6. So, after receiving the "LOOP_CLR_FD" command and just before the
> above
> >     check for open_partitions, if any other command
> >     (like blkid) opens any partition of the loop device, then the partition
> >     scan will not proceed and EBUSY is returned as shown in above code
> > 7. But in "__loop_clr_fd()", this EBUSY error is not propagated  8. We
> > have noticed that this is causing the partitions of the loop to
> >     remain stale even after the loop device is detached resulting in the
> >     IO errors on the partitions
> >
> > Fix
> > ---
> > Re-introduce the lo_open() call to restrict any process to open the
> > loop device when its being detached
> >
> > Test case
> > =========
> > Test case involves the following two scripts:
> >
> > script1.sh
> > ----------
> > while [ 1 ];
> > do
> > 	losetup -P -f /home/opt/looptest/test10.img
> > 	blkid /dev/loop0p1
> > done
> >
> > script2.sh
> > ----------
> > while [ 1 ];
> > do
> > 	losetup -d /dev/loop0
> > done
> >
> > Without fix, the following IO errors have been observed:
> >
> > kernel: __loop_clr_fd: partition scan of loop0 failed (rc=-16)
> > kernel: I/O error, dev loop0, sector 20971392 op 0x0:(READ) flags 0x80700
> >         phys_seg 1 prio class 0
> > kernel: I/O error, dev loop0, sector 108868 op 0x0:(READ) flags 0x0
> >         phys_seg 1 prio class 0
> > kernel: Buffer I/O error on dev loop0p1, logical block 27201, async page
> >         read
> >
> > V1->V2:
> > 	Added a test case, 010, in blktests in tests/loop/
> > Signed-off-by: Gulam Mohamed <gulam.mohamed@oracle.com>
> > ---
> >  drivers/block/loop.c | 19 +++++++++++++++++++
> >  1 file changed, 19 insertions(+)
> >
> > diff --git a/drivers/block/loop.c b/drivers/block/loop.c index
> > 28a95fd366fe..9a235d8c062d 100644
> > --- a/drivers/block/loop.c
> > +++ b/drivers/block/loop.c
> > @@ -1717,6 +1717,24 @@ static int lo_compat_ioctl(struct block_device
> > *bdev, blk_mode_t mode,  }  #endif
> >
> > +static int lo_open(struct gendisk *disk, blk_mode_t mode) {
> > +        struct loop_device *lo = disk->private_data;
> > +        int err;
> > +
> > +        if (!lo)
> > +                return -ENXIO;
> > +
> > +        err = mutex_lock_killable(&lo->lo_mutex);
> > +        if (err)
> > +                return err;
> > +
> > +        if (lo->lo_state == Lo_rundown)
> > +                err = -ENXIO;
> > +        mutex_unlock(&lo->lo_mutex);
> > +	return err;
> > +}
> 
> Most of this function uses spaces rather than tabs.
I am sorry I should have checked that. Will address the issue in next version
> 
> --
> Jens Axboe

Re: [PATCH V2 for-6.10/block 1/2] loop: Fix a race between loop detach and loop open

[Christoph Hellwig]

On Tue, May 21, 2024 at 10:42:48PM +0000, Gulam Mohamed wrote:
> Description
> ===========

That's a weird way to format a patch.  Between this and the odd subject
not matching patch 2 I was tricked into thinking this was just a cover
letter and patch 1 was missing for a while.  Please take a look at other
patches/commit and try to word it similarly.

> V1->V2:
> 	Added a test case, 010, in blktests in tests/loop/

These kind of patch revision changelogs belong after the --- so that they
don't go into git history.  Or even better into the cover letter, which
is missing here.

> Signed-off-by: Gulam Mohamed <gulam.mohamed@oracle.com>
> ---
>  drivers/block/loop.c | 19 +++++++++++++++++++
>  1 file changed, 19 insertions(+)
> 
> diff --git a/drivers/block/loop.c b/drivers/block/loop.c
> index 28a95fd366fe..9a235d8c062d 100644
> --- a/drivers/block/loop.c
> +++ b/drivers/block/loop.c
> @@ -1717,6 +1717,24 @@ static int lo_compat_ioctl(struct block_device *bdev, blk_mode_t mode,
>  }
>  #endif
>  
> +static int lo_open(struct gendisk *disk, blk_mode_t mode)
> +{
> +        struct loop_device *lo = disk->private_data;
> +        int err;
> +
> +        if (!lo)
> +                return -ENXIO;

->private_data is never cleared, so the NULL check here doesn't
make sense.

> +        err = mutex_lock_killable(&lo->lo_mutex);
> +        if (err)
> +                return err;
> +
> +        if (lo->lo_state == Lo_rundown)
> +                err = -ENXIO;
> +        mutex_unlock(&lo->lo_mutex);

What if we race with setting Lo_rundown and that gets set right
after we unlock here?

RE: [PATCH V2 for-6.10/block 1/2] loop: Fix a race between loop detach and loop open

[Gulam Mohamed]

Hi,

> -----Original Message-----
> From: Christoph Hellwig <hch@lst.de>
> Sent: Thursday, May 23, 2024 1:42 PM
> To: Gulam Mohamed <gulam.mohamed@oracle.com>
> Cc: linux-block@vger.kernel.org; linux-kernel@vger.kernel.org;
> axboe@kernel.dk; shinichiro.kawasaki@wdc.com; chaitanyak@nvidia.com;
> hch@lst.de
> Subject: Re: [PATCH V2 for-6.10/block 1/2] loop: Fix a race between loop
> detach and loop open
> 
> On Tue, May 21, 2024 at 10:42:48PM +0000, Gulam Mohamed wrote:
> > Description
> > ===========
> 
> That's a weird way to format a patch.  Between this and the odd subject not
> matching patch 2 I was tricked into thinking this was just a cover letter and
> patch 1 was missing for a while.  Please take a look at other patches/commit
> and try to word it similarly.
I will take care of this in the next version.
> 
> > V1->V2:
> > 	Added a test case, 010, in blktests in tests/loop/
> 
> These kind of patch revision changelogs belong after the --- so that they don't
> go into git history.  Or even better into the cover letter, which is missing here.
> 
Sure. I will take care of this in the next version.
> > Signed-off-by: Gulam Mohamed <gulam.mohamed@oracle.com>
> > ---
> >  drivers/block/loop.c | 19 +++++++++++++++++++
> >  1 file changed, 19 insertions(+)
> >
> > diff --git a/drivers/block/loop.c b/drivers/block/loop.c index
> > 28a95fd366fe..9a235d8c062d 100644
> > --- a/drivers/block/loop.c
> > +++ b/drivers/block/loop.c
> > @@ -1717,6 +1717,24 @@ static int lo_compat_ioctl(struct block_device
> > *bdev, blk_mode_t mode,  }  #endif
> >
> > +static int lo_open(struct gendisk *disk, blk_mode_t mode) {
> > +        struct loop_device *lo = disk->private_data;
> > +        int err;
> > +
> > +        if (!lo)
> > +                return -ENXIO;
> 
> ->private_data is never cleared, so the NULL check here doesn't
> make sense.
> 
> > +        err = mutex_lock_killable(&lo->lo_mutex);
> > +        if (err)
> > +                return err;
> > +
> > +        if (lo->lo_state == Lo_rundown)
> > +                err = -ENXIO;
> > +        mutex_unlock(&lo->lo_mutex);
> 
> What if we race with setting Lo_rundown and that gets set right after we
> unlock here?
Similar race was mentioned by Kuai in his comments. We think these race conditions can be resolved by bringing back the "lo->refcnt" ,
by reverting the commit a0e286b6a5b61d4da01bdf865071c4da417046d6 plus the above Lo_rundown check in lo_open.