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 47740CFA755 for ; Fri, 4 Oct 2024 10:13:25 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id B45656B040B; Fri, 4 Oct 2024 06:13:24 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id AF4C96B040C; Fri, 4 Oct 2024 06:13:24 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 9BCE16B040D; Fri, 4 Oct 2024 06:13:24 -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 7EABD6B040B for ; Fri, 4 Oct 2024 06:13:24 -0400 (EDT) Received: from smtpin12.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay10.hostedemail.com (Postfix) with ESMTP id 29E62C0E0F for ; Fri, 4 Oct 2024 10:13:24 +0000 (UTC) X-FDA: 82635507528.12.D49B8E4 Received: from mail-pf1-f171.google.com (mail-pf1-f171.google.com [209.85.210.171]) by imf03.hostedemail.com (Postfix) with ESMTP id 65EE120018 for ; Fri, 4 Oct 2024 10:13:21 +0000 (UTC) Authentication-Results: imf03.hostedemail.com; dkim=pass header.d=iiitd.ac.in header.s=google header.b=QAWMNvvR; spf=pass (imf03.hostedemail.com: domain of manas18244@iiitd.ac.in designates 209.85.210.171 as permitted sender) smtp.mailfrom=manas18244@iiitd.ac.in; dmarc=none ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1728036671; 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=XPGRnLkJcNuyTgTIqxv1cy1FTj/54JB3cbTz2KFQ0BE=; b=mwcjkejQG2vB9HXYjO8sbmJZWCt3lc2jsp0mwDxAXnJG4OgyvjkVIE9dewF3zyE13bjfPg gxcs/AinjW6xYbdVqIFpTTyl6loDeIFARPjHBgww4k1h/CBGK94BaBXsbbTKaqPWKRKfID EW1w5Ixm3hoNLpVn0NydoFzAQFnp8Qs= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1728036671; a=rsa-sha256; cv=none; b=U0gwqz1nOzUzZ4gtjbkvazT/qy6JI2NwkNi8xRVDOdT/NfaWIIBHw9N6h4rAd/d+C8i/hK 8dvC1rXgt7rxHK5CFcnIQky41MPWzgyNaEzRA1QkLmc0vsmybJDETEoGUuG2yvv7jDJggS lkZXVtT1nA4YX1+UivGUpvBZarnp/+U= ARC-Authentication-Results: i=1; imf03.hostedemail.com; dkim=pass header.d=iiitd.ac.in header.s=google header.b=QAWMNvvR; spf=pass (imf03.hostedemail.com: domain of manas18244@iiitd.ac.in designates 209.85.210.171 as permitted sender) smtp.mailfrom=manas18244@iiitd.ac.in; dmarc=none Received: by mail-pf1-f171.google.com with SMTP id d2e1a72fcca58-71dae4fc4c9so1654012b3a.0 for ; Fri, 04 Oct 2024 03:13:20 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=iiitd.ac.in; s=google; t=1728036800; x=1728641600; darn=kvack.org; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:from:to:cc:subject:date:message-id:reply-to; bh=XPGRnLkJcNuyTgTIqxv1cy1FTj/54JB3cbTz2KFQ0BE=; b=QAWMNvvRK+DYKI5hs+h7fLDSpGmXZliYjTSEUkjLirJdN5ND3c/4IfbVBBSQ0TDe4D x66bxsjz+Hv8GMSwiw2cESXyEA4qCjMyEoSV0WyKmfOHC/cq3XaN8+3MIg44wON+MFpi /05n7vl+7wkbY2Y7nFKlqsDzwkH9yGyu/DZAc= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1728036800; x=1728641600; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=XPGRnLkJcNuyTgTIqxv1cy1FTj/54JB3cbTz2KFQ0BE=; b=hMGEHcKVORsL4jWXZ4IXK/7BDp/FOxSNuY5REePYxtNLG+v85uzCb/lnjitGpO4TXb VWaYnxg1XY8k8Ku31RDtBEZWiVPHMw/XAlzXXggZqHvBEXtIrw0fcqotTOVDg+SreKfW Ta34OKkdkaqkJ4uudeRqBvGTPM7Fmnv9WTRoORP61kcMumdVajdDzgICbaniqtLgMqPy /cc5M1J+/SW+goDx4uhPLXFOojZBJmdpsr070s4kavr5XqvuSeptVxptqciN5DlHHPnG wahr0aSc1PgOs+kPBCYHz/rPS8/kQjjzEafSe6HUiX4HA0NiZ6pR/1fPJkQ/H5MakkYj Q/SQ== X-Forwarded-Encrypted: i=1; AJvYcCUcvrbjP+PqeRFUJgwkBCB39yiReFweNVrAeF1GXpoQasS63nQX1yLSYQqPDJDbMDU2cn1sVpzk1g==@kvack.org X-Gm-Message-State: AOJu0Yy25EEuYr5qf2FLmkYPvvYjBwSr92Gdo/BN6FRyPbIyKg8cXdad przW2oCcd+S4A6+8ewm3xymcUwgR5oXIk4qRbqythaP9WJCz9C0AwPFwbkYn1oo= X-Google-Smtp-Source: AGHT+IFpDRip7ZoLiaDkcYkyz5tKjOu56kdo3ARfMFyFPFHyuu3hykcKSxokPboYpQhFfAFyTwCeFg== X-Received: by 2002:a05:6a21:a4c1:b0:1d3:294e:65fb with SMTP id adf61e73a8af0-1d6dfa422b1mr3291408637.25.1728036799581; Fri, 04 Oct 2024 03:13:19 -0700 (PDT) Received: from fedora ([103.3.204.140]) by smtp.gmail.com with ESMTPSA id d2e1a72fcca58-71dd9defc6fsm2848697b3a.158.2024.10.04.03.13.15 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 04 Oct 2024 03:13:18 -0700 (PDT) Date: Fri, 4 Oct 2024 15:43:12 +0530 From: Manas To: Peter Xu Cc: Andrew Morton , Shuah Khan , Anup Sharma , linux-mm@kvack.org, linux-kernel@vger.kernel.org, syzbot+093d096417e7038a689b@syzkaller.appspotmail.com Subject: Re: [PATCH] Fixes: null pointer dereference in pfnmap_lockdep_assert Message-ID: References: <20241003-fix-null-deref-v1-1-0a45df9d016a@iiitd.ac.in> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8; format=flowed Content-Disposition: inline In-Reply-To: X-Stat-Signature: 9s3yp5tnzadfpgpm6bsdapwio6ax9n9q X-Rspamd-Queue-Id: 65EE120018 X-Rspam-User: X-Rspamd-Server: rspam08 X-HE-Tag: 1728036801-854997 X-HE-Meta: U2FsdGVkX1+eHRn9T7PyaR9PhFrouURNNLS995SGKV+AUKSXI77I/Zepn4GLATvtl5fs7xHPhPySOZtj50Fxyi1UZbQYmi/kZUYJtFzbpMXpltF52sHb1CIef5l0zhkKu8Gzv7TTL/sZ1oA6X6AwcPLSKnTtj5aVOMZ9uOErzIXAinhncpPlya+UPA0OEuFd9fu8JjcpaO559Gifc3g53+5jgg6xNpfb5o55BqLxCJVKJ5CeD/gcBlx17gPWjPy1eavh8aYa39+fuzUsTSr2Ggg8+TM/5NTSAJU92Mv2nBJcwj+TPWsVLdQIj9TBP2D7PNC7i7RVmyOquMSoxYEB1lPZP3hLzuZGCWnkXknx44QCvm/Kv1Jk+2ZHQC5S34lA/AAAPDP6c4APmoF0CFkpjpMrnbRn4uSn5eXtL+3dgFsrtpxrUt7JIpD1hhwjADOcQlsVyPdI9V0a6TuHYIJ8E6YjbzjEWMG4CvDn4G9MSM0IqmGH6Wx0YgIZz0bNNtDZQqEuJTVjKSFa471/YJwf7ZK5+nffyrJ9GHnJlXtH9tz53inmL4RnOumrgd21hstzHioZQ5AWm4Z2+5y4DdB2dIaEMG5o9vC3hO7Kv2Z9xdtqXAxJrFb36UcjExV5T+dM1uhbpeXImaPLc1DjIXutoWBAPFEkBIds1yXrHwyA8EMp0qeYSoxAyLI7rvQ5Q0sNb289zzNtB0aUyJyIqQ3FrOw+bKC6IOwdirVQ1/JfY85TJeW9x09i41/XuQJaV0VgNuUJqKUv6P/SdQHjfsKAlwjUdGLqihQPfI/wbSdFT7umwE/AU/k5SyFEY4ui1f80aHloHEC8v0Ky0odJ9RIGGVry/ajtbbFRXK0lROXP8LBjx21ddouh2u2ku0MWb4Va+dNxSh3t0TfJJe6I63+Ipr9GU+JjRHtiGnz4coYcsgvtHbRCcbAUUcJu31uDWLXbEuN3dZAI6uLrlYIBeL2 WAQYUk7x nZotdkeMRFZ2/eOu1UzSVdMl65uLg1Oe3ilGBzm8uvOHp8RNsbZh+SZiwH5fpCL2S9pGUvqYVQIHSzEBDTGbFFVn6APRk8EB9vqCYepZHCL++pQ+dXRCsdjbsFIOkS29riMSkrhBk4Gxdd/8Iy5z55iED1YKkBkAXntn+fkuECwl3Hrrnd7xMdCkby/92mFqXn5sMYWa+5k25wKbUY1jvmmtk53gA0DPvo9qQAbU7v4Md4REZvnNQWhxEWVzTcOBbe5VgCYUZ8oN4isxb6Yfb22nOmoXFPgtx2R8hYieVaSFx2zhDfcaBZAZsEo1II0sJz8jAapf//bzSZr5xZnBtCXOMb6i5Ylj0edS3mnUZuc98OmAx1qHdiMD47Yr3jpMB0giDBObhOHOb9WQZEB+zUNWqQ1/g9EEULvMHHuFO3JbnAqc3M7V8Ru5FHAnwkb6v3F1J8rr2uruHiSYW5Be/ELEpXv93maq9VPM34QzLf9YYnogNqWjl4mxs6YOVAjg07gv2QnxAKw6lt9eBgAZrAWWXuppZnquLgNDozj59+EyBiORY6cC8sIO+6VFeBW1+dDke320FqR74TCyOlsVWTDv5yufiSLL//Y7OUP7mv3nbuS/Frsbapz5G96iegcnNXD+D 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: Hi Peter, thanks for reviewing. On 03.10.2024 12:41, Peter Xu wrote: >On Thu, Oct 03, 2024 at 09:31:06PM +0530, Manas via B4 Relay wrote: >> From: Manas >> >> syzbot has pointed to a possible null pointer dereference in >> pfnmap_lockdep_assert. vm_file member of vm_area_struct is being >> dereferenced without any checks. >> >> This fix returns if vm_file member in vm_area_struct is NULL. >> >> Reported-by: syzbot+093d096417e7038a689b@syzkaller.appspotmail.com >> Closes: https://syzkaller.appspot.com/bug?extid=093d096417e7038a689b >> --- >> This bug[1] triggers a general protection fault in follow_pfnmap_start >> function. An assertion pfnmap_lockdep_assert inside this function >> dereferences vm_file member of vm_area_struct. And panic gets triggered >> when vm_file is NULL. >> >> This patch returns from the assertion pfnmap_lockdep_assert if vm_file >> is found to be NULL. >> >> [1] https://syzkaller.appspot.com/bug?extid=093d096417e7038a689b >> >> Signed-off-by: Manas > >Thanks for the patch! > >> --- >> mm/memory.c | 3 +++ >> 1 file changed, 3 insertions(+) >> >> diff --git a/mm/memory.c b/mm/memory.c >> index 2366578015ad..b152a95e543f 100644 >> --- a/mm/memory.c >> +++ b/mm/memory.c >> @@ -6346,6 +6346,9 @@ static inline void pfnmap_args_setup(struct follow_pfnmap_args *args, >> static inline void pfnmap_lockdep_assert(struct vm_area_struct *vma) >> { >> #ifdef CONFIG_LOCKDEP >> + if (!vma->vm_file) >> + return; >> + > >Hmm I guess I wasn't careful enough here as I was mostly only thinking >about file mappings, but I just notice we have other pfnmaps like the vvar >mappings.. the mapping var can also already be reused later when available. > >Logically even if !vm_file we can still check against mmap write lock. So >would it be better to do this instead: > > struct address_space *mapping = vma->vm_file && vma->vm_file->f_mapping; > This will lead to `-Wint-conversion` error in the assignment. We can either do a cast like the following: struct address_space *mapping = (struct address_space *)(vma->vm_file && vma->vm_file->f_mapping); But I am not sure if it is the canonical way of doing it. It will also lead to warning about pointer from integer casting. Or will a conditional like this work here? struct address_space *mapping = vma->vm_file ? vma->vm_file->f_mapping : NULL; > if (mapping) > lockdep_assert(lockdep_is_held(&mapping->i_mmap_rwsem) || > lockdep_is_held(&vma->vm_mm->mmap_lock)); > else > lockdep_assert(lockdep_is_held(&vma->vm_mm->mmap_lock)); > >? > >> struct address_space *mapping = vma->vm_file->f_mapping; >> >> if (mapping) >> >> --- >> base-commit: 9852d85ec9d492ebef56dc5f229416c925758edc >> change-id: 20241003-fix-null-deref-6bfa0337efc3 >> >> Best regards, >> -- >> Manas >> >> >> > >-- >Peter Xu > -- Manas