diff mbox series

null_blk: Fix return value of nullb_device_power_store()

Message ID 20240527043445.235267-1-dlemoal@kernel.org (mailing list archive)
State New, archived
Headers show
Series null_blk: Fix return value of nullb_device_power_store() | expand

Commit Message

Damien Le Moal May 27, 2024, 4:34 a.m. UTC
When powering on a null_blk device that is not already on, the return
value ret that is initialized to be count is reused to check the return
value of null_add_dev(), leading to nullb_device_power_store() to return
null_add_dev() return value (0 on success) instead of "count".
So make sure to set ret to be equal to count when there are no errors.

Fixes: a2db328b0839 ("null_blk: fix null-ptr-dereference while configuring 'power' and 'submit_queues'")
Signed-off-by: Damien Le Moal <dlemoal@kernel.org>
---
 drivers/block/null_blk/main.c | 1 +
 1 file changed, 1 insertion(+)

Comments

Kanchan Joshi May 27, 2024, 5:33 a.m. UTC | #1
Reviewed-by: Kanchan Joshi <joshi.k@samsung.com>
Yu Kuai May 27, 2024, 6:32 a.m. UTC | #2
Hi,

在 2024/05/27 12:34, Damien Le Moal 写道:
> When powering on a null_blk device that is not already on, the return
> value ret that is initialized to be count is reused to check the return
> value of null_add_dev(), leading to nullb_device_power_store() to return
> null_add_dev() return value (0 on success) instead of "count".
> So make sure to set ret to be equal to count when there are no errors.

Yes, thanks for the patch!

And the reason test did't find this problem is that the "echo" cmd will
write again to the configfs entry, and nullb_device_power_store() will
found the allocated nullb_device.

Reviewed-by: Yu Kuai <yukuai3@huawei.com>
> 
> Fixes: a2db328b0839 ("null_blk: fix null-ptr-dereference while configuring 'power' and 'submit_queues'")
> Signed-off-by: Damien Le Moal <dlemoal@kernel.org>
> ---
>   drivers/block/null_blk/main.c | 1 +
>   1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/block/null_blk/main.c b/drivers/block/null_blk/main.c
> index eb023d267369..631dca2e4e84 100644
> --- a/drivers/block/null_blk/main.c
> +++ b/drivers/block/null_blk/main.c
> @@ -494,6 +494,7 @@ static ssize_t nullb_device_power_store(struct config_item *item,
>   
>   		set_bit(NULLB_DEV_FL_CONFIGURED, &dev->flags);
>   		dev->power = newp;
> +		ret = count;
>   	} else if (dev->power && !newp) {
>   		if (test_and_clear_bit(NULLB_DEV_FL_UP, &dev->flags)) {
>   			dev->power = newp;
>
Jens Axboe May 27, 2024, 7:57 p.m. UTC | #3
On Mon, 27 May 2024 13:34:45 +0900, Damien Le Moal wrote:
> When powering on a null_blk device that is not already on, the return
> value ret that is initialized to be count is reused to check the return
> value of null_add_dev(), leading to nullb_device_power_store() to return
> null_add_dev() return value (0 on success) instead of "count".
> So make sure to set ret to be equal to count when there are no errors.
> 
> 
> [...]

Applied, thanks!

[1/1] null_blk: Fix return value of nullb_device_power_store()
      commit: d9ff882b54f99f96787fa3df7cd938966843c418

Best regards,
diff mbox series

Patch

diff --git a/drivers/block/null_blk/main.c b/drivers/block/null_blk/main.c
index eb023d267369..631dca2e4e84 100644
--- a/drivers/block/null_blk/main.c
+++ b/drivers/block/null_blk/main.c
@@ -494,6 +494,7 @@  static ssize_t nullb_device_power_store(struct config_item *item,
 
 		set_bit(NULLB_DEV_FL_CONFIGURED, &dev->flags);
 		dev->power = newp;
+		ret = count;
 	} else if (dev->power && !newp) {
 		if (test_and_clear_bit(NULLB_DEV_FL_UP, &dev->flags)) {
 			dev->power = newp;