linux-kernel - Re: regression: disk error loop (panic?) ide_do_rw

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]

Message-ID: <alpine.LFD.0.999.0707181238500.27353@woody.linux-foundation.org>
Date:	Wed, 18 Jul 2007 12:57:28 -0700 (PDT)
From:	Linus Torvalds <torvalds@...ux-foundation.org>
To:	Bartlomiej Zolnierkiewicz <bzolnier@...il.com>
cc:	Giacomo Catenazzi <cate@...eee.net>,
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
	Jens Axboe <jens.axboe@...cle.com>, akpm@...ux-foundation.org
Subject: Re: regression: disk error loop (panic?) ide_do_rw_disk-bad:

On Tue, 17 Jul 2007, Bartlomiej Zolnierkiewicz wrote:
> 
> On Tuesday 17 July 2007, Giacomo Catenazzi wrote:
> > 
> > last git changes to git give me the following error (repead very quickly):
> > 
> > sector 14657019, nr/cmr 0/0
> > bio f7b59280, biotail f7b59280, buffer 0000000, date 0000000, len 36
> > ide_do_rw_disk-bad command: dev hda: type 2, flags 104c8
> 
> ide-disk driver and type 2 (REQ_TYPE_BLOCK_PC) requests don't mix well

Hmm. I started wondering about this. 

Sure, ide-disk doesn't like non-fs requests, but I'm wondering why the 
error apparently keeps repeating? We should error out once, and that 
should be it.

So I'm wondering whether the

	ide_end_request(drive, 0, 0);
	return ide_stopped;

is the real bug.

The thing is, non-fs requests won't have nr_sectors set, so the 0 will 
stay as a zero (the IDE layer has some "turn zero into current sector 
number", but that doesn't work for commands that aren't sector-based!), 
and as a result, we'll "complete" zero bytes, and the bio will stay 
active!

So the code should probably at the _least_ say that it's finished one 
whole sector, and it would probably be much better to make it be based on 
"rq->data_len", which should be reliable.

Of course, since the "ide_end_request()" takes the data in sectors, but 
the request layer does it in bytes, we have to round the thing up and the 
ide_end_request() thing will turn it into too many bytes, but that's ok - 
"end_that_request_first()" doesn't care if we complete "too many" bytes.

So a diff something like this is, I think, the right thing to do, and 
should make the IDE disk driver handle non-fs requests gracefully (it will 
*fail* them, of course, but that's another issue).

Bartlomiej? Am I missing soemthing?

		Linus

---
 drivers/ide/ide-disk.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/drivers/ide/ide-disk.c b/drivers/ide/ide-disk.c
index b1304a7..fde2f79 100644
--- a/drivers/ide/ide-disk.c
+++ b/drivers/ide/ide-disk.c
@@ -311,8 +311,9 @@ static ide_startstop_t ide_do_rw_disk (ide_drive_t *drive, struct request *rq, s
 	BUG_ON(drive->blocked);

 	if (!blk_fs_request(rq)) {
+		int nsectors = (rq->data_len + 511) >> 9;
 		blk_dump_rq_flags(rq, "ide_do_rw_disk - bad command");
-		ide_end_request(drive, 0, 0);
+		ide_end_request(drive, 0, nsectors);
 		return ide_stopped;
 	}

-
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/