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 9186DEEB57C for ; Thu, 12 Sep 2024 09:35:10 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 154316B0085; Thu, 12 Sep 2024 05:35:10 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 0DE056B0089; Thu, 12 Sep 2024 05:35:10 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id E99346B008A; Thu, 12 Sep 2024 05:35:09 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0013.hostedemail.com [216.40.44.13]) by kanga.kvack.org (Postfix) with ESMTP id C81FF6B0085 for ; Thu, 12 Sep 2024 05:35:09 -0400 (EDT) Received: from smtpin05.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay06.hostedemail.com (Postfix) with ESMTP id 7F1BFAAF7A for ; Thu, 12 Sep 2024 09:35:09 +0000 (UTC) X-FDA: 82555577538.05.2195B0B Received: from sender4-pp-f112.zoho.com (sender4-pp-f112.zoho.com [136.143.188.112]) by imf15.hostedemail.com (Postfix) with ESMTP id 5006BA0013 for ; Thu, 12 Sep 2024 09:35:07 +0000 (UTC) Authentication-Results: imf15.hostedemail.com; dkim=pass header.d=collabora.com header.s=zohomail header.b=GzoRH4xm; spf=pass (imf15.hostedemail.com: domain of Usama.Anjum@collabora.com designates 136.143.188.112 as permitted sender) smtp.mailfrom=Usama.Anjum@collabora.com; dmarc=pass (policy=none) header.from=collabora.com; arc=pass ("zohomail.com:s=zohoarc:i=1") ARC-Message-Signature: i=2; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1726133568; 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:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=E0n7JRVOdlG+c3taXjUGqlrkCmLavvou2F5X4LHBmrA=; b=ZiG81uCzHI7kGM0qZLSuAJa0Vg/DDqttXxsmGyHTI95o0+m3EYQfbBZYQFVadE7q5W5YnX aIAsWQ09LeUuN3QQjyd2nm0ykMgSr0n4axRXHGLijnNQL5KJzFG7UHtQ8gWcfGSb6AXdpP sclRuWESb2zrGD4pD0sXNVQUpP9VOso= ARC-Authentication-Results: i=2; imf15.hostedemail.com; dkim=pass header.d=collabora.com header.s=zohomail header.b=GzoRH4xm; spf=pass (imf15.hostedemail.com: domain of Usama.Anjum@collabora.com designates 136.143.188.112 as permitted sender) smtp.mailfrom=Usama.Anjum@collabora.com; dmarc=pass (policy=none) header.from=collabora.com; arc=pass ("zohomail.com:s=zohoarc:i=1") ARC-Seal: i=2; s=arc-20220608; d=hostedemail.com; t=1726133568; a=rsa-sha256; cv=pass; b=JK+/Qj9oflZ/O9ir3om3j9GkH01S6uwLZ6aXMjtzJpKQyNhV2luRCwxmH05su/v0WDkOlI UagImfcjVZmgTNMW6hfracPL4xJ2cqim6sUH8PrS/vvMb0viiqKloA+R4jqs3EBHE3BD3U lwbgIv/8oivzt1oeKciePaqBvkYeamY= ARC-Seal: i=1; a=rsa-sha256; t=1726133703; cv=none; d=zohomail.com; s=zohoarc; b=L1sVBzz+Q//JWpyjuu7csmCT6cHY7f1SfyvmJFyExKmnvoHGS+3L60MPe47J7Ry5mnXbKlxwXQw4NkVOqfmIqkZED3VJ/trzlre3xmQ+Rys9UDWIxeCC6DeMSxQB3zApyM6agwj7pvDRF+haK+Sd/c2TRNBZFpS5Boakq0rh1eE= ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=zohomail.com; s=zohoarc; t=1726133703; h=Content-Type:Content-Transfer-Encoding:Cc:Cc:Date:Date:From:From:In-Reply-To:MIME-Version:Message-ID:References:Subject:Subject:To:To:Message-Id:Reply-To; bh=E0n7JRVOdlG+c3taXjUGqlrkCmLavvou2F5X4LHBmrA=; b=QqdKB1m8faNa8c1w6T38zghafV544IGX+hnckpCFrEZ7VUvsVh4jjRelYq5gDAryWmW1nn7Rn8TxhrEcCiztThXm2Ya1l68eC96Bg0noOEK/1XVZXeQ2OBQ/e19Pak/HTO5fHqeL5NhMUn57UBNopHJ3N3qUK6O/1mcAMHV0zLE= ARC-Authentication-Results: i=1; mx.zohomail.com; dkim=pass header.i=collabora.com; spf=pass smtp.mailfrom=Usama.Anjum@collabora.com; dmarc=pass header.from= DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; t=1726133703; s=zohomail; d=collabora.com; i=Usama.Anjum@collabora.com; h=Message-ID:Date:Date:MIME-Version:Cc:Cc:Subject:Subject:To:To:References:From:From:In-Reply-To:Content-Type:Content-Transfer-Encoding:Message-Id:Reply-To; bh=E0n7JRVOdlG+c3taXjUGqlrkCmLavvou2F5X4LHBmrA=; b=GzoRH4xmtRlxdsD8tsUf+S8ildZ4+cdvQYc6korTIqoc6iXJxMghFUUvUPOJqRMu 7ZMdUsiioAuqFocKlj6+VHWPgG6IAU0HUJ5pvC+pbzpx+9tbcxByaxUrDdxkv9VYWlI MPKXZvRiw4COIbaFAIV9lt/I0tBg21Ldvsffbfuk= Received: by mx.zohomail.com with SMTPS id 1726133702205592.6102112099303; Thu, 12 Sep 2024 02:35:02 -0700 (PDT) Message-ID: Date: Thu, 12 Sep 2024 14:34:54 +0500 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Cc: Usama.Anjum@collabora.com, linux-fsdevel@vger.kernel.org, Kees Cook Subject: Re: [bug report] fs/proc/task_mmu: implement IOCTL to get and optionally clear info about PTEs To: Dan Carpenter , linux-mm@kvack.org References: <3a4e2a3e-b395-41e6-807d-0e6ad8722c7d@stanley.mountain> Content-Language: en-US From: Muhammad Usama Anjum In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-ZohoMailClient: External X-Rspamd-Server: rspam06 X-Rspamd-Queue-Id: 5006BA0013 X-Stat-Signature: t9qgc65wq3di4zzrtmamdg58bi7yw6sr X-Rspam-User: X-HE-Tag: 1726133707-615968 X-HE-Meta: U2FsdGVkX1+crezQxRcYzryPZFzK88+9Lga4xjEuU4eac1GWSSXujsYibLUYBrnqUQo0KD5j9R8w0Mz76YcVfNvLJ32Lg8hhRenBVyB75tMUSrASyLvcr4NPbOKbLO4MCLELvw6KlXHIEcLPPsTG8nGAnjypUgkFqTV58xX/m/ApvZdpmIAiqlStUCmFgm0GGbsjY6Rx6a+En6lgMm3Qv06mDlChp8FleiLRVeiQWIT8YW7ka/A0pwrpxVkFQGi8QlM/bvoI3kVTHek0UgtVdCsyCVeXYIs5H5FnByUN3XPyjF6Qz51PYbIzQUQFJ8A1lcJWFTRbZof7zAkMEHWaeMUzFxx75KX0S0Bmz481xeQTPc1YAz7c6rh1hHj6ecDnHMbKU+3lNrydPWe2r94NZ7KHjODK1rzr9jwOJDxkec74CzcR3KOL8ZqXR3r3koW0qKAblnZiX5T6IEBlp8jbxb3l8hhq1OBZbE9tMmmnPXn2YZ2J5OAlhDWL7NsiKNbXXbp81MBCMJwBCuxVy6qhKJN/GIRuQ4Nb3kNUp/cNdivF1aaAdB8KJEdPDTsOUp+4o0vAAY7viDjX3OZBKJgLRy1UB6F70BWIgDO7PPfGpPDVeN/qHWoT8LAJpjvqFgn11PKoZ7f7RNRut1KR2PPgpshMxxHPT8LTcgHaqEFcZ3tVKxnBICwtJ5ZIqyTDnmkATowxtxipfO2JMg85uwpJOJlIA07s3AgJUv3oN3DXlNUEzoMlvQkKeuFeLfRqPb6ZGtw3VPQuG8ntwIIayBC6hvQT+v1Zt67wJjvTm9p5JsMRI1tGZwHig0IZFn4yv6tPVVNQ+mZqZoxNUk9UycmPx9XlsMkcW/ubFnAy5Fw1HBSf9fc8e4gRdBq84pw3C90MQqzKeopsz+NyQ1tiMRGGIBKB3TtP5bLshFzFCpUyNW/LoCinOhWRLXBVwGhgWMpK0cIDV7gP0f5eNb8Q1Cq YtRdXPDO nC2wTMYKA/O4uUf/2Pts85XE6T4d2NzaTIevaBwKYStw1gA9l3NKTqWyuYF/7ip6kzqDS4Hvii24fxEZ05Te6n5MM6cPCNNG7szwAHKEy6BAorOsClnjyXFaTkZsK3fQvwdC9vz/Sfexin1Oqga/dclbXACiDsa8KXRaHNGFTJn7oriMRfgYTmEQYMHFfZ6CbkCAUqAx5W/7lAM9gf3DLAj3sgfVsNftG+DYplcWasUijqbpd/BkovdJj7IM1uiKTx9rtWO8j4MpyNSqK/XVdYyNjeSl1HI2b/UwRdui+BEkt55PI9eQvegMzjG9y8hc59t3Sw/toV+FxXrbnJXYOGEWhaa2z+vqzBPmiGZ405GYwsoo= 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: List-Subscribe: List-Unsubscribe: On 9/12/24 11:36 AM, Muhammad Usama Anjum wrote: > Hi Dan, > > Thank you for reporting. I've debugged more and found out that no changes are required as access_ok() already deals well with the overflows. I've tested the corner cases on x86_64 and there are no issue found. I'll add more test cases in the selftest for this ioctl. Please share your thoughts if I may have missed something. > > On 9/11/24 3:21 PM, Dan Carpenter wrote: >> Hello Muhammad Usama Anjum, >> >> Commit 52526ca7fdb9 ("fs/proc/task_mmu: implement IOCTL to get and >> optionally clear info about PTEs") from Aug 21, 2023 (linux-next), >> leads to the following Smatch static checker warning: >> >> fs/proc/task_mmu.c:2664 pagemap_scan_get_args() >> warn: potential user controlled sizeof overflow 'arg->vec_len * 24' '0-u64max * 24' type='ullong' >> >> fs/proc/task_mmu.c >> 2637 static int pagemap_scan_get_args(struct pm_scan_arg *arg, >> 2638 unsigned long uarg) >> 2639 { >> 2640 if (copy_from_user(arg, (void __user *)uarg, sizeof(*arg))) >> >> arg comes from the user >> >> 2641 return -EFAULT; >> 2642 >> 2643 if (arg->size != sizeof(struct pm_scan_arg)) >> 2644 return -EINVAL; >> 2645 >> 2646 /* Validate requested features */ >> 2647 if (arg->flags & ~PM_SCAN_FLAGS) >> 2648 return -EINVAL; >> 2649 if ((arg->category_inverted | arg->category_mask | >> 2650 arg->category_anyof_mask | arg->return_mask) & ~PM_SCAN_CATEGORIES) >> 2651 return -EINVAL; >> 2652 >> 2653 arg->start = untagged_addr((unsigned long)arg->start); >> 2654 arg->end = untagged_addr((unsigned long)arg->end); >> 2655 arg->vec = untagged_addr((unsigned long)arg->vec); >> 2656 >> 2657 /* Validate memory pointers */ >> 2658 if (!IS_ALIGNED(arg->start, PAGE_SIZE)) >> 2659 return -EINVAL; >> >> We should probably check ->end here as well. >> >> 2660 if (!access_ok((void __user *)(long)arg->start, arg->end - arg->start)) > I'll add check to verify that end is equal or greater than start. > >> >> Otherwise we're checking access_ok() and then making ->end larger. Maybe move >> the arg->end = ALIGN(arg->end, PAGE_SIZE) before the access_ok() check? >> >> 2661 return -EFAULT; >> 2662 if (!arg->vec && arg->vec_len) >> 2663 return -EINVAL; >> --> 2664 if (arg->vec && !access_ok((void __user *)(long)arg->vec, >> 2665 arg->vec_len * sizeof(struct page_region))) >> >> This "arg->vec_len * sizeof(struct page_region)" multiply could have an integer >> overflow. > I'll check for overflow before calling access_ok(). > >> >> arg->vec_len is a u64 so size_add() won't work on a 32bit system. I wonder if >> size_add() should check for sizes larger than SIZE_MAX? >> >> 2666 return -EFAULT; >> 2667 >> 2668 /* Fixup default values */ >> 2669 arg->end = ALIGN(arg->end, PAGE_SIZE); >> 2670 arg->walk_end = 0; >> 2671 if (!arg->max_pages) >> 2672 arg->max_pages = ULONG_MAX; >> 2673 >> 2674 return 0; >> 2675 } > I'll send fix soon. > >> >> regards, >> dan carpenter > -- BR, Muhammad Usama Anjum