All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jens Axboe <axboe@kernel.dk>
To: Kyungtae Kim <kt0755@gmail.com>, jikos@kernel.org
Cc: Byoungyoung Lee <lifeasageek@gmail.com>,
	DaeRyong Jeong <threeearcat@gmail.com>,
	linux-block@vger.kernel.org, linux-kernel@vger.kernel.org,
	syzkaller@googlegroups.com
Subject: Re: UBSAN: Undefined behaviour in drivers/block/floppy.c:1495:32
Date: Tue, 23 Oct 2018 04:01:32 -0600	[thread overview]
Message-ID: <fcad3085-6801-71f9-b2cf-4c1702889d70@kernel.dk> (raw)
In-Reply-To: <CAEAjamtML1yMLL0DsV5JkD1H6P0Yg19F2DVq+_c-u09RaCKuDw@mail.gmail.com>

On 10/22/18 5:20 PM, Kyungtae Kim wrote:
> We report a bug found in v4.19-rc2 (v4.19-rc8 as well):
> UBSAN: Undefined behaviour in drivers/block/floppy.c:1495:32
> 
> kernel config: https://kt0755.github.io/etc/config_v2-4.19
> repro: https://kt0755.github.io/etc/repro.b4076.c
> 
> Analysis:
> 
> struct floppy_raw_cmd {
>    unsigned char cmd_count;
>    unsigned char cmd[16];
>   ...
> };
> 
> for (i=0; i<raw_cmd->cmd_count; i++)
>     output_byte(raw_cmd->cmd[i])
> 
> In driver/block/floppy.c:1495, the code snippet above is trying to
> write some bytes to the floppy disk controller, depending on "cmd_count".
> As you see "struct floppy_raw_cmd" above, the size of array “cmd” is
> fixed as 16.
> The thing is, there is no boundary check for the index of array "cmd"
> when this is used. Besides, "cmd_count" can be manipulated by raw_cmd_ioctl
> which is derived from ioctl system call.
> We observed that cmd_count is set at line 2540 (or 2111), but that is
> after such a bug arose in our experiment. So by manipulating system call ioctl,
> user program can have illegitimate memory access.
> 
> The following is a simple patch to stop this. (This might not be the
> best.)
> 
> diff --git a/linux-4.19-rc2/drivers/block/floppy.c
> b/linux-4.19-rc2/drivers/block/floppy.c
> index f2b6f4d..a3610c9 100644
> --- a/linux-4.19-rc2/drivers/block/floppy.c
> +++ b/linux-4.19-rc2/drivers/block/floppy.c
> @@ -3149,6 +3149,8 @@ static int raw_cmd_copyin(int cmd, void __user *param,
>                          */
>                 return -EINVAL;
> 
> +       if (ptr->cmd_count > ARRAY_SIZE(ptr->cmd)) {
> +               return -EINVAL;
> +
>         for (i = 0; i < 16; i++)
>                 ptr->reply[i] = 0;
>         ptr->resultcode = 0;

I think that's a decent way to fix it, but you probably want to
test your patch - it doesn't compile. Send something you've
tested that works.

-- 
Jens Axboe

  reply	other threads:[~2018-10-23 10:01 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-10-22 23:20 UBSAN: Undefined behaviour in drivers/block/floppy.c:1495:32 Kyungtae Kim
2018-10-23 10:01 ` Jens Axboe [this message]
2018-10-24  6:29   ` Kyungtae Kim
2018-10-24  6:29     ` Kyungtae Kim
2018-10-24  6:33     ` Kyungtae Kim
2018-10-24  6:33       ` Kyungtae Kim
2018-10-24  9:27       ` Jens Axboe
2018-10-26 13:22         ` Kyungtae Kim
2018-10-26 14:20           ` Jens Axboe
2019-08-09 13:40 ` Denis Efremov

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=fcad3085-6801-71f9-b2cf-4c1702889d70@kernel.dk \
    --to=axboe@kernel.dk \
    --cc=jikos@kernel.org \
    --cc=kt0755@gmail.com \
    --cc=lifeasageek@gmail.com \
    --cc=linux-block@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=syzkaller@googlegroups.com \
    --cc=threeearcat@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.