linux-kernel - Re: bsg locking patches update

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Wed, 28 May 2008 21:00:56 +0900
From:	FUJITA Tomonori <fujita.tomonori@....ntt.co.jp>
To:	pw@....edu
Cc:	fujita.tomonori@....ntt.co.jp, michaelc@...wisc.edu,
	linux-scsi@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: bsg locking patches update

On Mon, 26 May 2008 12:53:18 -0400
Pete Wyckoff <pw@....edu> wrote:

> I finally got around to testing the set of lifetime management
> fixes you applied.  This is 2.6.26-rc3 with some varlen, bidi,
> iser patches, and iovec on bsg, but nothing that should affect
> the locking.
> 
> I can confirm that the first two of these three old bugs are
> no longer reproducable:
> 
>     http://marc.info/?l=linux-scsi&m=120508166505141&w=2
>     http://marc.info/?l=linux-scsi&m=120508177905365&w=2
>     http://marc.info/?l=linux-scsi&m=120508178005376&w=2
> 
> Thanks!  The third, however, is a hang that still can happen.  But
> it is very obscure and requires a bit of timing to get right.  As a
> reminder, here's the setup, and updated traces.

Ah, sorry about it. I didn't understand the third correctly.


> Maybe it is necessary to split up that bsg_mutex to use multiple
> finer-grained locks.

We could but we use bsg_mutex to protect bsg_device_list and idr. So I
think that we don't need hold bsg_mutex during
bsg_complete_all_commands. How about this?


diff --git a/block/bsg.c b/block/bsg.c
index f0b7cd3..d81104e 100644
--- a/block/bsg.c
+++ b/block/bsg.c
@@ -721,8 +721,6 @@ static int bsg_put_device(struct bsg_device *bd)
 	int ret = 0, do_free;
 	struct request_queue *q = bd->queue;
 
-	mutex_lock(&bsg_mutex);
-
 	do_free = atomic_dec_and_test(&bd->ref_count);
 	if (!do_free)
 		goto out;
@@ -741,10 +739,12 @@ static int bsg_put_device(struct bsg_device *bd)
 	 */
 	ret = bsg_complete_all_commands(bd);
 
+	mutex_lock(&bsg_mutex);
 	hlist_del(&bd->dev_list);
+	mutex_unlock(&bsg_mutex);
+
 	kfree(bd);
 out:
-	mutex_unlock(&bsg_mutex);
 	kref_put(&q->bsg_dev.ref, bsg_kref_release_function);
 	if (do_free)
 		blk_put_queue(q);
-- 
1.5.4.2

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/