Re: [dpdk-dev] [PATCH v2] eal: add madvise to avoid dump memory

[David Marchand <david.marchand@redhat.com>]

On Thu, Apr 23, 2020 at 6:34 PM Burakov, Anatoly
<anatoly.burakov@intel.com> wrote:
> > diff --git a/lib/librte_eal/common/eal_common_memory.c b/lib/librte_eal/common/eal_common_memory.c
> > index cc7d54e0c..2d9564b28 100644
> > --- a/lib/librte_eal/common/eal_common_memory.c
> > +++ b/lib/librte_eal/common/eal_common_memory.c
> > @@ -177,6 +177,20 @@ eal_get_virtual_area(void *requested_addr, size_t *size,
> >               after_len = RTE_PTR_DIFF(map_end, aligned_end);
> >               if (after_len > 0)
> >                       munmap(aligned_end, after_len);
> > +
> > +             /*
> > +              * Exclude this pages from a core dump.
> > +              */
> > +             if (madvise(aligned_addr, *size, MADV_DONTDUMP) != 0)
> > +                     RTE_LOG(WARNING, EAL, "Madvise with MADV_DONTDUMP failed: %s\n",
> > +                             strerror(errno));> +   } else {
> > +             /*
> > +              * Exclude this pages from a core dump.
> > +              */
> > +             if (madvise(mapped_addr, map_sz, MADV_DONTDUMP) != 0)
> > +                     RTE_LOG(WARNING, EAL, "Madvise with MADV_DONTDUMP failed: %s\n",
> > +                             strerror(errno));
> >       }
> >
> >       return aligned_addr;
> >
>
> For the contents of this patch,

MADV_DONTDUMP does not seem POSIX, but as I said [1], there seems to
be a MADV_NOCORE option on FreeBSD.
1: http://inbox.dpdk.org/dev/CAJFAV8y9YtT-7njUz+mD6U8+3XUqYrgp28KD7jy2923EpAcXrg@mail.gmail.com/


>
> Acked-by: Anatoly Burakov <anatoly.burakov@intel.com>
>
> However, even though this is good to have, after some more thought, i
> believe the fix is incomplete, because this is not the only place we're
> reserving anonymous memory. We're also doing so in
> `eal_memalloc.c:free_seg()`, so an `madvise()` call should also be added
> there.
>
> @David, now that i think of it, the PROT_NONE patch also was incomplete,
> as we only set PROT_NONE to memory that's initially reserved, but not
> when it's unmapped and returned back to the pool of anonymous memory.
> So, eal_memalloc.c should also remap anonymous memory with PROT_NONE.

I can't disagree if you say so :-).

>
> @Li Feng, would you be so kind as to provide a patch replacing PROT_READ
> with PROT_NONE in eal_memalloc.c as well? Thank you very much!
>

Once we have the proper fixes, I'd like to get this Cc: stable@dpdk.org.
Thanks.


-- 
David Marchand