From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 2C29BA050B for ; Tue, 26 Apr 2022 16:15:24 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 26D5E427FC; Tue, 26 Apr 2022 16:15:24 +0200 (CEST) Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by mails.dpdk.org (Postfix) with ESMTP id E048340E78 for ; Tue, 26 Apr 2022 16:15:21 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1650982521; 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=9uE+kx6K4p7/oNqQ1GyZcrriRZxWfLl6RAOtj2g8JmI=; b=RlMvCj70xVYhdvr2yu7P3kA6BRUHIXC7IpbqC4fRurEVw1K9M2gRGzpk/qP3csnrEugCgi KoUbA4P8J3L+3M7vz77Xx3vcZpJ0EnEJKascI4R+7ZAmysx4Fz1h5BhOnU7z4xCVWvMy4R 97XCIUrzI4Kb9l1quwZSvKJM7ppTqR0= Received: from mail-lj1-f197.google.com (mail-lj1-f197.google.com [209.85.208.197]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-189-5pzFy0SOP2WE_9eST2AAOg-1; Tue, 26 Apr 2022 10:15:20 -0400 X-MC-Unique: 5pzFy0SOP2WE_9eST2AAOg-1 Received: by mail-lj1-f197.google.com with SMTP id o17-20020a2e0c51000000b0024b484876a8so4673660ljd.21 for ; Tue, 26 Apr 2022 07:15:19 -0700 (PDT) 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=9uE+kx6K4p7/oNqQ1GyZcrriRZxWfLl6RAOtj2g8JmI=; b=e1cPAi1jYYQj2hlFtikDlUIACjK4tC32iJB8b74YFPpjZ8hkONjOa4mlkawFaw5wY7 EXIddoWwQxpH9j3iROdAv58zekGTblvpwYdQP+VXNyms9CLfbEVldrut45unhT+AaRE1 D6Pf713qATRGYH9fjqpFAO9I8QXwa5a07qHUUD6ZUKe8G4f2K8P8BECMc5y6NEJpdodr qxyx8mmOjexJ4WasYXMxklKrRVrkW1/MgsnW2myBXn1xyUTnX09SPm/DxZad0/Hntiar xEwGCI9z6M0qpJ3QW1yJ1skn1cHyQT5u7sQBphR5T8s80mgyYN51yRpdF4NrUkOA2c3S 42dw== X-Gm-Message-State: AOAM531Y9b+/HdwIqZrI6WMLRWfprn9IIrbrvWeRzIcKNT0q3On+JAF+ XhYFhTlbMLTW7Um7BwBDha8cAOXVvqmA1eSo+/EAyeOpE3/hHQenj2WbZiziqNJwTzbm2qfXyQA f44Tqgx1Urbg0clp3uvUD1Fk= X-Received: by 2002:a05:6512:1326:b0:45a:3a4:f25a with SMTP id x38-20020a056512132600b0045a03a4f25amr16633782lfu.575.1650982518541; Tue, 26 Apr 2022 07:15:18 -0700 (PDT) X-Google-Smtp-Source: ABdhPJwh857N4GS913jtSKk9LrYmlBllO/jPy2abfbm49kaj6k5rUDtjsMx7F0knKdWycOyukI3dgOYchVSoqQWrszs= X-Received: by 2002:a05:6512:1326:b0:45a:3a4:f25a with SMTP id x38-20020a056512132600b0045a03a4f25amr16633764lfu.575.1650982518292; Tue, 26 Apr 2022 07:15:18 -0700 (PDT) MIME-Version: 1.0 References: <20220415173127.3838-1-david.marchand@redhat.com> <20220415173127.3838-3-david.marchand@redhat.com> <419bb7fc-cb04-10cf-a40a-5dba39323f9e@intel.com> <249b5c3e-513b-af96-399a-01d60ff93d26@intel.com> In-Reply-To: <249b5c3e-513b-af96-399a-01d60ff93d26@intel.com> From: David Marchand Date: Tue, 26 Apr 2022 16:15:07 +0200 Message-ID: Subject: Re: [PATCH 2/3] mem: fix ASan shadow for remapped memory segments To: "Burakov, Anatoly" Cc: dev , "Mcnamara, John" , Dmitry Kozlyuk , dpdk stable , Xueqin Lin Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=dmarchan@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset="UTF-8" X-BeenThere: stable@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: patches for DPDK stable branches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: stable-bounces@dpdk.org On Tue, Apr 26, 2022 at 2:54 PM Burakov, Anatoly wrote: > >> @@ -1040,9 +1040,25 @@ malloc_heap_free(struct malloc_elem *elem) > >> > >> rte_mcfg_mem_write_unlock(); > >> free_unlock: > >> - /* Poison memory range if belonging to some still mapped > >> pages. */ > >> - if (!unmapped_pages) > >> + if (!unmapped_pages) { > >> asan_set_freezone(asan_ptr, asan_data_len); > >> + } else { > >> + /* > >> + * We may be in a situation where we unmapped pages > >> like this: > >> + * malloc header | free space | unmapped space | free > >> space | malloc header > >> + */ > >> + void *free1_start = asan_ptr; > >> + void *free1_end = aligned_start; > >> + void *free2_start = RTE_PTR_ADD(aligned_start, > >> aligned_len); > >> + void *free2_end = RTE_PTR_ADD(asan_ptr, asan_data_len); > >> + > >> + if (free1_start < free1_end) > >> + asan_set_freezone(free1_start, > >> + RTE_PTR_DIFF(free1_end, free1_start)); > >> + if (free2_start < free2_end) > >> + asan_set_freezone(free2_start, > >> + RTE_PTR_DIFF(free2_end, free2_start)); > >> + } > >> > >> rte_spinlock_unlock(&(heap->lock)); > >> return ret; > >> > > > > Something like that, yes. I will have to think through this a bit more, > > especially in light of your func_reentrancy splat :) > > > > So, the reason splat in func_reentrancy test happens is as follows: the > above patch is sorta correct (i have a different one but does the same > thing), but incomplete. What happens then is when we add new memory, we > are integrating it into our existing malloc heap, which triggers > `malloc_elem_join_adjacent_free()` which will trigger a write into old > header space being merged, which may be marked as "freed". So, again we > are hit with our internal allocator messing with ASan. I ended up with the same conclusion. Thanks for confirming. > > To properly fix this is to answer the following question: what is the > goal of having ASan support in DPDK? Is it there to catch bugs *in the > allocator*, or can we just trust that our allocator code is correct, and > only concern ourselves with user-allocated areas of the code? Because it The best would be to handle both. I don't think clang disables ASan for the instrumentations on malloc. > seems like the best way to address this issue would be to just avoid > triggering ASan checks for certain allocator-internal actions: this way, > we don't need to care what allocator itself does, just what user code > does. As in, IIRC there was a compiler attribute that disables ASan > checks for a specific function: perhaps we could just wrap certain > access in that and be done with it? > > What do you think? It is tempting because it is the easiest way to avoid the issue. Though, by waiving those checks in the allocator, does it leave the ASan shadow in a consistent state? -- David Marchand