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

Laurent Vivier <[email protected]> Tue, 21 Jul 2026 07:59:52 +0200
Newsgroups org.kernel.vger.linux-m68k,org.kernel.vger.linux-block,org.kernel.vger.linux-kernel
Message-ID <[email protected]>
Le 16/07/2026 =C3=A0 12:02, Finn Thain a =C3=A9crit=C2=A0:
> 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.
>=20
> 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 handsha=
ke
> register, until flags indicate that byte(s) have arrived in the FIFO.
>=20
>          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
>=20
> 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).
>=20
>          moveb   %a5@, %a4@+
>          dbra    %d4, read_new_data
> exit_loop:
>=20
> 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 th=
e
> loop will continue copying until %d2 eventually reaches -1.
>=20
> Fix this bug by terminating the loop as soon as %d4 or %d2 reach -1.
> Reset the timeout whenever a byte is copied.
>=20
> 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]>

>=20
> 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
>  =20
>   	.equ	.Lhr_crc_error,		0x02
> +	.equ	.Lhr_fifo_2bytes,	0x40
> +	.equ	.Lhr_fifo_1byte,	0x80
>  =20
>   	.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:
>  =20
>   	/* read CRC */
>  =20