Re: [PATCH 16/31] swim: Fix buffer overflow

Le 16/07/2026 à 12:02, Finn Thain a écrit :
> The effect of this bug can be observed as swim_read_sector_data()
> inexplicably returning -5, or an error flag indicating that a mark byte
> was read from the data register, or other odd behviour.
> 
> When copying bytes from the chip FIFO to the read buffer, the driver
> keeps count of the remaining buffer space using register %d4. A counter
> in register %d2 serves as a timeout. The driver polls (%a2), the handshake
> register, until flags indicate that byte(s) have arrived in the FIFO.
> 
>          movel   #sector_size-1, %d4
> read_new_data:
>          movew   #max_retry, %d2
> read_data_loop:
>          moveb   %a2@, %d5
>          andb    #0xc0, %d5
>          dbne    %d2, read_data_loop
>          beq     data_exit
>          moveb   %a5@, %a4@+
>          andb    #0x40, %d5
>          dbne    %d4, read_new_data
>          beq     exit_loop
> 
> Note that the exit_loop branch depends upon a flag in the handshake
> register and not on the remaining buffer space. Hence there may be no
> branch to exit_loop after %d4 is decremented to -1 (i.e. full buffer).
> 
>          moveb   %a5@, %a4@+
>          dbra    %d4, read_new_data
> exit_loop:
> 
> Here is a second decrement of %d4 which can now reach -2. But the buffer
> bounds check is a comparison with -1, which is now ineffective. Hence the
> loop will continue copying until %d2 eventually reaches -1.
> 
> Fix this bug by terminating the loop as soon as %d4 or %d2 reach -1.
> Reset the timeout whenever a byte is copied.
> 
> Fixes: 8852ecd97488 ("m68k: mac - Add SWIM floppy support")
> Signed-off-by: Finn Thain <[email protected]>
> ---
>   drivers/block/swim_asm.S | 15 +++++++++------
>   1 file changed, 9 insertions(+), 6 deletions(-)

Reviewed-by: Laurent Vivier <[email protected]>

> 
> diff --git a/drivers/block/swim_asm.S b/drivers/block/swim_asm.S
> index 5caeba3a75d8..ae767da671e3 100644
> --- a/drivers/block/swim_asm.S
> +++ b/drivers/block/swim_asm.S
> @@ -43,6 +43,8 @@
>   	.equ	sector_size, 512
>   
>   	.equ	.Lhr_crc_error,		0x02
> +	.equ	.Lhr_fifo_2bytes,	0x40
> +	.equ	.Lhr_fifo_1byte,	0x80
>   
>   	.global swim_read_sector_header
>   swim_read_sector_header:
> @@ -192,16 +194,17 @@ read_new_data:
>   	movew	#max_retry, %d2
>   read_data_loop:
>   	moveb	%a2@, %d5
> -	andb	#0xc0, %d5
> +	andb	#(.Lhr_fifo_2bytes + .Lhr_fifo_1byte), %d5
> +	beq	1f
> +	movew	#max_retry, %d2
> +	moveb	%a5@, %a4@+
> +	dbra	%d4, 1f
> +	bra	data_crc0
> +1:	andb	#.Lhr_fifo_2bytes, %d5
>   	dbne	%d2, read_data_loop
>   	beq	data_exit
>   	moveb	%a5@, %a4@+
> -	andb	#0x40, %d5
> -	dbne	%d4, read_new_data
> -	beq	exit_loop
> -	moveb	%a5@, %a4@+
>   	dbra	%d4, read_new_data
> -exit_loop:
>   
>   	/* read CRC */
>