ide: fix races in handling of user-space SET XFER commands
authorBartlomiej Zolnierkiewicz <bzolnier@gmail.com>
Tue, 23 Jun 2009 11:35:51 +0000 (11:35 +0000)
committerDavid S. Miller <davem@davemloft.net>
Fri, 7 Aug 2009 17:43:00 +0000 (10:43 -0700)
* Make cmd->tf_flags field 'u16' and add IDE_TFLAG_SET_XFER taskfile flag.

* Update ide_finish_cmd() to set xfer / re-read id if the new flag is set.

* Convert set_xfer_rate() (write handler for /proc/ide/hd?/current_speed)
  and ide_cmd_ioctl() (HDIO_DRIVE_CMD ioctl handler) to use the new flag.

* Remove no longer needed disable_irq_nosync() + enable_irq() from
  ide_config_drive_speed().

Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
drivers/ide/ide-ioctls.c
drivers/ide/ide-iops.c
drivers/ide/ide-proc.c
drivers/ide/ide-taskfile.c
include/linux/ide.h

index e246d3d3fbcc5c280ff520b090c297eb42c05a1e..d3440b5010a5830fc936383b4a65ffd66b4163d6 100644 (file)
@@ -167,6 +167,8 @@ static int ide_cmd_ioctl(ide_drive_t *drive, unsigned long arg)
                        err = -EINVAL;
                        goto abort;
                }
+
+               cmd.tf_flags |= IDE_TFLAG_SET_XFER;
        }
 
        err = ide_raw_taskfile(drive, &cmd, buf, args[3]);
@@ -174,12 +176,6 @@ static int ide_cmd_ioctl(ide_drive_t *drive, unsigned long arg)
        args[0] = tf->status;
        args[1] = tf->error;
        args[2] = tf->nsect;
-
-       if (!err && xfer_rate) {
-               /* active-retuning-calls future */
-               ide_set_xfer_rate(drive, xfer_rate);
-               ide_driveid_update(drive);
-       }
 abort:
        if (copy_to_user((void __user *)arg, &args, 4))
                err = -EFAULT;
index b99873845d21a92c8fe4e8a7b36ae9aff18895e7..b14fa9a87c4923ed274cdea8ddaa590594adb8b4 100644 (file)
@@ -363,14 +363,6 @@ int ide_config_drive_speed(ide_drive_t *drive, u8 speed)
         * this point (lost interrupt).
         */
 
-       /*
-        *      FIXME: we race against the running IRQ here if
-        *      this is called from non IRQ context. If we use
-        *      disable_irq() we hang on the error path. Work
-        *      is needed.
-        */
-       disable_irq_nosync(hwif->irq);
-
        udelay(1);
        tp_ops->dev_select(drive);
        SELECT_MASK(drive, 1);
@@ -394,8 +386,6 @@ int ide_config_drive_speed(ide_drive_t *drive, u8 speed)
 
        SELECT_MASK(drive, 0);
 
-       enable_irq(hwif->irq);
-
        if (error) {
                (void) ide_dump_status(drive, "set_drive_speed_status", stat);
                return error;
index 3242698832a40c07d02c2dabbd60b338eeb15d69..021de41655e68224a5b1274cbfd89d96bc2717d1 100644 (file)
@@ -195,7 +195,6 @@ ide_devset_get(xfer_rate, current_speed);
 static int set_xfer_rate (ide_drive_t *drive, int arg)
 {
        struct ide_cmd cmd;
-       int err;
 
        if (arg < XFER_PIO_0 || arg > XFER_UDMA_6)
                return -EINVAL;
@@ -206,14 +205,9 @@ static int set_xfer_rate (ide_drive_t *drive, int arg)
        cmd.tf.nsect   = (u8)arg;
        cmd.valid.out.tf = IDE_VALID_FEATURE | IDE_VALID_NSECT;
        cmd.valid.in.tf  = IDE_VALID_NSECT;
+       cmd.tf_flags   = IDE_TFLAG_SET_XFER;
 
-       err = ide_no_data_taskfile(drive, &cmd);
-
-       if (!err) {
-               ide_set_xfer_rate(drive, (u8) arg);
-               ide_driveid_update(drive);
-       }
-       return err;
+       return ide_no_data_taskfile(drive, &cmd);
 }
 
 ide_devset_rw(current_speed, xfer_rate);
index 50336d51eebc0ae205f444613c889eb600009e88..cc8633cbe133db4d9b7f81330ef66a547a6176ac 100644 (file)
@@ -324,10 +324,17 @@ static void ide_error_cmd(ide_drive_t *drive, struct ide_cmd *cmd)
 void ide_finish_cmd(ide_drive_t *drive, struct ide_cmd *cmd, u8 stat)
 {
        struct request *rq = drive->hwif->rq;
-       u8 err = ide_read_error(drive);
+       u8 err = ide_read_error(drive), nsect = cmd->tf.nsect;
+       u8 set_xfer = !!(cmd->tf_flags & IDE_TFLAG_SET_XFER);
 
        ide_complete_cmd(drive, cmd, stat, err);
        rq->errors = err;
+
+       if (err == 0 && set_xfer) {
+               ide_set_xfer_rate(drive, nsect);
+               ide_driveid_update(drive);
+       }
+
        ide_complete_rq(drive, err ? -EIO : 0, blk_rq_bytes(rq));
 }
 
index cb6cd0459a5ee13bbe8f1ad915b528f45d0b74e8..803c1ae31237d5de17b4418db9e8e1e1d1cba3e9 100644 (file)
@@ -258,6 +258,7 @@ enum {
        IDE_TFLAG_DYN                   = (1 << 5),
        IDE_TFLAG_FS                    = (1 << 6),
        IDE_TFLAG_MULTI_PIO             = (1 << 7),
+       IDE_TFLAG_SET_XFER              = (1 << 8),
 };
 
 enum {
@@ -294,7 +295,7 @@ struct ide_cmd {
                } out, in;
        } valid;
 
-       u                     tf_flags;
+       u16                     tf_flags;
        u8                      ftf_flags;      /* for TASKFILE ioctl */
        int                     protocol;