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 08084C00140 for ; Wed, 10 Aug 2022 18:14:26 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 7FF886B0071; Wed, 10 Aug 2022 14:14:26 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 795268E0002; Wed, 10 Aug 2022 14:14:26 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 601948E0001; Wed, 10 Aug 2022 14:14:26 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0015.hostedemail.com [216.40.44.15]) by kanga.kvack.org (Postfix) with ESMTP id 49F7D6B0071 for ; Wed, 10 Aug 2022 14:14:26 -0400 (EDT) Received: from smtpin13.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay04.hostedemail.com (Postfix) with ESMTP id 102751A0677 for ; Wed, 10 Aug 2022 18:14:26 +0000 (UTC) X-FDA: 79784482932.13.A558EEC Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by imf04.hostedemail.com (Postfix) with ESMTP id 5CD9440172 for ; Wed, 10 Aug 2022 18:14:24 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1660155263; 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=dvFX4t0B/SpHtAyJJhX1X854uDLR0fd0SKbf+261CSI=; b=BaXyAh1eQp4SHOtNByh0tHunOopf1hV3xVhv1RvwXpieTyJkBVD0o4x/fpZ9IASAMVUNEc ZyjcAWVdC0tYgsw/+2Utn3cDQ4hDNnwk0Ta/IrCSYpx8H/x+QEIpsVM5VP+qAh2cQ7xeN/ I4P+eJHaHuVZ4gkuCM+DdsulVKvbHOM= Received: from mail-yb1-f199.google.com (mail-yb1-f199.google.com [209.85.219.199]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_128_GCM_SHA256) id us-mta-321-9AJjfTXZMm6__Z3adASz5w-1; Wed, 10 Aug 2022 14:14:22 -0400 X-MC-Unique: 9AJjfTXZMm6__Z3adASz5w-1 Received: by mail-yb1-f199.google.com with SMTP id y81-20020a253254000000b0067ba548d2a1so11090771yby.15 for ; Wed, 10 Aug 2022 11:14:22 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:x-gm-message-state:from:to:cc; bh=dvFX4t0B/SpHtAyJJhX1X854uDLR0fd0SKbf+261CSI=; b=cOBigHRE0NG/6fc97te/iPPDkFLRvvtR6gBh+s0ugvH/6vdckRx7ZMeJIlYsvO+ncX DUQ9m4d1QjpHa/hsxC41650g16IhUI50cPXq8Oa2boLtWT6kkEt2jR9Lp4FyFekkyboU zvQS3Byjfykezn8s7d9CEBjt/F2irBvROqzTQRkoVbc0xR4DWPlPIhiEMjSqEaCnLfVw ejE5CPtLV4Z/ZDgRwqrKSwBC/Q5KFYTTlrsSORQ6cAj6/H7uIiG8uhcYkfIVLN45t/n3 mysaX3lf01ilskvhrw3lhf0LOdCWl1xOIe6I3G6uz1xV8ip2/dmvb9uu8xeZomnyFeTg gLgA== X-Gm-Message-State: ACgBeo1MD1fhX7DO/KlkSK6NAMuh9WdsD7nPbrN53zWCfhLnATxT5bJB 2U7Plr9w9uV4Qt20ek/E4B8QkG70g2MMw/NL9/IjF/1KjqoFKSTQWowGyjEBD2AXaf4BDUtoVz1 80UremEvXoXqCfZBWoMI9aeOtuz4= X-Received: by 2002:a0d:c745:0:b0:324:8800:f63d with SMTP id j66-20020a0dc745000000b003248800f63dmr29130201ywd.106.1660155262277; Wed, 10 Aug 2022 11:14:22 -0700 (PDT) X-Google-Smtp-Source: AA6agR6muemIRMkvnwQwC0YGQ9xi48xUoLNMRmcd7lKsvryfN7vF3N1f9cU5/b6fdn1SeHdJ0SQPfUHfpAVCid56NvM= X-Received: by 2002:a0d:c745:0:b0:324:8800:f63d with SMTP id j66-20020a0dc745000000b003248800f63dmr29130162ywd.106.1660155262052; Wed, 10 Aug 2022 11:14:22 -0700 (PDT) MIME-Version: 1.0 References: <20220810160209.1630707-1-Liam.Howlett@oracle.com> In-Reply-To: <20220810160209.1630707-1-Liam.Howlett@oracle.com> From: Ondrej Mosnacek Date: Wed, 10 Aug 2022 20:14:10 +0200 Message-ID: Subject: Re: [PATCH v2] binder_alloc: Add missing mmap_lock calls when using the VMA To: Liam Howlett Cc: "linux-mm@kvack.org" , "linux-kernel@vger.kernel.org" , syzbot , Andrew Morton , "syzkaller-bugs@googlegroups.com" , Minchan Kim , Christian Brauner , Greg Kroah-Hartman , Hridya Valsaraju , Joel Fernandes , Martijn Coenen , Suren Baghdasaryan , Todd Kjos , Matthew Wilcox , =?UTF-8?B?QXJ2ZSBIasO4bm5ldsOlZw==?= , Carlos Llamas X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset="UTF-8" ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1660155264; a=rsa-sha256; cv=none; b=BKwWbDMJ9oDj1PPjQmSABtGlT2zOFP2Y5sgixgtvdzASTzyS2l5UtafzdnSYHrnH62xGOs qU9YwPH86d5ro9E02m7FzRoCgAZS3AKd9ggbe2pFzs+hEWLqOj0PfJD1Dw5RWhuNGzO0Iv twxpDNKAIRLux8hLLm883/1vm9skL18= ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1660155264; 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=dvFX4t0B/SpHtAyJJhX1X854uDLR0fd0SKbf+261CSI=; b=nnDOoPNIRCzd1Ur03WVyG1Rvs7uQhID07VvndXRMiOvfAuwK83iXvLTk9ImQn1Heo6FpEO ABnMgKNJDe1471bBhnYlSjS47wGgx4tOyxDCkVqbf+OR7aRs2V2GTZZpt/ZBidLY2iVOgI WePaxkCbTwsvBk9AK1KMAlvLwONPBDA= ARC-Authentication-Results: i=1; imf04.hostedemail.com; dkim=pass header.d=redhat.com header.s=mimecast20190719 header.b=BaXyAh1e; spf=pass (imf04.hostedemail.com: domain of omosnace@redhat.com designates 170.10.129.124 as permitted sender) smtp.mailfrom=omosnace@redhat.com; dmarc=pass (policy=none) header.from=redhat.com X-Stat-Signature: c5f4a9rgbe4wsqqhdecxpo5gxxjsz5ms X-Rspamd-Queue-Id: 5CD9440172 Authentication-Results: imf04.hostedemail.com; dkim=pass header.d=redhat.com header.s=mimecast20190719 header.b=BaXyAh1e; spf=pass (imf04.hostedemail.com: domain of omosnace@redhat.com designates 170.10.129.124 as permitted sender) smtp.mailfrom=omosnace@redhat.com; dmarc=pass (policy=none) header.from=redhat.com X-Rspam-User: X-Rspamd-Server: rspam04 X-HE-Tag: 1660155264-209592 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, Aug 10, 2022 at 6:02 PM Liam Howlett wrote: > Take the mmap_read_lock() when using the VMA in > binder_alloc_print_pages() and when checking for a VMA in > binder_alloc_new_buf_locked(). > > It is worth noting binder_alloc_new_buf_locked() drops the VMA read lock > after it verifies a VMA exists, but may be taken again deeper in the > call stack, if necessary. > > Reported-by: Ondrej Mosnacek > Reported-by: syzbot+a7b60a176ec13cafb793@syzkaller.appspotmail.com > Fixes: a43cfc87caaf (android: binder: stop saving a pointer to the VMA) > Signed-off-by: Liam R. Howlett > --- > drivers/android/binder_alloc.c | 31 +++++++++++++++++++++---------- > 1 file changed, 21 insertions(+), 10 deletions(-) > > diff --git a/drivers/android/binder_alloc.c b/drivers/android/binder_alloc.c > index f555eebceef6..c5e7c6d3a844 100644 > --- a/drivers/android/binder_alloc.c > +++ b/drivers/android/binder_alloc.c > @@ -395,12 +395,15 @@ static struct binder_buffer *binder_alloc_new_buf_locked( > size_t size, data_offsets_size; > int ret; > > + mmap_read_lock(alloc->vma_vm_mm); > if (!binder_alloc_get_vma(alloc)) { > + mmap_read_unlock(alloc->vma_vm_mm); > binder_alloc_debug(BINDER_DEBUG_USER_ERROR, > "%d: binder_alloc_buf, no vma\n", > alloc->pid); > return ERR_PTR(-ESRCH); > } > + mmap_read_unlock(alloc->vma_vm_mm); > > data_offsets_size = ALIGN(data_size, sizeof(void *)) + > ALIGN(offsets_size, sizeof(void *)); > @@ -922,17 +925,25 @@ void binder_alloc_print_pages(struct seq_file *m, > * Make sure the binder_alloc is fully initialized, otherwise we might > * read inconsistent state. > */ > - if (binder_alloc_get_vma(alloc) != NULL) { > - for (i = 0; i < alloc->buffer_size / PAGE_SIZE; i++) { > - page = &alloc->pages[i]; > - if (!page->page_ptr) > - free++; > - else if (list_empty(&page->lru)) > - active++; > - else > - lru++; > - } > + > + mmap_read_lock(alloc->vma_vm_mm); > + if (binder_alloc_get_vma(alloc) == NULL) { > + mmap_read_unlock(alloc->vma_vm_mm); > + goto uninitialized; > } > + > + mmap_read_unlock(alloc->vma_vm_mm); > + for (i = 0; i < alloc->buffer_size / PAGE_SIZE; i++) { > + page = &alloc->pages[i]; > + if (!page->page_ptr) > + free++; > + else if (list_empty(&page->lru)) > + active++; > + else > + lru++; > + } > + > +uninitialized: > mutex_unlock(&alloc->mutex); > seq_printf(m, " pages: %d:%d:%d\n", active, lru, free); > seq_printf(m, " pages high watermark: %zu\n", alloc->pages_high); > -- > 2.35.1 Can confirm that with this patch the selinux-testsuite's binder test passes again with no warnings in dmesg. Tested-by: Ondrej Mosnacek Thanks! -- Ondrej Mosnacek Senior Software Engineer, Linux Security - SELinux kernel Red Hat, Inc.