mirror of
https://github.com/torvalds/linux.git
synced 2026-11-07 06:17:02 +01:00
floppy: annotate data-races around command_status and floppy_work_fn
command_status is accessed by multiple contexts:
- generic_done() and do_wakeup() write it from the floppy
workqueue context.
- wait_til_done() reads it in wait_event conditions without
holding fdc_busy.
- is_alive() reads it locklessly to check driver liveness.
- floppy_queue_rq(), lock_fdc(), and unlock_fdc() write it
during FDC ownership transitions.
There is currently no LKMM annotation for these concurrent accesses
since command_status relies on the deprecated volatile qualifier.
Remove volatile and use READ_ONCE()/WRITE_ONCE() on every access
to command_status to provide proper LKMM data-race annotations.
Also annotate floppy_work_fn with WRITE_ONCE() in schedule_bh()
and READ_ONCE() in floppy_work_workfn(), and current_type[drive]
with READ_ONCE() in drive_no_geom() where it can be read without
holding the FDC lock.
The races were found by our tools:
1) command_status: generic_done (write) vs wait_til_done (read)
============ DATARACE ============
Function: generic_done+0x54/0xa0 drivers/block/floppy.c:985
Function: success_and_wakeup+0xeb/0x1e0 drivers/block/floppy.c:2046
Function: setup_rw_floppy+0x15dc/0x1d80 drivers/block/floppy.c:1046
Function: floppy_ready+0x2630/0x4000
Function: floppy_work_workfn+0x56/0x70 drivers/block/floppy.c:1012
Function: process_one_work kernel/workqueue.c:3236 [inline]
Function: process_scheduled_works+0x7a8/0x1030 kernel/workqueue.c:3319
============OTHER_INFO============
Function: wait_til_done+0x134/0x740 drivers/block/floppy.c:2026
Function: poll_drive+0x1fa/0x2a0
Function: fd_ioctl+0x13c8/0x1da0 drivers/block/floppy.c:3873
Function: blkdev_ioctl+0x421/0x510 block/ioctl.c:705
Function: vfs_ioctl fs/ioctl.c:51 [inline]
Function: __do_sys_ioctl fs/ioctl.c:598 [inline]
Function: __se_sys_ioctl+0xfc/0x170 fs/ioctl.c:584
=================END==============
2) current_type[drive]: drive_no_geom (read) vs set_geometry (write)
============ DATARACE ============
Function: drive_no_geom drivers/block/floppy.c:606 [inline]
Function: floppy_check_events+0x9c9/0xcf0 drivers/block/floppy.c:4187
Function: disk_check_events+0xca/0x4d0 block/disk-events.c:193
Function: process_one_work kernel/workqueue.c:3236 [inline]
Function: process_scheduled_works+0x7a8/0x1030 kernel/workqueue.c:3319
Function: worker_thread+0xb97/0x11d0 kernel/workqueue.c:3400
Function: kthread+0x3d4/0x800 kernel/kthread.c:463
============OTHER_INFO============
Function: set_geometry+0xe1d/0x1980 drivers/block/floppy.c:2237
Function: fd_ioctl+0xff4/0x1da0 drivers/block/floppy.c:3912
Function: blkdev_ioctl+0x421/0x510 block/ioctl.c:705
Function: vfs_ioctl fs/ioctl.c:51 [inline]
Function: __do_sys_ioctl fs/ioctl.c:598 [inline]
Function: __se_sys_ioctl+0xfc/0x170 fs/ioctl.c:584
=================END==============
3) floppy_work_fn: schedule_bh (write) vs
floppy_interrupt->schedule_bh (write)
============ DATARACE ============
Function: schedule_bh drivers/block/floppy.c:1005 [inline]
Function: do_wakeup+0x19c/0x220 drivers/block/floppy.c:3219
Function: success_and_wakeup+0x192/0x1e0 drivers/block/floppy.c:1994
Function: setup_rw_floppy+0x15dc/0x1d80 drivers/block/floppy.c:1046
Function: floppy_ready+0x2630/0x4000
Function: floppy_work_workfn+0x56/0x70 drivers/block/floppy.c:1012
Function: process_one_work kernel/workqueue.c:3236 [inline]
Function: process_scheduled_works+0x7a8/0x1030 kernel/workqueue.c:3319
============OTHER_INFO============
Function: floppy_interrupt+0xbf4/0x1220
Function: floppy_hardint+0x432/0x630
Function: __handle_irq_event_percpu+0x10a/0x5d0 kernel/irq/handle.c:158
Function: handle_irq_event+0x8b/0x1e0 kernel/irq/handle.c:210
Function: handle_edge_irq+0x223/0x9f0 kernel/irq/chip.c:855
=================END==============
Signed-off-by: Cen Zhang <zzzccc427@gmail.com>
Link: https://patch.msgid.link/20260318034810.3089217-1-zzzccc427@gmail.com
Signed-off-by: Denis Efremov (Oracle) <efremov@linux.com>
This commit is contained in:
parent
f1e72d80ad
commit
9dfafe7ed0
|
|
@ -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);
|
||||
}
|
||||
|
||||
|
|
@ -1865,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");
|
||||
}
|
||||
|
||||
|
|
@ -1992,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);
|
||||
}
|
||||
|
||||
|
|
@ -2020,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();
|
||||
|
|
@ -2032,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;
|
||||
}
|
||||
|
||||
|
|
@ -2874,7 +2878,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();
|
||||
|
|
|
|||
Loading…
Reference in New Issue
Block a user