mirror of
https://github.com/torvalds/linux.git
synced 2026-11-07 06:17:02 +01:00
Floppy fixes for 7.4
Six fixes for long-standing floppy bugs, most of them found by syzbot
or by race detectors:
- Flush pending work before releasing the IRQ and DMA when no FDC is
found, from Karl Mehltretter. The work queued by the probe could run
after the IRQ and DMA were already freed ("work still pending").
- Unregister the platform device when add_disk() fails, from Guangshuo
Li. The failing drive's device stayed registered after the module
was unloaded, and reading its sysfs attributes oopsed on freed module
memory.
- Return early from reset_interrupt() when cont is already NULL, from
Yang Xiuwei. This fixes the syzbot GPF in reset_interrupt(). I decided
to take this v1 as a partial solution; the full fix will require
additional rework.
- Replace volatile on command_status with READ_ONCE()/WRITE_ONCE() and
annotate floppy_work_fn and current_type[], from Cen Zhang. No
functional change.
- Don't keep a pointer to a stack buffer in timeout_message.
show_floppy() printed it from the interrupt handler after
request_done() had returned (syzbot KASAN report).
- Copy the drive state flags in the 32-bit FDGETDRVSTAT and
FDPOLLDRVSTAT. Since 4.13 the compat path returned flags == 0, so
32-bit fdutils saw every disk as write-protected and never saw a
disk change.
Tested in QEMU with a floppy test suite (including the compat ioctls
from 32-bit userspace, and fdutils/mtools/dosfstools workflows) on
KASAN and KCSAN kernels, with no new warnings.
Signed-off-by: Denis Efremov (Oracle) <efremov@linux.com>
-----BEGIN PGP SIGNATURE-----
iJEEABYKADkWIQSxkVn8wiFDxFltLWIZHYv5MCjHVQUCasO9hBsUgAAAAAAEAA5t
YW51MiwyLjUrMS4xMiwyLDIACgkQGR2L+TAox1UInAEAyxLByH85coSOvVp34Y6C
5l6PI1p8c2e8awTuoZ2TP3UA/RUveFeRexNppvAezxyIsekR0p62YiqVpUIqPt4K
RHQA
=ANsv
-----END PGP SIGNATURE-----
Merge tag 'floppy-for-7.4' of https://github.com/evdenis/linux-floppy into for-7.4/block
Pull floppy fixes from Denis:
"Six fixes for long-standing floppy bugs, most of them found by syzbot
or by race detectors:
- Flush pending work before releasing the IRQ and DMA when no FDC is
found, from Karl Mehltretter. The work queued by the probe could run
after the IRQ and DMA were already freed ("work still pending").
- Unregister the platform device when add_disk() fails, from Guangshuo
Li. The failing drive's device stayed registered after the module was
unloaded, and reading its sysfs attributes oopsed on freed module
memory.
- Return early from reset_interrupt() when cont is already NULL, from
Yang Xiuwei. This fixes the syzbot GPF in reset_interrupt(). I
decided to take this v1 as a partial solution; the full fix will
require additional rework.
- Replace volatile on command_status with READ_ONCE()/WRITE_ONCE() and
annotate floppy_work_fn and current_type[], from Cen Zhang. No
functional change.
- Don't keep a pointer to a stack buffer in timeout_message.
show_floppy() printed it from the interrupt handler after
request_done() had returned (syzbot KASAN report).
- Copy the drive state flags in the 32-bit FDGETDRVSTAT and
FDPOLLDRVSTAT. Since 4.13 the compat path returned flags == 0, so
32-bit fdutils saw every disk as write-protected and never saw a disk
change.
Tested in QEMU with a floppy test suite (including the compat ioctls
from 32-bit userspace, and fdutils/mtools/dosfstools workflows) on
KASAN and KCSAN kernels, with no new warnings."
* tag 'floppy-for-7.4' of https://github.com/evdenis/linux-floppy:
floppy: return the drive state flags from the 32-bit FDGETDRVSTAT
floppy: don't store a stack buffer in timeout_message
floppy: annotate data-races around command_status and floppy_work_fn
floppy: unregister platform device on add_disk failure
floppy: avoid NULL deref in reset_interrupt when cont is cleared
floppy: flush pending work before releasing resources on init failure
This commit is contained in:
commit
b766aba70e
|
|
@ -500,7 +500,7 @@ static int probing;
|
|||
#define FD_COMMAND_ERROR 2
|
||||
#define FD_COMMAND_OKAY 3
|
||||
|
||||
static volatile int command_status = FD_COMMAND_NONE;
|
||||
static int command_status = FD_COMMAND_NONE;
|
||||
static unsigned long fdc_busy;
|
||||
static DECLARE_WAIT_QUEUE_HEAD(fdc_wait);
|
||||
static DECLARE_WAIT_QUEUE_HEAD(command_done);
|
||||
|
|
@ -599,7 +599,8 @@ static inline void fdc_outb(unsigned char value, int fdc, int reg)
|
|||
|
||||
static inline bool drive_no_geom(int drive)
|
||||
{
|
||||
return !current_type[drive] && !ITYPE(drive_state[drive].fd_device);
|
||||
return !READ_ONCE(current_type[drive]) &&
|
||||
!ITYPE(drive_state[drive].fd_device);
|
||||
}
|
||||
|
||||
#ifndef fd_eject
|
||||
|
|
@ -638,7 +639,7 @@ static const char *timeout_message;
|
|||
static void is_alive(const char *func, const char *message)
|
||||
{
|
||||
/* this routine checks whether the floppy driver is "alive" */
|
||||
if (test_bit(0, &fdc_busy) && command_status < 2 &&
|
||||
if (test_bit(0, &fdc_busy) && READ_ONCE(command_status) < 2 &&
|
||||
!delayed_work_pending(&fd_timeout)) {
|
||||
DPRINT("%s: timeout handler died. %s\n", func, message);
|
||||
}
|
||||
|
|
@ -890,7 +891,7 @@ static int lock_fdc(int drive)
|
|||
if (wait_event_interruptible(fdc_wait, !test_and_set_bit(0, &fdc_busy)))
|
||||
return -EINTR;
|
||||
|
||||
command_status = FD_COMMAND_NONE;
|
||||
WRITE_ONCE(command_status, FD_COMMAND_NONE);
|
||||
|
||||
reschedule_timeout(drive, "lock fdc");
|
||||
set_fdc(drive);
|
||||
|
|
@ -904,7 +905,7 @@ static void unlock_fdc(void)
|
|||
DPRINT("FDC access conflict!\n");
|
||||
|
||||
raw_cmd = NULL;
|
||||
command_status = FD_COMMAND_NONE;
|
||||
WRITE_ONCE(command_status, FD_COMMAND_NONE);
|
||||
cancel_delayed_work(&fd_timeout);
|
||||
do_floppy = NULL;
|
||||
cont = NULL;
|
||||
|
|
@ -988,7 +989,9 @@ static void (*floppy_work_fn)(void);
|
|||
|
||||
static void floppy_work_workfn(struct work_struct *work)
|
||||
{
|
||||
floppy_work_fn();
|
||||
void (*fn)(void) = READ_ONCE(floppy_work_fn);
|
||||
|
||||
fn();
|
||||
}
|
||||
|
||||
static DECLARE_WORK(floppy_work, floppy_work_workfn);
|
||||
|
|
@ -997,7 +1000,7 @@ static void schedule_bh(void (*handler)(void))
|
|||
{
|
||||
WARN_ON(work_pending(&floppy_work));
|
||||
|
||||
floppy_work_fn = handler;
|
||||
WRITE_ONCE(floppy_work_fn, handler);
|
||||
queue_work(floppy_wq, &floppy_work);
|
||||
}
|
||||
|
||||
|
|
@ -1784,6 +1787,9 @@ static void reset_interrupt(void)
|
|||
{
|
||||
debugt(__func__, "");
|
||||
result(current_fdc); /* get the status ready for set_fdc */
|
||||
/* Stale work: unlock_fdc()/do_wakeup() may have cleared cont. */
|
||||
if (!cont)
|
||||
return;
|
||||
if (fdc_state[current_fdc].reset) {
|
||||
pr_info("reset set in interrupt, calling %ps\n", cont->error);
|
||||
cont->error(); /* a reset just after a reset. BAD! */
|
||||
|
|
@ -1862,7 +1868,7 @@ static void show_floppy(int fdc)
|
|||
|
||||
pr_info("cont=%p\n", cont);
|
||||
pr_info("current_req=%p\n", current_req);
|
||||
pr_info("command_status=%d\n", command_status);
|
||||
pr_info("command_status=%d\n", READ_ONCE(command_status));
|
||||
pr_info("\n");
|
||||
}
|
||||
|
||||
|
|
@ -1989,7 +1995,7 @@ static void do_wakeup(void)
|
|||
{
|
||||
reschedule_timeout(MAXTIMEOUT, "do wakeup");
|
||||
cont = NULL;
|
||||
command_status += 2;
|
||||
WRITE_ONCE(command_status, READ_ONCE(command_status) + 2);
|
||||
wake_up(&command_done);
|
||||
}
|
||||
|
||||
|
|
@ -2017,11 +2023,12 @@ static int wait_til_done(void (*handler)(void), bool interruptible)
|
|||
schedule_bh(handler);
|
||||
|
||||
if (interruptible)
|
||||
wait_event_interruptible(command_done, command_status >= 2);
|
||||
wait_event_interruptible(command_done,
|
||||
READ_ONCE(command_status) >= 2);
|
||||
else
|
||||
wait_event(command_done, command_status >= 2);
|
||||
wait_event(command_done, READ_ONCE(command_status) >= 2);
|
||||
|
||||
if (command_status < 2) {
|
||||
if (READ_ONCE(command_status) < 2) {
|
||||
cancel_activity();
|
||||
cont = &intr_cont;
|
||||
reset_fdc();
|
||||
|
|
@ -2029,18 +2036,18 @@ static int wait_til_done(void (*handler)(void), bool interruptible)
|
|||
}
|
||||
|
||||
if (fdc_state[current_fdc].reset)
|
||||
command_status = FD_COMMAND_ERROR;
|
||||
if (command_status == FD_COMMAND_OKAY)
|
||||
WRITE_ONCE(command_status, FD_COMMAND_ERROR);
|
||||
if (READ_ONCE(command_status) == FD_COMMAND_OKAY)
|
||||
ret = 0;
|
||||
else
|
||||
ret = -EIO;
|
||||
command_status = FD_COMMAND_NONE;
|
||||
WRITE_ONCE(command_status, FD_COMMAND_NONE);
|
||||
return ret;
|
||||
}
|
||||
|
||||
static void generic_done(int result)
|
||||
{
|
||||
command_status = result;
|
||||
WRITE_ONCE(command_status, result);
|
||||
cont = &wakeup_cont;
|
||||
}
|
||||
|
||||
|
|
@ -2276,11 +2283,10 @@ static void request_done(int uptodate)
|
|||
{
|
||||
struct request *req = current_req;
|
||||
int block;
|
||||
char msg[sizeof("request done ") + sizeof(int) * 3];
|
||||
|
||||
probing = 0;
|
||||
snprintf(msg, sizeof(msg), "request done %d", uptodate);
|
||||
reschedule_timeout(MAXTIMEOUT, msg);
|
||||
reschedule_timeout(MAXTIMEOUT,
|
||||
uptodate ? "request done 1" : "request done 0");
|
||||
|
||||
if (!req) {
|
||||
pr_info("floppy.c: no request in request_done\n");
|
||||
|
|
@ -2871,7 +2877,7 @@ static blk_status_t floppy_queue_rq(struct blk_mq_hw_ctx *hctx,
|
|||
list_add_tail(&bd->rq->queuelist, &floppy_reqs);
|
||||
spin_unlock_irq(&floppy_lock);
|
||||
|
||||
command_status = FD_COMMAND_NONE;
|
||||
WRITE_ONCE(command_status, FD_COMMAND_NONE);
|
||||
__reschedule_timeout(MAXTIMEOUT, "fd_request");
|
||||
set_fdc(0);
|
||||
process_fd_request();
|
||||
|
|
@ -3601,7 +3607,7 @@ struct compat_floppy_drive_params {
|
|||
};
|
||||
|
||||
struct compat_floppy_drive_struct {
|
||||
signed char flags;
|
||||
compat_ulong_t flags;
|
||||
compat_ulong_t spinup_date;
|
||||
compat_ulong_t select_date;
|
||||
compat_ulong_t first_read_date;
|
||||
|
|
@ -3785,6 +3791,7 @@ static int compat_getdrvstat(int drive, bool poll,
|
|||
goto Eintr;
|
||||
process_fd_request();
|
||||
}
|
||||
v.flags = drive_state[drive].flags;
|
||||
v.spinup_date = drive_state[drive].spinup_date;
|
||||
v.select_date = drive_state[drive].select_date;
|
||||
v.first_read_date = drive_state[drive].first_read_date;
|
||||
|
|
@ -4729,8 +4736,11 @@ static int __init do_floppy_init(void)
|
|||
|
||||
err = device_add_disk(&floppy_device[drive].dev,
|
||||
disks[drive][0], NULL);
|
||||
if (err)
|
||||
if (err) {
|
||||
platform_device_unregister(&floppy_device[drive]);
|
||||
registered[drive] = false;
|
||||
goto out_remove_drives;
|
||||
}
|
||||
}
|
||||
|
||||
return 0;
|
||||
|
|
@ -4744,6 +4754,8 @@ static int __init do_floppy_init(void)
|
|||
}
|
||||
}
|
||||
out_release_dma:
|
||||
/* Drain the queued redo_fd_request() before releasing IRQ and DMA. */
|
||||
flush_workqueue(floppy_wq);
|
||||
if (atomic_read(&usage_count))
|
||||
floppy_release_irq_and_dma();
|
||||
out_unreg_driver:
|
||||
|
|
|
|||
Loading…
Reference in New Issue
Block a user