From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1759221AbcDAPCJ (ORCPT ); Fri, 1 Apr 2016 11:02:09 -0400 Received: from mx2.suse.de ([195.135.220.15]:39081 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752360AbcDAPCI (ORCPT ); Fri, 1 Apr 2016 11:02:08 -0400 From: Takashi Iwai To: Al Viro Cc: Jiri Slaby , Andrew Morton , linux-kernel@vger.kernel.org Subject: [PATCH v2] iov_iter: Fix out-of-bound access in iov_iter_advance() Date: Fri, 1 Apr 2016 17:02:04 +0200 Message-Id: <1459522924-17720-1-git-send-email-tiwai@suse.de> X-Mailer: git-send-email 2.7.4 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Currently, iov_iter_advance() just calls iterate_and_advance() macro as is, even if size=0 is passed. Usually it is OK to pass size=0 to the macro. However, when the iov_iter has been already advanced to the end of the array, it may lead to an out-of-bound access, since the macro always reads the length of the vector at first. This bug is actually seen via KASAN with net tun driver, for example. BUG: KASAN: stack-out-of-bounds in iov_iter_advance+0x510/0x540 at addr ffff88003d5efd40 Read of size 8 by task syz-executor/22356 page:ffffea0000f57bc0 count:0 mapcount:0 mapping: (null) index:0x0 flags: 0x1fffff80000000() page dumped because: kasan: bad access detected CPU: 0 PID: 22356 Comm: syz-executor Tainted: G W E 4.4.6-0-default #1 Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.8.1-0-g4adadbd-20151112_172657-sheep25 04/01/2014 0000000000000000 ffff88003d5ef9d0 ffffffff819f42c1 ffff88003d5efa68 ffff88003d5efd40 0000000000000000 ffff88003d5efd38 ffff88003d5efa58 ffffffff815f7267 000000000000000a ffff88003d5efad8 0000000000000296 Call Trace: [] ? dump_stack+0xb3/0x112 [] ? kasan_report_error+0x507/0x540 [] ? __might_fault+0x3f/0x50 [] ? __asan_report_load8_noabort+0x43/0x50 [] ? iov_iter_advance+0x510/0x540 [] ? iov_iter_advance+0x510/0x540 [] ? tun_get_user+0x745/0x21a0 [tun] [] ? debug_check_no_locks_freed+0x290/0x290 [] ? tun_select_queue+0x370/0x370 [tun] [] ? futex_wake+0x149/0x420 [] ? debug_lockdep_rcu_enabled+0x77/0x90 [] ? __tun_get+0x5/0x220 [tun] [] ? __tun_get+0x121/0x220 [tun] [] ? tun_chr_write_iter+0xda/0x190 [tun] [] ? __vfs_write+0x30a/0x480 [] ? vfs_iter_write+0x320/0x320 [] ? debug_lockdep_rcu_enabled+0x77/0x90 [] ? common_file_perm+0x158/0x7a0 [] ? apparmor_file_permission+0x27/0x30 [] ? rw_verify_area+0x105/0x2f0 [] ? vfs_write+0x16c/0x4a0 [] ? SyS_write+0x11a/0x230 This patch adds the proper check of the size to iov_iter_advance(), like all other functions calling iterate_and_advance() macro. Reported-by: Jiri Slaby Signed-off-by: Takashi Iwai --- We can put these checks in iterate_and_advance(), too. I chose this patch since it's smaller, and doing in the macro will be a bit ugly. Let me know if you prefer another option. v1->v2: Fix the bogus return value lib/iov_iter.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/lib/iov_iter.c b/lib/iov_iter.c index 5fecddc32b1b..2545c31fa0de 100644 --- a/lib/iov_iter.c +++ b/lib/iov_iter.c @@ -508,6 +508,10 @@ EXPORT_SYMBOL(iov_iter_copy_from_user_atomic); void iov_iter_advance(struct iov_iter *i, size_t size) { + if (unlikely(size > i->count)) + size = i->count; + if (unlikely(!size)) + return; iterate_and_advance(i, size, v, 0, 0, 0) } EXPORT_SYMBOL(iov_iter_advance); -- 2.7.4