Persistent grants feature can be used only when both backend and the frontend supports the feature. The feature was always supported by 'blkback', but commit aac8a70db24b ("xen-blkback: add a parameter for disabling of persistent grants") has introduced a parameter for disabling it runtime.

To avoid the parameter be updated while being used by 'blkback', the commit caches the parameter into 'vbd->feature_gnt_persistent' in 'xen_vbd_create()', and then check if the guest also supports the feature and finally updates the field in 'connect_ring()'.

However, 'connect_ring()' could be called before 'xen_vbd_create()', so later execution of 'xen_vbd_create()' can wrongly overwrite 'true' to 'vbd->feature_gnt_persistent'. As a result, 'blkback' could try to use 'persistent grants' feature even if the guest doesn't support the feature.

This commit fixes the issue by moving the parameter value caching to 'xen_blkif_alloc()', which allocates the 'blkif'. Because the struct embeds 'vbd' object, which will be used by 'connect_ring()' later, this should be called before 'connect_ring()' and therefore this should be the right and safe place to do the caching.

Fixes: aac8a70db24b ("xen-blkback: add a parameter for disabling of persistent grants") Cc: stable@vger.kernel.org # 5.10.x Signed-off-by: Maximilian Heyne mheyne@amazon.de Signed-off-by: SeongJae Park sj@kernel.org --- drivers/block/xen-blkback/xenbus.c | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-)

diff --git a/drivers/block/xen-blkback/xenbus.c b/drivers/block/xen-blkback/xenbus.c index 97de13b14175..16c6785d260c 100644 --- a/drivers/block/xen-blkback/xenbus.c +++ b/drivers/block/xen-blkback/xenbus.c @@ -157,6 +157,11 @@ static int xen_blkif_alloc_rings(struct xen_blkif *blkif) return 0; }

+/* Enable the persistent grants feature. */ +static bool feature_persistent = true; +module_param(feature_persistent, bool, 0644); +MODULE_PARM_DESC(feature_persistent, "Enables the persistent grants feature"); + static struct xen_blkif *xen_blkif_alloc(domid_t domid) { struct xen_blkif *blkif; @@ -181,6 +186,8 @@ static struct xen_blkif *xen_blkif_alloc(domid_t domid) __module_get(THIS_MODULE); INIT_WORK(&blkif->free_work, xen_blkif_deferred_free);

+ blkif->vbd.feature_gnt_persistent = feature_persistent; + return blkif; }

@@ -472,12 +479,6 @@ static void xen_vbd_free(struct xen_vbd *vbd) vbd->bdev = NULL; }

-/* Enable the persistent grants feature. */ -static bool feature_persistent = true; -module_param(feature_persistent, bool, 0644); -MODULE_PARM_DESC(feature_persistent, - "Enables the persistent grants feature"); - static int xen_vbd_create(struct xen_blkif *blkif, blkif_vdev_t handle, unsigned major, unsigned minor, int readonly, int cdrom) @@ -520,8 +521,6 @@ static int xen_vbd_create(struct xen_blkif *blkif, blkif_vdev_t handle, if (bdev_max_secure_erase_sectors(bdev)) vbd->discard_secure = true;

- vbd->feature_gnt_persistent = feature_persistent; - pr_debug("Successful creation of handle=%04x (dom=%u)\n", handle, blkif->domid); return 0;

In some use cases[1], the backend is created while the frontend doesn't support the persistent grants feature, but later the frontend can be changed to support the feature and reconnect. In the past, 'blkback' enabled the persistent grants feature since it unconditionally checked if frontend supports the persistent grants feature for every connect ('connect_ring()') and decided whether it should use persistent grans or not.

However, commit aac8a70db24b ("xen-blkback: add a parameter for disabling of persistent grants") has mistakenly changed the behavior. It made the frontend feature support check to not be repeated once it shown the 'feature_persistent' as 'false', or the frontend doesn't support persistent grants.

Similar behavioral change has made on 'blkfront' by commit 74a852479c68 ("xen-blkfront: add a parameter for disabling of persistent grants"). This commit changes the behavior of the parameter to make effect for every connect, so that the previous behavior of 'blkfront' can be restored.

[1] https://lore.kernel.org/xen-devel/CAJwUmVB6H3iTs-C+U=v-pwJB7-_ZRHPxHzKRJZ22x...

Fixes: 74a852479c68 ("xen-blkfront: add a parameter for disabling of persistent grants") Cc: stable@vger.kernel.org # 5.10.x Signed-off-by: SeongJae Park sj@kernel.org --- Documentation/ABI/testing/sysfs-driver-xen-blkfront | 2 +- drivers/block/xen-blkfront.c | 4 +--- 2 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/Documentation/ABI/testing/sysfs-driver-xen-blkfront b/Documentation/ABI/testing/sysfs-driver-xen-blkfront index 7f646c58832e..4d36c5a10546 100644 --- a/Documentation/ABI/testing/sysfs-driver-xen-blkfront +++ b/Documentation/ABI/testing/sysfs-driver-xen-blkfront @@ -15,5 +15,5 @@ KernelVersion: 5.10 Contact: Maximilian Heyne mheyne@amazon.de Description: Whether to enable the persistent grants feature or not. Note - that this option only takes effect on newly created frontends. + that this option only takes effect on newly connected frontends. The default is Y (enable). diff --git a/drivers/block/xen-blkfront.c b/drivers/block/xen-blkfront.c index 3646c0cae672..4e763701b372 100644 --- a/drivers/block/xen-blkfront.c +++ b/drivers/block/xen-blkfront.c @@ -1988,8 +1988,6 @@ static int blkfront_probe(struct xenbus_device *dev, info->vdevice = vdevice; info->connected = BLKIF_STATE_DISCONNECTED;

- info->feature_persistent = feature_persistent; - /* Front end dir is a number, which is used as the id. */ info->handle = simple_strtoul(strrchr(dev->nodename, '/')+1, NULL, 0); dev_set_drvdata(&dev->dev, info); @@ -2283,7 +2281,7 @@ static void blkfront_gather_backend_features(struct blkfront_info *info) if (xenbus_read_unsigned(info->xbdev->otherend, "feature-discard", 0)) blkfront_setup_discard(info);

- if (info->feature_persistent) + if (feature_persistent) info->feature_persistent = !!xenbus_read_unsigned(info->xbdev->otherend, "feature-persistent", 0);