Potential concurrency bug in ide-disk.c ?

[Tushar Adeshara]

Hi,
The way file ide-disk.c handles usage count, it seems to me that its
concurrency bug.
In open method and release, it uses code as follows


static int idedisk_open(struct inode *inode, struct file *filp)
{
	ide_drive_t *drive = inode->i_bdev->bd_disk->private_data;
	drive->usage++;
	if (drive->removable && drive->usage == 1) {
		ide_task_t args;
		memset(&args, 0, sizeof(ide_task_t));
		args.tfRegister[IDE_COMMAND_OFFSET] = WIN_DOORLOCK;
		args.command_type = IDE_DRIVE_TASK_NO_DATA;
		args.handler	  = &task_no_data_intr;
		check_disk_change(inode->i_bdev);
		/*
		 * Ignore the return code from door_lock,
		 * since the open() has already succeeded,
		 * and the door_lock is irrelevant at this point.
		 */
		if (drive->doorlocking && ide_raw_taskfile(drive, &args, NULL))
			drive->doorlocking = 0;
	}
	return 0;
}


Here, if drive->usage=0 initially and two process concurrently executes 
drive->usage++, then drive->usage will become 2.  Both of them will
think that drive is already initialized. Something similar can happen
in case of release.
                      I think a semaphore need to be added in
ide_drive_t structure and method should be modified as

static int idedisk_open(struct inode *inode, struct file *filp)
{
	ide_drive_t *drive = inode->i_bdev->bd_disk->private_data;
        if(down_interruptible(&drive->sem)){
                    /*error handling code*/
        } 
	drive->usage++;
	if (drive->removable && drive->usage == 1) {
		ide_task_t args;
		memset(&args, 0, sizeof(ide_task_t));
		args.tfRegister[IDE_COMMAND_OFFSET] = WIN_DOORLOCK;
		args.command_type = IDE_DRIVE_TASK_NO_DATA;
		args.handler	  = &task_no_data_intr;
		check_disk_change(inode->i_bdev);
		/*
		 * Ignore the return code from door_lock,
		 * since the open() has already succeeded,
		 * and the door_lock is irrelevant at this point.
		 */
		if (drive->doorlocking && ide_raw_taskfile(drive, &args, NULL))
			drive->doorlocking = 0;
	}
         up(&drive->sem);
	return 0;
}
Similar modifications are also required in release.

Please let me know if there is anything wrong in above code. Also let
me know to whom I should offer patches for this.

-- 
Regards,
Tushar
--------------------
It's not a problem, it's an opportunity for improvement. Lets improve.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/