<div dir="ltr"><div dir="ltr">On Fri, Sep 3, 2021 at 10:02 AM Dinghao Liu <<a href="mailto:dinghao.liu@zju.edu.cn">dinghao.liu@zju.edu.cn</a>> wrote:<br></div><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">mddev_unlock() is called on all paths after we call mddev_lock_nointr(),<br>
except for three error handling paths, which may cause a deadlock. This<br>
bug is suggested by a static analysis tool, please advise.<br></blockquote><div><br></div><div>Hi,</div><div><br></div><div>correct, those unlock calls are missing.</div><div><br></div><div>As we are bailing out after md_run() with lock held, </div><div>we can clean the lot of error paths underneath up by jumping to before<br>md_stop() and add the mddev_unlock upfront it like:</div><div><br></div><div>From 5c72f1d07195127f5fd49bccbe0705854463c199 Mon Sep 17 00:00:00 2001<br>Message-Id: <<a href="mailto:5c72f1d07195127f5fd49bccbe0705854463c199.1630675612.git.heinzm@redhat.com">5c72f1d07195127f5fd49bccbe0705854463c199.1630675612.git.heinzm@redhat.com</a>><br>From: Heinz Mauelshagen <<a href="mailto:heinzm@redhat.com">heinzm@redhat.com</a>><br>Date: Fri, 3 Sep 2021 15:26:50 +0200<br>Subject: [PATCH] dm raid: fix mddev unlocking in raid_ctr() error paths<br><br>Signed-off-by: Heinz Mauelshagen <<a href="mailto:heinzm@redhat.com">heinzm@redhat.com</a>><br>---<br> drivers/md/dm-raid.c | 7 +++----<br> 1 file changed, 3 insertions(+), 4 deletions(-)<br><br>diff --git a/drivers/md/dm-raid.c b/drivers/md/dm-raid.c<br>index d9ef52159a22..741bab00e922 100644<br>--- a/drivers/md/dm-raid.c<br>+++ b/drivers/md/dm-raid.c<br>@@ -3249,14 +3249,12 @@ static int raid_ctr(struct dm_target *ti, unsigned int argc, char **argv)<br> rs->md.in_sync = 0; /* Assume already marked dirty */<br> if (r) {<br> ti->error = "Failed to run raid array";<br>- mddev_unlock(&rs->md);<br>- goto bad;<br>+ goto bad_unlock;<br> } <br> <br> r = md_start(&rs->md);<br> if (r) {<br> ti->error = "Failed to start raid array";<br>- mddev_unlock(&rs->md);<br> goto bad_md_start;<br> } <br> <br>@@ -3265,7 +3263,6 @@ static int raid_ctr(struct dm_target *ti, unsigned int argc, char **argv)<br> r = r5c_journal_mode_set(&rs->md, rs->journal_dev.mode);<br> if (r) {<br> ti->error = "Failed to set raid4/5/6 journal mode";<br>- mddev_unlock(&rs->md);<br> goto bad_journal_mode_set;<br> } <br> } <br>@@ -3304,10 +3301,12 @@ static int raid_ctr(struct dm_target *ti, unsigned int argc, char **argv)<br> mddev_unlock(&rs->md);<br> return 0;<br> <br>+bad_unlock:<br> bad_md_start:<br> bad_journal_mode_set:<br> bad_stripe_cache:<br> bad_check_reshape:<br>+ mddev_unlock(&rs->md);<br> md_stop(&rs->md);<br> bad:<br> raid_set_free(rs);<br>-- <br>2.31.1<br><br></div><div><br></div><div>-- lvmguy</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
Fixes: 9dbd1aa3a81c ("dm raid: add reshaping support to the target")<br>
Signed-off-by: Dinghao Liu <<a href="mailto:dinghao.liu@zju.edu.cn" target="_blank">dinghao.liu@zju.edu.cn</a>><br>
---<br>
drivers/md/dm-raid.c | 9 +++++++--<br>
1 file changed, 7 insertions(+), 2 deletions(-)<br>
<br>
diff --git a/drivers/md/dm-raid.c b/drivers/md/dm-raid.c<br>
index d9ef52159a22..79f36a806082 100644<br>
--- a/drivers/md/dm-raid.c<br>
+++ b/drivers/md/dm-raid.c<br>
@@ -3276,15 +3276,19 @@ static int raid_ctr(struct dm_target *ti, unsigned int argc, char **argv)<br>
/* Try to adjust the raid4/5/6 stripe cache size to the stripe size */<br>
if (rs_is_raid456(rs)) {<br>
r = rs_set_raid456_stripe_cache(rs);<br>
- if (r)<br>
+ if (r) {<br>
+ mddev_unlock(&rs->md);<br>
goto bad_stripe_cache;<br>
+ }<br>
}<br>
<br>
/* Now do an early reshape check */<br>
if (test_bit(RT_FLAG_RESHAPE_RS, &rs->runtime_flags)) {<br>
r = rs_check_reshape(rs);<br>
- if (r)<br>
+ if (r) {<br>
+ mddev_unlock(&rs->md);<br>
goto bad_check_reshape;<br>
+ }<br>
<br>
/* Restore new, ctr requested layout to perform check */<br>
rs_config_restore(rs, &rs_layout);<br>
@@ -3293,6 +3297,7 @@ static int raid_ctr(struct dm_target *ti, unsigned int argc, char **argv)<br>
r = rs->md.pers->check_reshape(&rs->md);<br>
if (r) {<br>
ti->error = "Reshape check failed";<br>
+ mddev_unlock(&rs->md);<br>
goto bad_check_reshape;<br>
}<br>
}<br>
-- <br>
2.17.1<br>
<br>
</blockquote></div></div>