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 8FB74C433F5 for ; Thu, 10 Feb 2022 01:02:50 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id EECBA6B0071; Wed, 9 Feb 2022 20:02:49 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id E9ABD6B007B; Wed, 9 Feb 2022 20:02:49 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id D3AB66B0080; Wed, 9 Feb 2022 20:02:49 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0004.hostedemail.com [216.40.44.4]) by kanga.kvack.org (Postfix) with ESMTP id C20456B0071 for ; Wed, 9 Feb 2022 20:02:49 -0500 (EST) Received: from smtpin21.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay01.hostedemail.com (Postfix) with ESMTP id 5AE4B181CA083 for ; Thu, 10 Feb 2022 01:02:49 +0000 (UTC) X-FDA: 79125070458.21.E71DAD8 Received: from mail-yb1-f172.google.com (mail-yb1-f172.google.com [209.85.219.172]) by imf14.hostedemail.com (Postfix) with ESMTP id DD487100005 for ; Thu, 10 Feb 2022 01:02:48 +0000 (UTC) Received: by mail-yb1-f172.google.com with SMTP id v186so11075749ybg.1 for ; Wed, 09 Feb 2022 17:02:48 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20210112; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=GJJ5yMq4ZICn27lugqpl19qQCQjAVf5SyLvlpWNSqt0=; b=VNgm+lzKXsiNKKdpaoeaWNAu32B5xjtsHA1m4wVSv9gTf4IV5ROsg7e2nuHKpdvAn3 TRXaj9ph4kV3IW/BUH8OPDoCqAv78qclFKnKW/daXW1QL0PKct5O7o9NrHL8UXd7ZGBY poD4IygP47kjs50QclsEQIXz+N7WaDSRxVS9mmj3sri59JIg/pI8PfpTHXp5pHLcznCt 0nzNI2NsRkn3hoiQRjdvB0ARz87iRF6/T1uHX8sT0QzCYSNFpsurMsvPdpl3Ue+sdo4V peZlpOYn+oATQ2bv85shLDAzrS51pfQ2mZtthqN45XvU5FtWB5tEnOkaN2541gkgbjyP relw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=GJJ5yMq4ZICn27lugqpl19qQCQjAVf5SyLvlpWNSqt0=; b=NUH6BPzC2q2IW0Ozl2Hom/qfRj+BSWdw08PYje9Kk9im6xUtpkp1DZrpZebuA757E5 /GEOIZ5o4OaGzhrsaehKsq0c5blyPNGGpHoypZ270B2n6zuMdUcWb0CUSXpjSgo4HynV gHab39J84QlTwl6zIZn8yoH3pVDsYA6QBk015jiLmrYd4qNW2L91v2A+TL7oMNCTqSpv 4P259Eqf1lIUsDoDisS9Avuj/T83WPY/+6HetdAc5WoOeMgpZ4HVI94uAo5xAeX0HoOl aRCGZagCzaIoiRFVGCEwuFoVKAaGL17MJGSQQDaCSQo4AGz5lSdtz9ZLRrAaE46WZnc8 6USQ== X-Gm-Message-State: AOAM5325YX6L7ikfY5XaViyDe6LBOMTklDc1nbtKJNStNc+Sk1TVbQxP ChQz7oZ56+puAe34r+RmUcNPHXf//Mf9uKbuiTz2gQ== X-Google-Smtp-Source: ABdhPJxQZxm3apDeuDNnV3eo9lSJfTQBksY2fWlFYAWl9sDbeKsof4m++lkshWfOLuTQ90yHcGl80ySA19rW5Cwqyv8= X-Received: by 2002:a25:7a47:: with SMTP id v68mr5063005ybc.488.1644454967763; Wed, 09 Feb 2022 17:02:47 -0800 (PST) MIME-Version: 1.0 References: <20220210001801.15413-1-surenb@google.com> <20220209163324.bbf26e7462b217d453c5a34f@linux-foundation.org> In-Reply-To: <20220209163324.bbf26e7462b217d453c5a34f@linux-foundation.org> From: Suren Baghdasaryan Date: Wed, 9 Feb 2022 17:02:36 -0800 Message-ID: Subject: Re: [PATCH 1/1] mm: Fix UAF when anon vma name is used after vma is freed To: Andrew Morton Cc: Colin Cross , Sumit Semwal , Michal Hocko , Dave Hansen , Kees Cook , Matthew Wilcox , "Kirill A . Shutemov" , Vlastimil Babka , Johannes Weiner , "Eric W. Biederman" , brauner@kernel.org, legion@kernel.org, ran.xiaokai@zte.com.cn, sashal@kernel.org, Chris Hyser , Davidlohr Bueso , Peter Collingbourne , caoxiaofeng@yulong.com, David Hildenbrand , Cyrill Gorcunov , linux-mm , LKML , kernel-team , syzbot+aa7b3d4b35f9dc46a366@syzkaller.appspotmail.com Content-Type: text/plain; charset="UTF-8" X-Stat-Signature: kwumbec76m58e8f8h6bdek3e58s93mha Authentication-Results: imf14.hostedemail.com; dkim=pass header.d=google.com header.s=20210112 header.b=VNgm+lzK; spf=pass (imf14.hostedemail.com: domain of surenb@google.com designates 209.85.219.172 as permitted sender) smtp.mailfrom=surenb@google.com; dmarc=pass (policy=reject) header.from=google.com X-Rspam-User: X-Rspamd-Server: rspam09 X-Rspamd-Queue-Id: DD487100005 X-HE-Tag: 1644454968-709158 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: On Wed, Feb 9, 2022 at 4:33 PM Andrew Morton wrote: > > On Wed, 9 Feb 2022 16:18:01 -0800 Suren Baghdasaryan wrote: > > > When adjacent vmas are being merged it can result in the vma that was > > originally passed to madvise_update_vma being destroyed. In the current > > implementation, the name parameter passed to madvise_update_vma points > > directly to vma->anon_name->name and it is used after the call to > > vma_merge. In the cases when vma_merge merges the original vma and > > destroys it, this will result in use-after-free bug as shown below: > > > > madvise_vma_behavior << passes vma->anon_name->name as name param > > madvise_update_vma(name) > > vma_merge > > __vma_adjust > > vm_area_free <-- frees the vma > > replace_vma_anon_name(name) <-- UAF > > > > Fix this by passing madvise_update_vma a copy of the name. > > > > ... > > > > --- a/kernel/sys.c > > +++ b/kernel/sys.c > > @@ -2263,7 +2263,6 @@ int __weak arch_prctl_spec_ctrl_set(struct task_struct *t, unsigned long which, > > > > #ifdef CONFIG_ANON_VMA_NAME > > > > -#define ANON_VMA_NAME_MAX_LEN 80 > > #define ANON_VMA_NAME_INVALID_CHARS "\\`$[]" > > > > static inline bool is_valid_name_char(char ch) > > diff --git a/mm/madvise.c b/mm/madvise.c > > index 5604064df464..f36a5a9942d8 100644 > > --- a/mm/madvise.c > > +++ b/mm/madvise.c > > @@ -976,6 +976,8 @@ static int madvise_vma_behavior(struct vm_area_struct *vma, > > { > > int error; > > unsigned long new_flags = vma->vm_flags; > > + char name_buf[ANON_VMA_NAME_MAX_LEN]; > > + const char *anon_name; > > > > switch (behavior) { > > case MADV_REMOVE: > > @@ -1040,8 +1042,18 @@ static int madvise_vma_behavior(struct vm_area_struct *vma, > > break; > > } > > > > + anon_name = vma_anon_name(vma); > > + if (anon_name) { > > + /* > > + * Make a copy of the name because vma might be destroyed when > > + * merged with another one and the name parameter might be used > > + * after that. > > + */ > > + strcpy(name_buf, anon_name); > > + anon_name = name_buf; > > + } > > error = madvise_update_vma(vma, prev, start, end, new_flags, > > - vma_anon_name(vma)); > > + anon_name); > > anon_name is refcounted. Why not use kref_get()/kref_put() instead of > taking a copy? Yes, I considered that. It would require new get/put APIs for anon_name and I thought I better keep it simple. This path is used only by madvise() syscall, so the copy overhead should not be critical. But if you think refcounting is more appropriate here I'll happily rework it. It should still be quite simple. Please let me know. >