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

["Burakov, Anatoly" <anatoly.burakov@intel.com>]

On 23-Apr-20 9:04 PM, David Marchand wrote:
> 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/
> 
> 

Oh, right, so this would probably not compile on FreeBSD. Perhaps this 
function would have to be OS-specific after all (or call into an 
OS-specific madvise() after reserving the memory area).

>>
>> 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 :-).

Nice to have that kind of power! *evil laugh*

> 
>>
>> @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.
> 
> 


-- 
Thanks,
Anatoly