Re: [PATCH] floppy: avoid NULL deref in reset_interrupt when cont is cleared

["Denis Efremov (Oracle)" <efremov@linux.com>]

Hello,

Thank you for the patch.

On 23/07/2026 11:35, Yang Xiuwei wrote:
> reset_fdc() arms do_floppy = reset_interrupt; the IRQ handler then
> queues that function via schedule_bh(). If unlock_fdc() or do_wakeup()
> clears cont before the work runs, reset_interrupt() dereferences a NULL
> cont and oopses.
> 
> Example crash excerpt:
> 
> [ 1070.468148] status=80
> [ 1070.468152] fdc_busy=1
> [ 1070.468158] cont=          (null)
> [ 1070.468161] current_req=          (null)
> [ 1070.468162] command_status=-1
> [ 1070.468163]
> [ 1070.557051] floppy0: floppy timeout called
> [ 1070.557053] no cont in shutdown!
> [ 1070.557056] floppy0: floppy_shutdown: timeout handler died.
> [ 1074.419509] floppy0: FDC access conflict!
> [ 1074.419782] BUG: unable to handle kernel NULL pointer dereference at
> 0000000000000008
> [ 1074.421969] PGD 0 P4D 0
> [ 1074.422320] Oops: 0000 [#1] SMP NOPTI
> [ 1074.422648] CPU: 10 PID: 3269 Comm: kworker/u256:4 Kdump: loaded
> [ 1074.423830] Hardware name: inspur Standard PC (i440FX + PIIX, 1996),
> BIOS 0.0.0 02/06/2015
> [ 1074.424284] Workqueue: floppy floppy_work_workfn [floppy]
> [ 1074.424757] RIP: 0010:reset_interrupt+0x3a/0xa0 [floppy]
> 
> The same NULL deref has also been reported by syzbot on upstream.
> 
> Return early if cont is already NULL after result(). The hardware reset
> has completed; result() still drains the FDC. A later request can lock
> the FDC and reset again. This is a defensive fix for stale work, not a
> rewrite of the floppy continuation state machine.
> 
> Reported-by: syzbot+619e27617b2abe6b9b72@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=619e27617b2abe6b9b72
> Link: https://lore.kernel.org/linux-block/00000000000093c4d105f9aa34d6@google.com/
> Link: https://lists.openwall.net/linux-kernel/2021/10/27/56
> Signed-off-by: Yang Xiuwei <yangxiuwei@kylinos.cn>
> ---
>  drivers/block/floppy.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/block/floppy.c b/drivers/block/floppy.c
> index f04397b8e381..3d2df99dbb29 100644
> --- a/drivers/block/floppy.c
> +++ b/drivers/block/floppy.c
> @@ -1784,6 +1784,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;

It looks to me that this check closes the exact reported crash, but it doesn't
fix the root cause of the problem. I think cont can be set here with a valid
but unrelated continuation while stale reset work is inside result(). We need
a more generic solution to close the bug here.


>  	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! */


Thanks,
Denis