Commit a0c80efe authored by Jiri Kosina's avatar Jiri Kosina
Browse files

floppy: fix lock_fdc() signal handling 



floppy_revalidate() doesn't perform any error handling on lock_fdc()
result. lock_fdc() might actually be interrupted by a signal (it waits for
fdc becoming non-busy interruptibly). In such case, floppy_revalidate()
proceeds as if it had claimed the lock, but it fact it doesn't.

In case of multiple threads trying to open("/dev/fdX"), this leads to
serious corruptions all over the place, because all of a sudden there is
no critical section protection (that'd otherwise be guaranteed by locked
fd) whatsoever.

While at this, fix the fact that the 'interruptible' parameter to
lock_fdc() doesn't make any sense whatsoever, because we always wait
interruptibly anyway.

Most of the lock_fdc() callsites do properly handle error (and propagate
EINTR), but floppy_revalidate() and floppy_check_events() don't. Fix this.

Spotted by 'syzkaller' tool.

Reported-by: default avatarDmitry Vyukov <dvyukov@google.com>
Tested-by: default avatarDmitry Vyukov <dvyukov@google.com>
Signed-off-by: default avatarJiri Kosina <jkosina@suse.cz>
parent aa0818c6

drivers/block/floppy.c

+18 −15
Original line number Diff line number Diff line
@@ -866,7 +866,7 @@ static void set_fdc(int drive) 
}



/* locks the driver */

static int lock_fdc(int drive, bool interruptible)

static int lock_fdc(int drive)

{

	if (WARN(atomic_read(&usage_count) == 0,

		 "Trying to lock fdc while usage count=0\n"))
@@ -2173,7 +2173,7 @@ static int do_format(int drive, struct format_descr *tmp_format_req) 
{

	int ret;



	if (lock_fdc(drive, true))

	if (lock_fdc(drive))

		return -EINTR;



	set_floppy(drive);
@@ -2960,7 +2960,7 @@ static int user_reset_fdc(int drive, int arg, bool interruptible) 
{

	int ret;



	if (lock_fdc(drive, interruptible))

	if (lock_fdc(drive))

		return -EINTR;



	if (arg == FD_RESET_ALWAYS)
@@ -3243,7 +3243,7 @@ static int set_geometry(unsigned int cmd, struct floppy_struct *g, 
		if (!capable(CAP_SYS_ADMIN))

			return -EPERM;

		mutex_lock(&open_lock);

		if (lock_fdc(drive, true)) {

		if (lock_fdc(drive)) {

			mutex_unlock(&open_lock);

			return -EINTR;

		}
@@ -3263,7 +3263,7 @@ static int set_geometry(unsigned int cmd, struct floppy_struct *g, 
	} else {

		int oldStretch;



		if (lock_fdc(drive, true))

		if (lock_fdc(drive))

			return -EINTR;

		if (cmd != FDDEFPRM) {

			/* notice a disk change immediately, else
@@ -3349,7 +3349,7 @@ static int get_floppy_geometry(int drive, int type, struct floppy_struct **g) 
	if (type)

		*g = &floppy_type[type];

	else {

		if (lock_fdc(drive, false))

		if (lock_fdc(drive))

			return -EINTR;

		if (poll_drive(false, 0) == -EINTR)

			return -EINTR;
@@ -3433,7 +3433,7 @@ static int fd_locked_ioctl(struct block_device *bdev, fmode_t mode, unsigned int 
		if (UDRS->fd_ref != 1)

			/* somebody else has this drive open */

			return -EBUSY;

		if (lock_fdc(drive, true))

		if (lock_fdc(drive))

			return -EINTR;



		/* do the actual eject. Fails on
@@ -3445,7 +3445,7 @@ static int fd_locked_ioctl(struct block_device *bdev, fmode_t mode, unsigned int 
		process_fd_request();

		return ret;

	case FDCLRPRM:

		if (lock_fdc(drive, true))

		if (lock_fdc(drive))

			return -EINTR;

		current_type[drive] = NULL;

		floppy_sizes[drive] = MAX_DISK_SIZE << 1;
@@ -3467,7 +3467,7 @@ static int fd_locked_ioctl(struct block_device *bdev, fmode_t mode, unsigned int 
		UDP->flags &= ~FTD_MSG;

		return 0;

	case FDFMTBEG:

		if (lock_fdc(drive, true))

		if (lock_fdc(drive))

			return -EINTR;

		if (poll_drive(true, FD_RAW_NEED_DISK) == -EINTR)

			return -EINTR;
@@ -3484,7 +3484,7 @@ static int fd_locked_ioctl(struct block_device *bdev, fmode_t mode, unsigned int 
		return do_format(drive, &inparam.f);

	case FDFMTEND:

	case FDFLUSH:

		if (lock_fdc(drive, true))

		if (lock_fdc(drive))

			return -EINTR;

		return invalidate_drive(bdev);

	case FDSETEMSGTRESH:
@@ -3507,7 +3507,7 @@ static int fd_locked_ioctl(struct block_device *bdev, fmode_t mode, unsigned int 
		outparam = UDP;

		break;

	case FDPOLLDRVSTAT:

		if (lock_fdc(drive, true))

		if (lock_fdc(drive))

			return -EINTR;

		if (poll_drive(true, FD_RAW_NEED_DISK) == -EINTR)

			return -EINTR;
@@ -3530,7 +3530,7 @@ static int fd_locked_ioctl(struct block_device *bdev, fmode_t mode, unsigned int 
	case FDRAWCMD:

		if (type)

			return -EINVAL;

		if (lock_fdc(drive, true))

		if (lock_fdc(drive))

			return -EINTR;

		set_floppy(drive);

		i = raw_cmd_ioctl(cmd, (void __user *)param);
@@ -3539,7 +3539,7 @@ static int fd_locked_ioctl(struct block_device *bdev, fmode_t mode, unsigned int 
		process_fd_request();

		return i;

	case FDTWADDLE:

		if (lock_fdc(drive, true))

		if (lock_fdc(drive))

			return -EINTR;

		twaddle();

		process_fd_request();
@@ -3747,7 +3747,8 @@ static unsigned int floppy_check_events(struct gendisk *disk, 
		return DISK_EVENT_MEDIA_CHANGE;



	if (time_after(jiffies, UDRS->last_checked + UDP->checkfreq)) {

		lock_fdc(drive, false);

		if (lock_fdc(drive))

			return -EINTR;

		poll_drive(false, 0);

		process_fd_request();

	}
@@ -3845,7 +3846,9 @@ static int floppy_revalidate(struct gendisk *disk) 
			 "VFS: revalidate called on non-open device.\n"))

			return -EFAULT;



		lock_fdc(drive, false);

		res = lock_fdc(drive);

		if (res)

			return res;

		cf = (test_bit(FD_DISK_CHANGED_BIT, &UDRS->flags) ||

		      test_bit(FD_VERIFY_BIT, &UDRS->flags));

		if (!(cf || test_bit(drive, &fake_change) || drive_no_geom(drive))) {

CRA Git | Maintained and supported by SUSTech CRA and CCSE