| Message ID | 20240209215247.5213-1-michael.christie@oracle.com (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [v2,1/1] scsi: target: Fix unmap setup during configuration | expand |
pá 9. 2. 2024 v 22:52 odesílatel Mike Christie <michael.christie@oracle.com> napsal: > > This issue was found and also debugged by Carl Lei <me@xecycle.info>. > > If the device is not enabled, iblock/file will have not setup their > se_device to bdev/file mappings. If a user tries to config the unmap > settings at this time, we will then crash trying to access a NULL > pointer where the bdev/file should be. > > This patch adds a check to make sure the device is configured before > we try to call the configure_unmap callout. > > Fixes: 34bd1dcacf0d ("scsi: target: Detect UNMAP support post configuration") > Reported-by: Carl Lei <me@xecycle.info> > Signed-off-by: Mike Christie <michael.christie@oracle.com> > --- > > v2: Fix missing configure_unmap handling so failure is returned. > > drivers/target/target_core_configfs.c | 48 ++++++++++++++++++--------- > 1 file changed, 32 insertions(+), 16 deletions(-) > > diff --git a/drivers/target/target_core_configfs.c b/drivers/target/target_core_configfs.c > index a5f58988130a..c1fbcdd16182 100644 > --- a/drivers/target/target_core_configfs.c > +++ b/drivers/target/target_core_configfs.c > @@ -759,6 +759,29 @@ static ssize_t emulate_tas_store(struct config_item *item, > return count; > } > > +static int target_try_configure_unmap(struct se_device *dev, > + const char *config_opt) > +{ > + if (!dev->transport->configure_unmap) { > + pr_err("Generic Block Discard not supported\n"); > + return -ENOSYS; > + } > + > + if (!target_dev_configured(dev)) { > + pr_err("Generic Block Discard setup for %s requires device to be configured\n", > + config_opt); > + return -ENODEV; > + } > + > + if (!dev->transport->configure_unmap(dev)) { > + pr_err("Generic Block Discard setup for %s failed\n", > + config_opt); > + return -ENOSYS; > + } > + > + return 0; > +} > + > static ssize_t emulate_tpu_store(struct config_item *item, > const char *page, size_t count) > { > @@ -776,11 +799,9 @@ static ssize_t emulate_tpu_store(struct config_item *item, > * Discard supported is detected iblock_create_virtdevice(). > */ > if (flag && !da->max_unmap_block_desc_count) { > - if (!dev->transport->configure_unmap || > - !dev->transport->configure_unmap(dev)) { > - pr_err("Generic Block Discard not supported\n"); > - return -ENOSYS; > - } > + ret = target_try_configure_unmap(dev, "emulate_tpu"); > + if (ret) > + return ret; > } > > da->emulate_tpu = flag; > @@ -806,11 +827,9 @@ static ssize_t emulate_tpws_store(struct config_item *item, > * Discard supported is detected iblock_create_virtdevice(). > */ > if (flag && !da->max_unmap_block_desc_count) { > - if (!dev->transport->configure_unmap || > - !dev->transport->configure_unmap(dev)) { > - pr_err("Generic Block Discard not supported\n"); > - return -ENOSYS; > - } > + ret = target_try_configure_unmap(dev, "emulate_tpws"); > + if (ret) > + return ret; > } > > da->emulate_tpws = flag; > @@ -1022,12 +1041,9 @@ static ssize_t unmap_zeroes_data_store(struct config_item *item, > * Discard supported is detected iblock_configure_device(). > */ > if (flag && !da->max_unmap_block_desc_count) { > - if (!dev->transport->configure_unmap || > - !dev->transport->configure_unmap(dev)) { > - pr_err("dev[%p]: Thin Provisioning LBPRZ will not be set because max_unmap_block_desc_count is zero\n", > - da->da_dev); > - return -ENOSYS; > - } > + ret = target_try_configure_unmap(dev, "unmap_zeroes_data"); > + if (ret) > + return ret; > } > da->unmap_zeroes_data = flag; > pr_debug("dev[%p]: SE Device Thin Provisioning LBPRZ bit: %d\n", > -- > 2.34.1 > Reviewed-by: Maurizio Lombardi <mlombard@redhat.com>
On Fri, 09 Feb 2024 15:52:47 -0600, Mike Christie wrote: > This issue was found and also debugged by Carl Lei <me@xecycle.info>. > > If the device is not enabled, iblock/file will have not setup their > se_device to bdev/file mappings. If a user tries to config the unmap > settings at this time, we will then crash trying to access a NULL > pointer where the bdev/file should be. > > [...] Applied to 6.8/scsi-fixes, thanks! [1/1] scsi: target: Fix unmap setup during configuration https://git.kernel.org/mkp/scsi/c/4cbec7e89a41
diff --git a/drivers/target/target_core_configfs.c b/drivers/target/target_core_configfs.c index a5f58988130a..c1fbcdd16182 100644 --- a/drivers/target/target_core_configfs.c +++ b/drivers/target/target_core_configfs.c @@ -759,6 +759,29 @@ static ssize_t emulate_tas_store(struct config_item *item, return count; } +static int target_try_configure_unmap(struct se_device *dev, + const char *config_opt) +{ + if (!dev->transport->configure_unmap) { + pr_err("Generic Block Discard not supported\n"); + return -ENOSYS; + } + + if (!target_dev_configured(dev)) { + pr_err("Generic Block Discard setup for %s requires device to be configured\n", + config_opt); + return -ENODEV; + } + + if (!dev->transport->configure_unmap(dev)) { + pr_err("Generic Block Discard setup for %s failed\n", + config_opt); + return -ENOSYS; + } + + return 0; +} + static ssize_t emulate_tpu_store(struct config_item *item, const char *page, size_t count) { @@ -776,11 +799,9 @@ static ssize_t emulate_tpu_store(struct config_item *item, * Discard supported is detected iblock_create_virtdevice(). */ if (flag && !da->max_unmap_block_desc_count) { - if (!dev->transport->configure_unmap || - !dev->transport->configure_unmap(dev)) { - pr_err("Generic Block Discard not supported\n"); - return -ENOSYS; - } + ret = target_try_configure_unmap(dev, "emulate_tpu"); + if (ret) + return ret; } da->emulate_tpu = flag; @@ -806,11 +827,9 @@ static ssize_t emulate_tpws_store(struct config_item *item, * Discard supported is detected iblock_create_virtdevice(). */ if (flag && !da->max_unmap_block_desc_count) { - if (!dev->transport->configure_unmap || - !dev->transport->configure_unmap(dev)) { - pr_err("Generic Block Discard not supported\n"); - return -ENOSYS; - } + ret = target_try_configure_unmap(dev, "emulate_tpws"); + if (ret) + return ret; } da->emulate_tpws = flag; @@ -1022,12 +1041,9 @@ static ssize_t unmap_zeroes_data_store(struct config_item *item, * Discard supported is detected iblock_configure_device(). */ if (flag && !da->max_unmap_block_desc_count) { - if (!dev->transport->configure_unmap || - !dev->transport->configure_unmap(dev)) { - pr_err("dev[%p]: Thin Provisioning LBPRZ will not be set because max_unmap_block_desc_count is zero\n", - da->da_dev); - return -ENOSYS; - } + ret = target_try_configure_unmap(dev, "unmap_zeroes_data"); + if (ret) + return ret; } da->unmap_zeroes_data = flag; pr_debug("dev[%p]: SE Device Thin Provisioning LBPRZ bit: %d\n",
This issue was found and also debugged by Carl Lei <me@xecycle.info>. If the device is not enabled, iblock/file will have not setup their se_device to bdev/file mappings. If a user tries to config the unmap settings at this time, we will then crash trying to access a NULL pointer where the bdev/file should be. This patch adds a check to make sure the device is configured before we try to call the configure_unmap callout. Fixes: 34bd1dcacf0d ("scsi: target: Detect UNMAP support post configuration") Reported-by: Carl Lei <me@xecycle.info> Signed-off-by: Mike Christie <michael.christie@oracle.com> --- v2: Fix missing configure_unmap handling so failure is returned. drivers/target/target_core_configfs.c | 48 ++++++++++++++++++--------- 1 file changed, 32 insertions(+), 16 deletions(-)