[PATCH] fix BUG with IDE taskfile ioctl and cfq

[PATCH] fix BUG with IDE taskfile ioctl and cfq

[Mikulas Patocka]

Hi

This fixes a crash when using hdparm --security-* on IDE devices with CFQ 
scheduler.

BTW. I found out that it's impossible to use these security commands on 
sparc64 because of 64-bit kernel, 32-bit userspace and a lack of ioctl 
emulation (I succeeded when I recompiled hdparm on my own in 64-bit mode). 
What do you think is the best fix for it? Put it to fs/compat_ioctl.c 
(like SG_IO)? Or make a compat_ioctl method in IDE? Or don't fix it and 
persuade distros to compile hdparm as 64-bit?

Mikulas

---

Fix IDE taskfile with cfq scheduler

When ide taskfile access is being used (for example with hdparm --security
commands) and cfq scheduler is selected, the scheduler crashes on BUG in
cfq_put_request.

The reason is that the cfq scheduler is tracking counts of read and write
requests separately; the ide-taskfile subsystem allocates a read request and
then flips the flag to make it a write request. The counters in cfq will
mismatch.

This patch changes ide-taskfile to allocate the READ or WRITE request as
required and don't change the flag later.

Signed-off-by: Mikulas Patocka <mpatocka@redhat.com>

---
 drivers/ide/ide-taskfile.c |    6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

Index: linux-2.6.34-rc2-fast/drivers/ide/ide-taskfile.c
===================================================================
--- linux-2.6.34-rc2-fast.orig/drivers/ide/ide-taskfile.c	2010-03-29 21:44:59.000000000 +0200
+++ linux-2.6.34-rc2-fast/drivers/ide/ide-taskfile.c	2010-03-29 21:52:04.000000000 +0200
@@ -428,13 +428,11 @@ int ide_raw_taskfile(ide_drive_t *drive,
 {
 	struct request *rq;
 	int error;
+	int rw = !(cmd->tf_flags & IDE_TFLAG_WRITE) ? READ : WRITE;
 
-	rq = blk_get_request(drive->queue, READ, __GFP_WAIT);
+	rq = blk_get_request(drive->queue, rw, __GFP_WAIT);
 	rq->cmd_type = REQ_TYPE_ATA_TASKFILE;
 
-	if (cmd->tf_flags & IDE_TFLAG_WRITE)
-		rq->cmd_flags |= REQ_RW;
-
 	/*
 	 * (ks) We transfer currently only whole sectors.
 	 * This is suffient for now.  But, it would be great,

Re: [PATCH] fix BUG with IDE taskfile ioctl and cfq

[David Miller]

From: Mikulas Patocka <mpatocka@redhat.com>
Date: Tue, 6 Apr 2010 19:29:57 -0400 (EDT)

> This fixes a crash when using hdparm --security-* on IDE devices with CFQ 
> scheduler.

Thanks I'll take a look at a this.

> BTW. I found out that it's impossible to use these security commands on 
> sparc64 because of 64-bit kernel, 32-bit userspace and a lack of ioctl 
> emulation (I succeeded when I recompiled hdparm on my own in 64-bit mode). 
> What do you think is the best fix for it? Put it to fs/compat_ioctl.c 
> (like SG_IO)? Or make a compat_ioctl method in IDE? Or don't fix it and 
> persuade distros to compile hdparm as 64-bit?

The general pattern is to move the ioctl compat handlers into
the drivers themselves, so putting in a compat handler in the
IDE layer is the way to go.

Re: [PATCH] fix BUG with IDE taskfile ioctl and cfq

[David Miller]

From: Mikulas Patocka <mpatocka@redhat.com>
Date: Tue, 6 Apr 2010 19:29:57 -0400 (EDT)

> Fix IDE taskfile with cfq scheduler
> 
> When ide taskfile access is being used (for example with hdparm --security
> commands) and cfq scheduler is selected, the scheduler crashes on BUG in
> cfq_put_request.
> 
> The reason is that the cfq scheduler is tracking counts of read and write
> requests separately; the ide-taskfile subsystem allocates a read request and
> then flips the flag to make it a write request. The counters in cfq will
> mismatch.
> 
> This patch changes ide-taskfile to allocate the READ or WRITE request as
> required and don't change the flag later.
> 
> Signed-off-by: Mikulas Patocka <mpatocka@redhat.com>

Applied and queued up for -stable, thanks again Mikulas.