On Wed, Jun 27, 2018 at 10:44:25AM +0300, Leon Romanovsky wrote:
> From: Leon Romanovsky <leonro@xxxxxxxxxxxx>
>
> User can supply negative blue flame index and cause to "bfregn >
> bfregi->num_dyn_bfregs" protection check return "false". It will
> cause to below error while trying to access sys_pages[].
>
> BUG: KASAN: slab-out-of-bounds in bfregn_to_uar_index+0x34f/0x400
> Read of size 4 at addr ffff880065561904 by task syz-executor446/314
>
> CPU: 0 PID: 314 Comm: syz-executor446 Not tainted 4.18.0-rc1+ #256
> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.11.0-0-g63451fca13-prebuilt.qemu-project.org 04/01/2014
> Call Trace:
> dump_stack+0xef/0x17e
> print_address_description+0x83/0x3b0
> kasan_report+0x18d/0x4d0
> bfregn_to_uar_index+0x34f/0x400
> create_user_qp+0x272/0x227d
> create_qp_common+0x32eb/0x43e0
> mlx5_ib_create_qp+0x379/0x1ca0
> create_qp.isra.5+0xc94/0x22d0
> ib_uverbs_create_qp+0x21b/0x2a0
> ib_uverbs_write+0xc2c/0x1010
> vfs_write+0x1b0/0x550
> ksys_write+0xc6/0x1a0
> do_syscall_64+0xa7/0x590
> entry_SYSCALL_64_after_hwframe+0x49/0xbe
> RIP: 0033:0x433679
> Code: fd ff 48 81 c4 80 00 00 00 e9 f1 fe ff ff 0f 1f 00 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 0f 83 3b 91 fd ff c3 66 2e 0f 1f 84 00 00 00 00
> RSP: 002b:00007fff2b3d8e48 EFLAGS: 00000217 ORIG_RAX: 0000000000000001
> RAX: ffffffffffffffda RBX: 00000000004002f8 RCX: 0000000000433679
> RDX: 0000000000000040 RSI: 0000000020000240 RDI: 0000000000000003
> RBP: 00000000006d4018 R08: 00000000004002f8 R09: 00000000004002f8
> R10: 00000000004002f8 R11: 0000000000000217 R12: 0000000000000000
> R13: 000000000040cb00 R14: 000000000040cb90 R15: 0000000000000006
>
> Allocated by task 314:
> kasan_kmalloc+0xa0/0xd0
> __kmalloc+0x1a9/0x510
> mlx5_ib_alloc_ucontext+0x966/0x2620
> ib_uverbs_get_context+0x23f/0xa60
> ib_uverbs_write+0xc2c/0x1010
> __vfs_write+0x10d/0x720
> vfs_write+0x1b0/0x550
> ksys_write+0xc6/0x1a0
> do_syscall_64+0xa7/0x590
> entry_SYSCALL_64_after_hwframe+0x49/0xbe
>
> Freed by task 1:
> __kasan_slab_free+0x12e/0x180
> kfree+0x159/0x630
> kvfree+0x37/0x50
> single_release+0x8e/0xf0
> __fput+0x2d8/0x900
> task_work_run+0x102/0x1f0
> exit_to_usermode_loop+0x159/0x1c0
> do_syscall_64+0x408/0x590
> entry_SYSCALL_64_after_hwframe+0x49/0xbe
>
> The buggy address belongs to the object at ffff880065561100
> which belongs to the cache kmalloc-4096 of size 4096
> The buggy address is located 2052 bytes inside of
> 4096-byte region [ffff880065561100, ffff880065562100)
> The buggy address belongs to the page:
> page:ffffea0001955800 count:1 mapcount:0 mapping:ffff88006c402480 index:0x0 compound_mapcount: 0
> flags: 0x4000000000008100(slab|head)
> raw: 4000000000008100 ffffea0001a7c000 0000000200000002 ffff88006c402480
> raw: 0000000000000000 0000000080070007 00000001ffffffff 0000000000000000
> page dumped because: kasan: bad access detected
>
> Memory state around the buggy address:
> ffff880065561800: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> ffff880065561880: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> >ffff880065561900: 04 fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
> ^
> ffff880065561980: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
> ffff880065561a00: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
>
> Cc: <stable@xxxxxxxxxxxxxxx> # 4.15
> Fixes: 1ee47ab3e8d8 ("IB/mlx5: Enable QP creation with a given blue flame index")
> Reported-by: Noa Osherovich <noaos@xxxxxxxxxxxx>
> Signed-off-by: Leon Romanovsky <leonro@xxxxxxxxxxxx>
> drivers/infiniband/hw/mlx5/qp.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/drivers/infiniband/hw/mlx5/qp.c b/drivers/infiniband/hw/mlx5/qp.c
> index 5471b57b873d..6b0034c063f4 100644
> +++ b/drivers/infiniband/hw/mlx5/qp.c
> @@ -653,6 +653,9 @@ int bfregn_to_uar_index(struct mlx5_ib_dev *dev,
> int index_of_sys_page;
> int offset;
>
> + if (bfregn > MLX5_MAX_BFREGS)
> + return -EINVAL;
This is not enough, you should make 'int bfregn' into 'unsigned int
bfregn'.
Because here we do:
uar_index = bfregn_to_uar_index(dev, &context->bfregi,
ucmd.bfreg_index, true);
And
__u32 bfreg_index;
So, userspace could pass in 1<<31 and still pass (bfregn > MLX5_MAX_BFREGS)
after all the casting is worked out.
And during code review people need to understand that blindly using
'int' because it is 'short to type' leads to defeating security checks
like the above.
Values that are not expected to be unsigned should be declared as
unsigned!
Jason
> +
> bfregs_per_sys_page = get_uars_per_sys_page(dev, bfregi->lib_uar_4k) *
> MLX5_NON_FP_BFREGS_PER_UAR;
> index_of_sys_page = bfregn / bfregs_per_sys_page;
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
