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