linux-kernel - Re: [PATCH 3/3] floppy: suppress UBSAN warning in setup_rw

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20200422070921.GA19116@infradead.org>
Date:   Wed, 22 Apr 2020 00:09:21 -0700
From:   Christoph Hellwig <hch@...radead.org>
To:     Denis Efremov <efremov@...ux.com>
Cc:     linux-block@...r.kernel.org, Willy Tarreau <w@....eu>,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH 3/3] floppy: suppress UBSAN warning in setup_rw_floppy()

On Tue, Apr 21, 2020 at 03:57:22PM +0300, Denis Efremov wrote:
> UBSAN: array-index-out-of-bounds in drivers/block/floppy.c:1521:45
> index 16 is out of range for type 'unsigned char [16]'
> Call Trace:
> ...
>  setup_rw_floppy+0x5c3/0x7f0
>  floppy_ready+0x2be/0x13b0
>  process_one_work+0x2c1/0x5d0
>  worker_thread+0x56/0x5e0
>  kthread+0x122/0x170
>  ret_from_fork+0x35/0x40
> 
> >From include/uapi/linux/fd.h:
> struct floppy_raw_cmd {
> 	...
> 	unsigned char cmd_count;
> 	unsigned char cmd[16];
> 	unsigned char reply_count;
> 	unsigned char reply[16];
> 	...
> }
> 
> This out-of-bounds access is intentional. The command in struct
> floppy_raw_cmd may take up the space initially intended for the reply
> and the reply count. It is needed for long 82078 commands such as
> RESTORE, which takes 17 command bytes. Initial cmd size is not enough
> and since struct setup_rw_floppy is a part of uapi we check that
> cmd_count is in [0:16+1+16] in raw_cmd_copyin().
> 
> The patch replaces array subscript with pointer arithetic to suppress
> UBSAN warning.

Urghh.  I think the better way would be to use a union that creates
a larger cmd field, or something like:

struct floppy_raw_cmd {
	...
	u8 buf[34];

#define BUF_CMD_COUNT	0
#define BUF_CMD		1
#define BUF_REPLY_COUNT	17
#define BUF_REPLY	18

and use addressing based on that.