[PATCH] dm: add missing unlock on in dm_keyslot_evict()

[PATCH] dm: add missing unlock on in dm_keyslot_evict()

[Dan Carpenter]

We need to call dm_put_live_table() even if dm_get_live_table() returns
NULL.

Fixes: 9355a9eb21a5 ("dm: support key eviction from keyslot managers of underlying devices")
Signed-off-by: Dan Carpenter <dan.carpenter@linaro.org>
---
 drivers/md/dm-table.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/md/dm-table.c b/drivers/md/dm-table.c
index 933e01f3fab4..1a7e2623069b 100644
--- a/drivers/md/dm-table.c
+++ b/drivers/md/dm-table.c
@@ -1177,7 +1177,7 @@ static int dm_keyslot_evict(struct blk_crypto_profile *profile,
 
 	t = dm_get_live_table(md, &srcu_idx);
 	if (!t)
-		return 0;
+		goto put_live_table;
 
 	for (unsigned int i = 0; i < t->num_targets; i++) {
 		struct dm_target *ti = dm_table_get_target(t, i);
@@ -1188,6 +1188,7 @@ static int dm_keyslot_evict(struct blk_crypto_profile *profile,
 					  (void *)key);
 	}
 
+put_live_table:
 	dm_put_live_table(md, srcu_idx);
 	return 0;
 }
-- 
2.47.2

Re: [PATCH] dm: add missing unlock on in dm_keyslot_evict()

[Eric Biggers]

On Wed, Apr 30, 2025 at 11:05:54AM +0300, Dan Carpenter wrote:
> We need to call dm_put_live_table() even if dm_get_live_table() returns
> NULL.
> 
> Fixes: 9355a9eb21a5 ("dm: support key eviction from keyslot managers of underlying devices")
> Signed-off-by: Dan Carpenter <dan.carpenter@linaro.org>
> ---
>  drivers/md/dm-table.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 

Reviewed-by: Eric Biggers <ebiggers@kernel.org>

But that's an awfully error-prone API.

dm_blk_report_zones() gets this wrong too.

- Eric

Re: [PATCH] dm: add missing unlock on in dm_keyslot_evict()

[Dan Carpenter]

On Wed, Apr 30, 2025 at 09:50:37AM -0700, Eric Biggers wrote:
> On Wed, Apr 30, 2025 at 11:05:54AM +0300, Dan Carpenter wrote:
> > We need to call dm_put_live_table() even if dm_get_live_table() returns
> > NULL.
> > 
> > Fixes: 9355a9eb21a5 ("dm: support key eviction from keyslot managers of underlying devices")
> > Signed-off-by: Dan Carpenter <dan.carpenter@linaro.org>
> > ---
> >  drivers/md/dm-table.c | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> > 
> 
> Reviewed-by: Eric Biggers <ebiggers@kernel.org>
> 
> But that's an awfully error-prone API.

Yep.

> 
> dm_blk_report_zones() gets this wrong too.

Ugh...  dm_blk_report_zones() is too weird for my static checker tool.
The checker is looking very specifically for error paths with missing
unlocks.

regards,
dan carpenter

Re: [PATCH] dm: add missing unlock on in dm_keyslot_evict()

[Mikulas Patocka]

On Wed, 30 Apr 2025, Dan Carpenter wrote:

> On Wed, Apr 30, 2025 at 09:50:37AM -0700, Eric Biggers wrote:
> > On Wed, Apr 30, 2025 at 11:05:54AM +0300, Dan Carpenter wrote:
> > > We need to call dm_put_live_table() even if dm_get_live_table() returns
> > > NULL.
> > > 
> > > Fixes: 9355a9eb21a5 ("dm: support key eviction from keyslot managers of underlying devices")
> > > Signed-off-by: Dan Carpenter <dan.carpenter@linaro.org>
> > > ---
> > >  drivers/md/dm-table.c | 3 ++-
> > >  1 file changed, 2 insertions(+), 1 deletion(-)
> > > 
> > 
> > Reviewed-by: Eric Biggers <ebiggers@kernel.org>
> > 
> > But that's an awfully error-prone API.
> 
> Yep.
> 
> > 
> > dm_blk_report_zones() gets this wrong too.
> 
> Ugh...  dm_blk_report_zones() is too weird for my static checker tool.
> The checker is looking very specifically for error paths with missing
> unlocks.

Ben already tried to fix it in dm_blk_report_zones (see the linux-dm git, 
for-next branch) - but his fix is incorrect because the "if" condition for 
dm_get_live_table and dm_put_live_table differs. I'll update his patch to 
fix this mismatch.

Mikulas

> regards,
> dan carpenter
> 

Re: [PATCH] dm: add missing unlock on in dm_keyslot_evict()

[Mikulas Patocka]

On Wed, 30 Apr 2025, Mikulas Patocka wrote:

> Ben already tried to fix it in dm_blk_report_zones (see the linux-dm git, 
> for-next branch) - but his fix is incorrect because the "if" condition for 
> dm_get_live_table and dm_put_live_table differs. I'll update his patch to 
> fix this mismatch.
> 
> Mikulas

I fixed it in the Ben's patch "dm: fix dm_blk_report_zones". Ben, please 
review it - it's the patch 37f53a2c60d03743e0eacf7a0c01c279776fef4e in the 
linux-dm repository, for-next branch.

Mikulas

Re: [PATCH] dm: add missing unlock on in dm_keyslot_evict()

[Benjamin Marzinski]

On Sun, May 04, 2025 at 01:30:42PM +0200, Mikulas Patocka wrote:
> 
> 
> On Wed, 30 Apr 2025, Mikulas Patocka wrote:
> 
> > Ben already tried to fix it in dm_blk_report_zones (see the linux-dm git, 
> > for-next branch) - but his fix is incorrect because the "if" condition for 
> > dm_get_live_table and dm_put_live_table differs. I'll update his patch to 
> > fix this mismatch.
> > 
> > Mikulas
> 
> I fixed it in the Ben's patch "dm: fix dm_blk_report_zones". Ben, please 
> review it - it's the patch 37f53a2c60d03743e0eacf7a0c01c279776fef4e in the 
> linux-dm repository, for-next branch.

Your changes look good. Thanks for catching that.
-Ben

> 
> Mikulas