[PATCH] loop: Fix freeze if configured block size is not supported

[PATCH] loop: Fix freeze if configured block size is not supported

[Milan Broz]

The commit f2c6df7dbf9a60e1cd9941f9fb376d4d9ad1e8dd

    loop: support 4k physical blocksize

adds support for loop block size with only specific block sizes.

If the size is not supported, the code returns -EINVAL keeping
the loop queue frozen. This causes that device could be locked
for a long time by processes trying to scan device (udev).
(And also causing subsequent LOOP_CLR_FD operations noop.)

Fix it by using goto to proper exit location with queue unfreeze.

(The same bug is for setting crypt attribute but this code is
probably no more used. Patch fixes it as well though.)

Signed-off-by: Milan Broz <gmazyland@gmail.com>
---
 drivers/block/loop.c | 24 ++++++++++++++++--------
 1 file changed, 16 insertions(+), 8 deletions(-)

diff --git a/drivers/block/loop.c b/drivers/block/loop.c
index ef8334949b42..26548e07bc31 100644
--- a/drivers/block/loop.c
+++ b/drivers/block/loop.c
@@ -1125,11 +1125,15 @@ loop_set_status(struct loop_device *lo, const struct loop_info64 *info)
 	if (info->lo_encrypt_type) {
 		unsigned int type = info->lo_encrypt_type;
 
-		if (type >= MAX_LO_CRYPT)
-			return -EINVAL;
+		if (type >= MAX_LO_CRYPT) {
+			err = -EINVAL;
+			goto exit;
+		}
 		xfer = xfer_funcs[type];
-		if (xfer == NULL)
-			return -EINVAL;
+		if (xfer == NULL) {
+			err = -EINVAL;
+			goto exit;
+		}
 	} else
 		xfer = NULL;
 
@@ -1144,10 +1148,14 @@ loop_set_status(struct loop_device *lo, const struct loop_info64 *info)
 		if (LO_INFO_BLOCKSIZE(info) != 512 &&
 		    LO_INFO_BLOCKSIZE(info) != 1024 &&
 		    LO_INFO_BLOCKSIZE(info) != 2048 &&
-		    LO_INFO_BLOCKSIZE(info) != 4096)
-			return -EINVAL;
-		if (LO_INFO_BLOCKSIZE(info) > lo->lo_blocksize)
-			return -EINVAL;
+		    LO_INFO_BLOCKSIZE(info) != 4096) {
+			err = -EINVAL;
+			goto exit;
+		}
+		if (LO_INFO_BLOCKSIZE(info) > lo->lo_blocksize) {
+			err = -EINVAL;
+			goto exit;
+		}
 	}
 
 	if (lo->lo_offset != info->lo_offset ||
-- 
2.14.1

Re: [PATCH] loop: Fix freeze if configured block size is not supported

[Omar Sandoval]

On Fri, Aug 18, 2017 at 03:07:33PM +0200, Milan Broz wrote:
> The commit f2c6df7dbf9a60e1cd9941f9fb376d4d9ad1e8dd
> 
>     loop: support 4k physical blocksize
> 
> adds support for loop block size with only specific block sizes.
> 
> If the size is not supported, the code returns -EINVAL keeping
> the loop queue frozen. This causes that device could be locked
> for a long time by processes trying to scan device (udev).
> (And also causing subsequent LOOP_CLR_FD operations noop.)
> 
> Fix it by using goto to proper exit location with queue unfreeze.
> 
> (The same bug is for setting crypt attribute but this code is
> probably no more used. Patch fixes it as well though.)
> 
> Signed-off-by: Milan Broz <gmazyland@gmail.com>

Heh, I sent the same patch [1] only hours before :) v2 is here [2] if
you want to give it a reviewed-by.

1: https://marc.info/?l=linux-block&m=150303694018659&w=2
2: https://marc.info/?l=linux-block&m=150308446732748&w=2

Re: [PATCH] loop: Fix freeze if configured block size is not supported

[Milan Broz]

On 08/21/2017 08:47 PM, Omar Sandoval wrote:
> On Fri, Aug 18, 2017 at 03:07:33PM +0200, Milan Broz wrote:
>> The commit f2c6df7dbf9a60e1cd9941f9fb376d4d9ad1e8dd
>>
>>     loop: support 4k physical blocksize
>>
>> adds support for loop block size with only specific block sizes.
>>
>> If the size is not supported, the code returns -EINVAL keeping
>> the loop queue frozen. This causes that device could be locked
>> for a long time by processes trying to scan device (udev).
>> (And also causing subsequent LOOP_CLR_FD operations noop.)
>>
>> Fix it by using goto to proper exit location with queue unfreeze.
>>
>> (The same bug is for setting crypt attribute but this code is
>> probably no more used. Patch fixes it as well though.)
>>
>> Signed-off-by: Milan Broz <gmazyland@gmail.com>
> 
> Heh, I sent the same patch [1] only hours before :) v2 is here [2] if
> you want to give it a reviewed-by.

Yes, and I noticed it 10 seconds after I sent my patch :)
You can add reviewed by, if it helps anything...

Actually you fixed another problems there with following patches
(physical blocks sizes), we discussed this with Karel
that it need some changes because the original patch caused
that reported blocks differs between old and new kernel (lsblk -t),
even if block size was not used.

I will test them as well (we have code in cryptsetup that allocates
loop device automatically if the image is in file and I would like
to add block size setting there).

Thanks,
Milan

> 
> 1: https://marc.info/?l=linux-block&m=150303694018659&w=2
> 2: https://marc.info/?l=linux-block&m=150308446732748&w=2
> 

Re: [PATCH] loop: Fix freeze if configured block size is not supported

[Omar Sandoval]

On Mon, Aug 21, 2017 at 09:11:32PM +0200, Milan Broz wrote:
> On 08/21/2017 08:47 PM, Omar Sandoval wrote:
> > On Fri, Aug 18, 2017 at 03:07:33PM +0200, Milan Broz wrote:
> >> The commit f2c6df7dbf9a60e1cd9941f9fb376d4d9ad1e8dd
> >>
> >>     loop: support 4k physical blocksize
> >>
> >> adds support for loop block size with only specific block sizes.
> >>
> >> If the size is not supported, the code returns -EINVAL keeping
> >> the loop queue frozen. This causes that device could be locked
> >> for a long time by processes trying to scan device (udev).
> >> (And also causing subsequent LOOP_CLR_FD operations noop.)
> >>
> >> Fix it by using goto to proper exit location with queue unfreeze.
> >>
> >> (The same bug is for setting crypt attribute but this code is
> >> probably no more used. Patch fixes it as well though.)
> >>
> >> Signed-off-by: Milan Broz <gmazyland@gmail.com>
> > 
> > Heh, I sent the same patch [1] only hours before :) v2 is here [2] if
> > you want to give it a reviewed-by.
> 
> Yes, and I noticed it 10 seconds after I sent my patch :)
> You can add reviewed by, if it helps anything...

Thanks!

> Actually you fixed another problems there with following patches
> (physical blocks sizes), we discussed this with Karel
> that it need some changes because the original patch caused
> that reported blocks differs between old and new kernel (lsblk -t),
> even if block size was not used.

Yes there was definitely some funkiness there, let me know if I covered
everything or if I need to make more changes.