| Message ID | 20170818130733.11254-1-gmazyland@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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
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 >
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.
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 ||
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(-)