From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id 757A4C3DA78 for ; Sun, 15 Jan 2023 14:09:08 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id C7DE46B0071; Sun, 15 Jan 2023 09:09:07 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id C08436B0072; Sun, 15 Jan 2023 09:09:07 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id A7FD86B0073; Sun, 15 Jan 2023 09:09:07 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0010.hostedemail.com [216.40.44.10]) by kanga.kvack.org (Postfix) with ESMTP id 941B96B0071 for ; Sun, 15 Jan 2023 09:09:07 -0500 (EST) Received: from smtpin20.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay03.hostedemail.com (Postfix) with ESMTP id 547FCA0302 for ; Sun, 15 Jan 2023 14:09:07 +0000 (UTC) X-FDA: 80357215134.20.4C942A5 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by imf11.hostedemail.com (Postfix) with ESMTP id AE5F74000D for ; Sun, 15 Jan 2023 14:09:04 +0000 (UTC) Authentication-Results: imf11.hostedemail.com; dkim=pass header.d=redhat.com header.s=mimecast20190719 header.b=bFoxhWsS; spf=pass (imf11.hostedemail.com: domain of bhe@redhat.com designates 170.10.133.124 as permitted sender) smtp.mailfrom=bhe@redhat.com; dmarc=pass (policy=none) header.from=redhat.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1673791745; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=uYsStTjUz3WGDaAYUWg24ZE611xTjkecNGKieBDCjA8=; b=Axq2V7+B3zZzu/aEmxjq44xpEiuLH4YE06xpBdGkn6FVL7iAWsrwRPjy/7wvkzx4OCc4wz 7p8sg+JIIQktwlflNS6yQ2gvF90wseyYh3RpdE//xRV1yu9+3Sa7TIGNHJqvpETZ86I7Uv 6qmk4erIPCJOsJxaBRqEm2bv1YNDRO8= ARC-Authentication-Results: i=1; imf11.hostedemail.com; dkim=pass header.d=redhat.com header.s=mimecast20190719 header.b=bFoxhWsS; spf=pass (imf11.hostedemail.com: domain of bhe@redhat.com designates 170.10.133.124 as permitted sender) smtp.mailfrom=bhe@redhat.com; dmarc=pass (policy=none) header.from=redhat.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1673791745; a=rsa-sha256; cv=none; b=m3CHdZx/jXZOGBZrP/lmx4i9AiulhIzlEURPGZAVK9ncQRJdqNwjXD09EdbkfV0MRssOLv ClqSczZo5qj2MiAKCbJnUjjaX2AopsvA78Sr9tWlXbGTQZo3wRQqNT4v4mr5YF+HlSiTk6 XD3T6r3wseoafQhEyOPIGze5yyTy6gc= DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1673791744; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=uYsStTjUz3WGDaAYUWg24ZE611xTjkecNGKieBDCjA8=; b=bFoxhWsS+rDET9/valItO8vO9jpf9zErVtsDA5q6JW8OuScoXYsT9+oAa4LwB79PRewMFC yhcFz4m6F9OA7d9kcVA7rUxwMXBnlRJcFd6gK3SAcP4DU7lDkJ6Zv8dH2wc3EISi5ci8B4 NX2bxEOtSiiE9NGgdFFF0/oF594WMOo= Received: from mimecast-mx02.redhat.com (mx3-rdu2.redhat.com [66.187.233.73]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-486-Ziwo1vRyNbeAEKupitVnJw-1; Sun, 15 Jan 2023 09:09:00 -0500 X-MC-Unique: Ziwo1vRyNbeAEKupitVnJw-1 Received: from smtp.corp.redhat.com (int-mx02.intmail.prod.int.rdu2.redhat.com [10.11.54.2]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id 08B351C07543; Sun, 15 Jan 2023 14:09:00 +0000 (UTC) Received: from localhost (ovpn-12-229.pek2.redhat.com [10.72.12.229]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 16FD640C6EC4; Sun, 15 Jan 2023 14:08:58 +0000 (UTC) Date: Sun, 15 Jan 2023 22:08:55 +0800 From: Baoquan He To: Dan Carpenter Cc: oe-kbuild@lists.linux.dev, linux-mm@kvack.org, lkp@intel.com, oe-kbuild-all@lists.linux.dev, linux-kernel@vger.kernel.org, urezki@gmail.com, lstoakes@gmail.com, stephen.s.brennan@oracle.com, willy@infradead.org, akpm@linux-foundation.org, hch@infradead.org Subject: Re: [PATCH v3 3/7] mm/vmalloc.c: allow vread() to read out vm_map_ram areas Message-ID: References: <20230113031921.64716-4-bhe@redhat.com> <202301132345.KVjvHMFq-lkp@intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <202301132345.KVjvHMFq-lkp@intel.com> X-Scanned-By: MIMEDefang 3.1 on 10.11.54.2 X-Rspamd-Server: rspam05 X-Rspamd-Queue-Id: AE5F74000D X-Stat-Signature: 8b381efk5shq7bgsoxb9q7y7bwwg16wb X-Rspam-User: X-HE-Tag: 1673791744-605659 X-HE-Meta: U2FsdGVkX18Z7ntMzVzlnPE2Hsn6Pl8h54rVkL6Cf7YlOrulYVdvMpppsYGZJaGKR41oL2aHSI4T9dgcWPuifUfXVw7bI/jRfncVdZkteKKJZjVsUV3uuQvNmERYXgiCwioh33NPcS8QjbHQjX8wcfWVhRjAol86YK7gMaPBRIeipD8MMg81PeMgxpaeDERjTp+7+ZBihObCQlmZPL/poGMxW+o0R+QfX6rRijzOlJnqXUFuhy2xxqu76LszJwqE8iihYUV/V2w0NgfJuBzaqVp59Azp00lU6a7nQOMTYEgprPf2EW5sB0pk7hvXfGbte+HwRlNHlNrAF+RmhbnvY+XnDrk1IUEdD24j9Z4HpOaGr15WtoCdqAR7v9SYfrVAy7M75Tt5w3TZ8rZXAz+nhstUzNVNKHUaLkyDavSP1YIThMfmfBuS76dQSX89HAevDiCDjy7zvGbo3GixYk+SM5MYfIu8JAnh6/yGxv+VHWiDy6Llh6APnCKje1SrGSnpmiGFSdbQk+cBc7scQ6SfntDEeA8F9HopSt88XF294flx/8CBRckdP53/lSH2uWS6dLbKnNuCJK+OTnGJ5bkY9GI0Ole/JrjkurH6z3oO0PIEfZnkYpwnSKb2NWhuYC/Yhz7s43hcuHDOA1V+c+gnRYRCJrOaZngVBRt5RgGT5q4vn2Gt45VUyG0u65u8HNxw/YHWGDA8y5lWIXr+k/ZLButmKlM4oY169ox0K1/WLMmwPLmENcJqD1sGmelRTq0QLRatJccM/T3y0pWWgHHnW7VTeq7Fvnv3oVaWLmlRfXuqjswW+kVc1Yhkf/NirKmcR+lURjASjQjGMG5qJp8CoW83eNsJvcRoNfkVQL8yzGNHUDBkRI/Yr4roA+r9OEV3Jfpy2rk1IsucgwfE10CgERHY0M+QWEUx4cVlGjmWjR4= X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: Hi Dan, On 01/14/23 at 10:57am, Dan Carpenter wrote: > Hi Baoquan, > > url: https://github.com/intel-lab-lkp/linux/commits/Baoquan-He/mm-vmalloc-c-add-used_map-into-vmap_block-to-track-space-of-vmap_block/20230113-112149 > base: https://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm.git mm-everything > patch link: https://lore.kernel.org/r/20230113031921.64716-4-bhe%40redhat.com > patch subject: [PATCH v3 3/7] mm/vmalloc.c: allow vread() to read out vm_map_ram areas > config: i386-randconfig-m021 > compiler: gcc-11 (Debian 11.3.0-8) 11.3.0 > > If you fix the issue, kindly add following tag where applicable > | Reported-by: kernel test robot > | Reported-by: Dan Carpenter > > smatch warnings: > mm/vmalloc.c:3682 vread() error: we previously assumed 'vm' could be null (see line 3664) Thanks for checking this. I went through the code flow again, personally think that the issue and risk pointed out could not exist. Please see the comment at below. > > vim +/vm +3682 mm/vmalloc.c > > ^1da177e4c3f415 Linus Torvalds 2005-04-16 3630 long vread(char *buf, char *addr, unsigned long count) > ^1da177e4c3f415 Linus Torvalds 2005-04-16 3631 { > e81ce85f960c2e2 Joonsoo Kim 2013-04-29 3632 struct vmap_area *va; > e81ce85f960c2e2 Joonsoo Kim 2013-04-29 3633 struct vm_struct *vm; > ^1da177e4c3f415 Linus Torvalds 2005-04-16 3634 char *vaddr, *buf_start = buf; > d0107eb07320b5d KAMEZAWA Hiroyuki 2009-09-21 3635 unsigned long buflen = count; > 129dbdf298d7383 Baoquan He 2023-01-13 3636 unsigned long n, size, flags; > ^1da177e4c3f415 Linus Torvalds 2005-04-16 3637 > 4aff1dc4fb3a5a3 Andrey Konovalov 2022-03-24 3638 addr = kasan_reset_tag(addr); > 4aff1dc4fb3a5a3 Andrey Konovalov 2022-03-24 3639 > ^1da177e4c3f415 Linus Torvalds 2005-04-16 3640 /* Don't allow overflow */ > ^1da177e4c3f415 Linus Torvalds 2005-04-16 3641 if ((unsigned long) addr + count < count) > ^1da177e4c3f415 Linus Torvalds 2005-04-16 3642 count = -(unsigned long) addr; > ^1da177e4c3f415 Linus Torvalds 2005-04-16 3643 > e81ce85f960c2e2 Joonsoo Kim 2013-04-29 3644 spin_lock(&vmap_area_lock); > f181234a5a21fd0 Chen Wandun 2021-09-02 3645 va = find_vmap_area_exceed_addr((unsigned long)addr); > f608788cd2d6cae Serapheim Dimitropoulos 2021-04-29 3646 if (!va) > f608788cd2d6cae Serapheim Dimitropoulos 2021-04-29 3647 goto finished; > f181234a5a21fd0 Chen Wandun 2021-09-02 3648 > f181234a5a21fd0 Chen Wandun 2021-09-02 3649 /* no intersects with alive vmap_area */ > f181234a5a21fd0 Chen Wandun 2021-09-02 3650 if ((unsigned long)addr + count <= va->va_start) > f181234a5a21fd0 Chen Wandun 2021-09-02 3651 goto finished; > f181234a5a21fd0 Chen Wandun 2021-09-02 3652 > f608788cd2d6cae Serapheim Dimitropoulos 2021-04-29 3653 list_for_each_entry_from(va, &vmap_area_list, list) { > e81ce85f960c2e2 Joonsoo Kim 2013-04-29 3654 if (!count) > e81ce85f960c2e2 Joonsoo Kim 2013-04-29 3655 break; > e81ce85f960c2e2 Joonsoo Kim 2013-04-29 3656 > 129dbdf298d7383 Baoquan He 2023-01-13 3657 vm = va->vm; > 129dbdf298d7383 Baoquan He 2023-01-13 3658 flags = va->flags & VMAP_FLAGS_MASK; > 129dbdf298d7383 Baoquan He 2023-01-13 3659 > 129dbdf298d7383 Baoquan He 2023-01-13 3660 if (!vm && !flags) > ^^^ > vm can be NULL if a flag in VMAP_FLAGS_MASK is set. > > e81ce85f960c2e2 Joonsoo Kim 2013-04-29 3661 continue; Right, after the 'continue;' line, only two cases could happen when it comes here. (vm != null) or (vm->flags & VMAP_RAM) is true. > e81ce85f960c2e2 Joonsoo Kim 2013-04-29 3662 > 129dbdf298d7383 Baoquan He 2023-01-13 3663 vaddr = (char *) va->va_start; > 129dbdf298d7383 Baoquan He 2023-01-13 @3664 size = vm ? get_vm_area_size(vm) : va_size(va); > ^^ > > 129dbdf298d7383 Baoquan He 2023-01-13 3665 > 129dbdf298d7383 Baoquan He 2023-01-13 3666 if (addr >= vaddr + size) > ^1da177e4c3f415 Linus Torvalds 2005-04-16 3667 continue; > ^1da177e4c3f415 Linus Torvalds 2005-04-16 3668 while (addr < vaddr) { > ^1da177e4c3f415 Linus Torvalds 2005-04-16 3669 if (count == 0) > ^1da177e4c3f415 Linus Torvalds 2005-04-16 3670 goto finished; > ^1da177e4c3f415 Linus Torvalds 2005-04-16 3671 *buf = '\0'; > ^1da177e4c3f415 Linus Torvalds 2005-04-16 3672 buf++; > ^1da177e4c3f415 Linus Torvalds 2005-04-16 3673 addr++; > ^1da177e4c3f415 Linus Torvalds 2005-04-16 3674 count--; > ^1da177e4c3f415 Linus Torvalds 2005-04-16 3675 } > 129dbdf298d7383 Baoquan He 2023-01-13 3676 n = vaddr + size - addr; > d0107eb07320b5d KAMEZAWA Hiroyuki 2009-09-21 3677 if (n > count) > d0107eb07320b5d KAMEZAWA Hiroyuki 2009-09-21 3678 n = count; > 129dbdf298d7383 Baoquan He 2023-01-13 3679 > 129dbdf298d7383 Baoquan He 2023-01-13 3680 if (flags & VMAP_RAM) > > assume VMAP_RAM is not set OK, then vm is not null. > > 129dbdf298d7383 Baoquan He 2023-01-13 3681 vmap_ram_vread(buf, addr, n, flags); > 129dbdf298d7383 Baoquan He 2023-01-13 @3682 else if (!(vm->flags & VM_IOREMAP)) > ^^^^^^^^^ > Unchecked dereference. Should this be "flags" instead of "vm->flags"? Thus, here, in 'else if', vm is not null. And in this 'else if', we are intending to check vm->flags. I don't see issue or risk here. Please help check if I miss anything. Thanks Baoquan > > d0107eb07320b5d KAMEZAWA Hiroyuki 2009-09-21 3683 aligned_vread(buf, addr, n); > d0107eb07320b5d KAMEZAWA Hiroyuki 2009-09-21 3684 else /* IOREMAP area is treated as memory hole */ > d0107eb07320b5d KAMEZAWA Hiroyuki 2009-09-21 3685 memset(buf, 0, n); > d0107eb07320b5d KAMEZAWA Hiroyuki 2009-09-21 3686 buf += n; > d0107eb07320b5d KAMEZAWA Hiroyuki 2009-09-21 3687 addr += n; > d0107eb07320b5d KAMEZAWA Hiroyuki 2009-09-21 3688 count -= n; > ^1da177e4c3f415 Linus Torvalds 2005-04-16 3689 } > ^1da177e4c3f415 Linus Torvalds 2005-04-16 3690 finished: > e81ce85f960c2e2 Joonsoo Kim 2013-04-29 3691 spin_unlock(&vmap_area_lock); > d0107eb07320b5d KAMEZAWA Hiroyuki 2009-09-21 3692 > d0107eb07320b5d KAMEZAWA Hiroyuki 2009-09-21 3693 if (buf == buf_start) > d0107eb07320b5d KAMEZAWA Hiroyuki 2009-09-21 3694 return 0; > d0107eb07320b5d KAMEZAWA Hiroyuki 2009-09-21 3695 /* zero-fill memory holes */ > d0107eb07320b5d KAMEZAWA Hiroyuki 2009-09-21 3696 if (buf != buf_start + buflen) > d0107eb07320b5d KAMEZAWA Hiroyuki 2009-09-21 3697 memset(buf, 0, buflen - (buf - buf_start)); > d0107eb07320b5d KAMEZAWA Hiroyuki 2009-09-21 3698 > d0107eb07320b5d KAMEZAWA Hiroyuki 2009-09-21 3699 return buflen; > ^1da177e4c3f415 Linus Torvalds 2005-04-16 3700 } > > -- > 0-DAY CI Kernel Test Service > https://github.com/intel/lkp-tests >