Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1704853 > unrolled thread

[PATCH v2 0/2] mm,fork,security: introduce MADV_WIPEONFORK

Back to article view | Back to linux.kernel

Contents

  [PATCH v2 0/2] mm,fork,security: introduce MADV_WIPEONFORK riel@redhat.com - 2017-08-06 16:10 +0200
    Re: [PATCH v2 0/2] mm,fork,security: introduce MADV_WIPEONFORK Michal Hocko <mhocko@kernel.org> - 2017-08-07 15:30 +0200
      Re: [PATCH v2 0/2] mm,fork,security: introduce MADV_WIPEONFORK Michal Hocko <mhocko@kernel.org> - 2017-08-07 15:50 +0200
        Re: [PATCH v2 0/2] mm,fork,security: introduce MADV_WIPEONFORK Florian Weimer <fweimer@redhat.com> - 2017-08-07 16:20 +0200
          Re: [PATCH v2 0/2] mm,fork,security: introduce MADV_WIPEONFORK Michal Hocko <mhocko@kernel.org> - 2017-08-10 15:10 +0200
        Re: [PATCH v2 0/2] mm,fork,security: introduce MADV_WIPEONFORK Rik van Riel <riel@redhat.com> - 2017-08-07 17:10 +0200
          Re: [PATCH v2 0/2] mm,fork,security: introduce MADV_WIPEONFORK "Kirill A. Shutemov" <kirill@shutemov.name> - 2017-08-09 12:10 +0200
            Re: [PATCH v2 0/2] mm,fork,security: introduce MADV_WIPEONFORK Rik van Riel <riel@redhat.com> - 2017-08-09 14:40 +0200
            Re: [PATCH v2 0/2] mm,fork,security: introduce MADV_WIPEONFORK Florian Weimer <fweimer@redhat.com> - 2017-08-09 14:50 +0200
          Re: [PATCH v2 0/2] mm,fork,security: introduce MADV_WIPEONFORK Michal Hocko <mhocko@kernel.org> - 2017-08-10 15:10 +0200
            Re: [PATCH v2 0/2] mm,fork,security: introduce MADV_WIPEONFORK Colm MacCárthaigh <colm@allcosts.net> - 2017-08-10 15:30 +0200
              Re: [PATCH v2 0/2] mm,fork,security: introduce MADV_WIPEONFORK Michal Hocko <mhocko@kernel.org> - 2017-08-10 17:40 +0200
                Re: [PATCH v2 0/2] mm,fork,security: introduce MADV_WIPEONFORK Michal Hocko <mhocko@kernel.org> - 2017-08-10 19:10 +0200
                  Re: [PATCH v2 0/2] mm,fork,security: introduce MADV_WIPEONFORK Colm MacCárthaigh <colm@allcosts.net> - 2017-08-11 00:20 +0200
                    Re: [PATCH v2 0/2] mm,fork,security: introduce MADV_WIPEONFORK Michal Hocko <mhocko@kernel.org> - 2017-08-11 16:10 +0200
                      Re: [PATCH v2 0/2] mm,fork,security: introduce MADV_WIPEONFORK Florian Weimer <fweimer@redhat.com> - 2017-08-11 16:20 +0200
                        Re: [PATCH v2 0/2] mm,fork,security: introduce MADV_WIPEONFORK Michal Hocko <mhocko@kernel.org> - 2017-08-11 16:30 +0200
                          Re: [PATCH v2 0/2] mm,fork,security: introduce MADV_WIPEONFORK Florian Weimer <fweimer@redhat.com> - 2017-08-11 17:30 +0200
                            Re: [PATCH v2 0/2] mm,fork,security: introduce MADV_WIPEONFORK Michal Hocko <mhocko@kernel.org> - 2017-08-11 17:40 +0200
        Re: [PATCH v2 0/2] mm,fork,security: introduce MADV_WIPEONFORK Colm MacCárthaigh <colm@allcosts.net> - 2017-08-07 18:10 +0200
        Re: [PATCH v2 0/2] mm,fork,security: introduce MADV_WIPEONFORK Michal Hocko <mhocko@kernel.org> - 2017-08-10 15:30 +0200
          Re: [PATCH v2 0/2] mm,fork,security: introduce MADV_WIPEONFORK Michal Hocko <mhocko@kernel.org> - 2017-08-10 16:20 +0200
    Re: [PATCH v2 0/2] mm,fork,security: introduce MADV_WIPEONFORK Mike Kravetz <mike.kravetz@oracle.com> - 2017-08-07 20:30 +0200
      Re: [PATCH v2 0/2] mm,fork,security: introduce MADV_WIPEONFORK Florian Weimer <fweimer@redhat.com> - 2017-08-08 12:00 +0200
        Re: [PATCH v2 0/2] mm,fork,security: introduce MADV_WIPEONFORK Rik van Riel <riel@redhat.com> - 2017-08-08 15:20 +0200
          Re: [PATCH v2 0/2] mm,fork,security: introduce MADV_WIPEONFORK Mike Kravetz <mike.kravetz@oracle.com> - 2017-08-08 17:30 +0200
            Re: [PATCH v2 0/2] mm,fork,security: introduce MADV_WIPEONFORK Florian Weimer <fweimer@redhat.com> - 2017-08-08 17:30 +0200
            Re: [PATCH v2 0/2] mm,fork,security: introduce MADV_WIPEONFORK Rik van Riel <riel@redhat.com> - 2017-08-08 17:50 +0200
              Re: [PATCH v2 0/2] mm,fork,security: introduce MADV_WIPEONFORK Colm MacCárthaigh <colm@allcosts.net> - 2017-08-08 18:50 +0200
              Re: [PATCH v2 0/2] mm,fork,security: introduce MADV_WIPEONFORK Matthew Wilcox <willy@infradead.org> - 2017-08-08 19:00 +0200
                Re: [PATCH v2 0/2] mm,fork,security: introduce MADV_WIPEONFORK Rik van Riel <riel@redhat.com> - 2017-08-08 20:50 +0200
                  Re: [PATCH v2 0/2] mm,fork,security: introduce MADV_WIPEONFORK Michal Hocko <mhocko@kernel.org> - 2017-08-10 17:40 +0200

Page 2 of 2 — ← Prev page 1 [2]

#1708629

On Mon 07-08-17 17:55:45, Colm MacCárthaigh wrote:
> On Mon, Aug 7, 2017 at 3:46 PM, Michal Hocko <mhocko@kernel.org> wrote:
> 
> >
> > > > The use case is libraries that store or cache information, and
> > > > want to know that they need to regenerate it in the child process
> > > > after fork.
> >
> > How do they know that they need to regenerate if they do not get SEGV?
> > Are they going to assume that a read of zeros is a "must init again"? Isn't
> > that too fragile? Or do they play other tricks like parse /proc/self/smaps
> > and read in the flag?
> >
> 
> Hi from a user space crypto maintainer :) Here's how we do exactly this it
> in s2n:
> 
> https://github.com/awslabs/s2n/blob/master/utils/s2n_random.c , lines 62 -
> 91
> 
> and here's how LibreSSL does it:
> 
> https://github.com/libressl-portable/openbsd/blob/57dcd4329d83bff3dd67a293d5c4a53b795c587e/src/lib/libc/crypt/arc4random.h
> (lines 37 on)
> https://github.com/libressl-portable/openbsd/blob/57dcd4329d83bff3dd67a293d5c4a53b795c587e/src/lib/libc/crypt/arc4random.c
> (Line 110)
> 
> OpenSSL and libc are in the process of adding similar DRBGs and would use a
> WIPEONFORK. BoringSSL's maintainers are also interested as it adds
> robustness.  I also recall it being a topic of discussion at the High
> Assurance Cryptography Symposium (HACS) where many crypto maintainers meet
> and several more maintainers there indicated it would be nice to have.
> 
> Right now on Linux we all either use pthread_atfork() to zero the memory on
> fork, or getpid() and getppid() guards. The former can be evaded by direct
> syscall() and other tricks (which things like Language VMs are prone to
> doing), and the latter check is probabilistic as pids can repeat, though if
> you use both getpid() and getppid() - which is slow! - the probability of
> both PIDs colliding is very low indeed.

Thanks, these references are really useful to build a picture. I would
probably use an unlinked fd with O_CLOEXEC to dect this but I can see
how this is not the greatest option for a library.

> The result at the moment on Linux there's no bulletproof way to detect a
> fork and erase a key or DRBG state. It would really be nice to be able to
> match what we can do with MAP_INHERIT_ZERO and minherit() on BSD.
>  madvise() does seem like the established idiom for behavior like this on
> Linux.  I don't imagine it will be hard to use in practice, we can fall
> back to existing behavior if the flag isn't accepted.

The reason why I dislike madvise, as already said, is that it should be
an advise rather than something correctness related. Sure we do have
some exceptions there but that doesn't mean we should repeat the same
error. If anything an mmap MAP_$FOO sounds like a better approach to me.

-- 
Michal Hocko
SUSE Labs

[toc] | [prev] | [next] | [standalone]

#1708669

On Thu 10-08-17 15:21:10, Michal Hocko wrote:
[...]
> Thanks, these references are really useful to build a picture. I would
> probably use an unlinked fd with O_CLOEXEC to dect this but I can see
> how this is not the greatest option for a library.

Blee, brainfart on my end. For some reason I mixed fork/exec...
-- 
Michal Hocko
SUSE Labs

[toc] | [prev] | [next] | [standalone]

#1705772

On 08/06/2017 07:04 AM, riel@redhat.com wrote:
> v2: fix MAP_SHARED case and kbuild warnings
> 
> Introduce MADV_WIPEONFORK semantics, which result in a VMA being
> empty in the child process after fork. This differs from MADV_DONTFORK
> in one important way.

It seems that the target use case might be private anonymous mappings.
If a shared or file backed mapping exists, one would assume that it
was created with the intention of sharing, even across fork.  So,
setting MADV_DONTFORK on such a mapping seems to change the meaning
and conflict with the original intention of the mapping.

If my thoughts above are correct, what about returning EINVAL if one
attempts to set MADV_DONTFORK on mappings set up for sharing?

If not, and you really want this to be applicable to all mappings, then
you should be more specific about what happens at fork time.  Do they
all get turned into anonymous mappings?  What happens to file references?
What about the really ugly case of hugetlb mappings?  Do they get
'transformed' to non-hugetlb mappings?  Or, do you create a separate
hugetlb mapping for the child?

-- 
Mike Kravetz

[toc] | [prev] | [next] | [standalone]

#1706278

On 08/07/2017 08:23 PM, Mike Kravetz wrote:
> If my thoughts above are correct, what about returning EINVAL if one
> attempts to set MADV_DONTFORK on mappings set up for sharing?

That's my preference as well.  If there is a use case for shared or
non-anonymous mappings, then we can implement MADV_DONTFORK with the
semantics for this use case.  If we pick some arbitrary semantics now,
without any use case, we might end up with something that's not actually
useful.

Thanks,
Florian

[toc] | [prev] | [next] | [standalone]

#1706534

On Tue, 2017-08-08 at 11:58 +0200, Florian Weimer wrote:
> On 08/07/2017 08:23 PM, Mike Kravetz wrote:
> > If my thoughts above are correct, what about returning EINVAL if
> > one
> > attempts to set MADV_DONTFORK on mappings set up for sharing?
> 
> That's my preference as well.  If there is a use case for shared or
> non-anonymous mappings, then we can implement MADV_DONTFORK with the
> semantics for this use case.  If we pick some arbitrary semantics
> now,
> without any use case, we might end up with something that's not
> actually
> useful.

MADV_DONTFORK is existing semantics, and it is enforced
on shared, non-anonymous mappings. It is frequently used
for things like device mappings, which should not be
inherited by a child process, because the device can only
be used by one process at a time.

When someone requests MADV_DONTFORK on a shared VMA, they
will get it. The later madvise request overrides the mmap
flags that were used earlier.

The question is, should MADV_WIPEONFORK (introduced by
this series) have not just different semantics, but also
totally different behavior from MADV_DONTFORK?

Does the principle of least surprise dictate that the
last request determines the policy on an area, or should
later requests not be able to override policy that was
set at mmap time?

[toc] | [prev] | [next] | [standalone]

#1706647

On 08/08/2017 06:15 AM, Rik van Riel wrote:
> On Tue, 2017-08-08 at 11:58 +0200, Florian Weimer wrote:
>> On 08/07/2017 08:23 PM, Mike Kravetz wrote:
>>> If my thoughts above are correct, what about returning EINVAL if
>>> one
>>> attempts to set MADV_DONTFORK on mappings set up for sharing?
>>
>> That's my preference as well.  If there is a use case for shared or
>> non-anonymous mappings, then we can implement MADV_DONTFORK with the
>> semantics for this use case.  If we pick some arbitrary semantics
>> now,
>> without any use case, we might end up with something that's not
>> actually
>> useful.
> 
> MADV_DONTFORK is existing semantics, and it is enforced
> on shared, non-anonymous mappings. It is frequently used
> for things like device mappings, which should not be
> inherited by a child process, because the device can only
> be used by one process at a time.
> 
> When someone requests MADV_DONTFORK on a shared VMA, they
> will get it. The later madvise request overrides the mmap
> flags that were used earlier.
> 
> The question is, should MADV_WIPEONFORK (introduced by
> this series) have not just different semantics, but also
> totally different behavior from MADV_DONTFORK?

Sorry for the confusion.  I accidentally used MADV_DONTFORK instead
of MADV_WIPEONFORK in my reply (which Florian commented on).

> Does the principle of least surprise dictate that the
> last request determines the policy on an area, or should
> later requests not be able to override policy that was
> set at mmap time?

That is the question.

The other question I was trying to bring up is "What does MADV_WIPEONFORK
mean for various types of mappings?"  For example, if we allow
MADV_WIPEONFORK on a file backed mapping what does that mapping look
like in the child after fork?  Does it have any connection at all to the
file?  Or, do we drop all references to the file and essentially transform
it to a private (or shared?) anonymous mapping after fork.  What about
System V shared memory?  What about hugetlb?

If the use case is fairly specific, then perhaps it makes sense to
make MADV_WIPEONFORK not applicable (EINVAL) for mappings where the
result is 'questionable'.

-- 
Mike Kravetz

[toc] | [prev] | [next] | [standalone]

#1706650

On 08/08/2017 05:19 PM, Mike Kravetz wrote:
> On 08/08/2017 06:15 AM, Rik van Riel wrote:
>> On Tue, 2017-08-08 at 11:58 +0200, Florian Weimer wrote:
>>> On 08/07/2017 08:23 PM, Mike Kravetz wrote:
>>>> If my thoughts above are correct, what about returning EINVAL if
>>>> one
>>>> attempts to set MADV_DONTFORK on mappings set up for sharing?
>>>
>>> That's my preference as well.  If there is a use case for shared or
>>> non-anonymous mappings, then we can implement MADV_DONTFORK with the
>>> semantics for this use case.  If we pick some arbitrary semantics
>>> now,
>>> without any use case, we might end up with something that's not
>>> actually
>>> useful.
>>
>> MADV_DONTFORK is existing semantics, and it is enforced
>> on shared, non-anonymous mappings. It is frequently used
>> for things like device mappings, which should not be
>> inherited by a child process, because the device can only
>> be used by one process at a time.
>>
>> When someone requests MADV_DONTFORK on a shared VMA, they
>> will get it. The later madvise request overrides the mmap
>> flags that were used earlier.
>>
>> The question is, should MADV_WIPEONFORK (introduced by
>> this series) have not just different semantics, but also
>> totally different behavior from MADV_DONTFORK?
> 
> Sorry for the confusion.  I accidentally used MADV_DONTFORK instead
> of MADV_WIPEONFORK in my reply (which Florian commented on).

Yes, I made the same mistake.  I meant MADV_WIPEONFORK as well.

Florian

[toc] | [prev] | [next] | [standalone]

#1706690

On Tue, 2017-08-08 at 08:19 -0700, Mike Kravetz wrote:

> The other question I was trying to bring up is "What does
> MADV_WIPEONFORK
> mean for various types of mappings?"  For example, if we allow
> MADV_WIPEONFORK on a file backed mapping what does that mapping look
> like in the child after fork?  Does it have any connection at all to
> the
> file?  Or, do we drop all references to the file and essentially
> transform
> it to a private (or shared?) anonymous mapping after fork.  What
> about
> System V shared memory?  What about hugetlb?

My current patch turns any file-backed VMA into an empty
anonymous VMA if MADV_WIPEONFORK was used on that VMA.

> If the use case is fairly specific, then perhaps it makes sense to
> make MADV_WIPEONFORK not applicable (EINVAL) for mappings where the
> result is 'questionable'.

That would be a question for Florian and Colm.

If they are OK with MADV_WIPEONFORK only working on
anonymous VMAs (no file mapping), that certainly could
be implemented.

On the other hand, I am not sure that introducing cases
where MADV_WIPEONFORK does not implement wipe-on-fork
semantics would reduce user confusion...

[toc] | [prev] | [next] | [standalone]

#1706755

On Tue, Aug 8, 2017 at 5:46 PM, Rik van Riel <riel@redhat.com> wrote:

>> If the use case is fairly specific, then perhaps it makes sense to
>> make MADV_WIPEONFORK not applicable (EINVAL) for mappings where the
>> result is 'questionable'.
>
> That would be a question for Florian and Colm.
>
> If they are OK with MADV_WIPEONFORK only working on
> anonymous VMAs (no file mapping), that certainly could
> be implemented.

Anonymous would be sufficient for all of the Crypto-cases that I've
come across. But I can imagine someone wanting to initialize all
application state from a saved file, or share it between processes.

The comparable minherit call sidesteps all of this by simply
documenting that it results in a new anonymous page after fork, and so
the previous state doesn't matter.

Maybe the problem here is the poor name (my fault). WIPEONFORK
suggests an action being taken ... like a user might think that it
literally zeroes a file, for example.  At the risk of bike shedding:
maybe ZEROESONFORK would resolve that small ambiguity?

-- 
Colm

[toc] | [prev] | [next] | [standalone]

#1706763

On Tue, Aug 08, 2017 at 11:46:08AM -0400, Rik van Riel wrote:
> On Tue, 2017-08-08 at 08:19 -0700, Mike Kravetz wrote:
> > If the use case is fairly specific, then perhaps it makes sense to
> > make MADV_WIPEONFORK not applicable (EINVAL) for mappings where the
> > result is 'questionable'.
> 
> That would be a question for Florian and Colm.
> 
> If they are OK with MADV_WIPEONFORK only working on
> anonymous VMAs (no file mapping), that certainly could
> be implemented.
> 
> On the other hand, I am not sure that introducing cases
> where MADV_WIPEONFORK does not implement wipe-on-fork
> semantics would reduce user confusion...

It'll simply do exactly what it does today, so it won't introduce any
new fallback code.

[toc] | [prev] | [next] | [standalone]

#1706824

On Tue, 2017-08-08 at 09:52 -0700, Matthew Wilcox wrote:
> On Tue, Aug 08, 2017 at 11:46:08AM -0400, Rik van Riel wrote:
> > On Tue, 2017-08-08 at 08:19 -0700, Mike Kravetz wrote:
> > > If the use case is fairly specific, then perhaps it makes sense
> > > to
> > > make MADV_WIPEONFORK not applicable (EINVAL) for mappings where
> > > the
> > > result is 'questionable'.
> > 
> > That would be a question for Florian and Colm.
> > 
> > If they are OK with MADV_WIPEONFORK only working on
> > anonymous VMAs (no file mapping), that certainly could
> > be implemented.
> > 
> > On the other hand, I am not sure that introducing cases
> > where MADV_WIPEONFORK does not implement wipe-on-fork
> > semantics would reduce user confusion...
> 
> It'll simply do exactly what it does today, so it won't introduce any
> new fallback code.

Sure, but actually implementing MADV_WIPEONFORK in a
way that turns file mapped VMAs into zero page backed
anonymous VMAs after fork takes no more code than
implementing it in a way that refuses to work on VMAs
that have a file backing.

There is no complexity argument for or against either
approach.

The big question is, what is the best for users?

Should we return -EINVAL when MADV_WIPEONFORK is called
on a VMA that has a file backing, and only succeed on
anonymous VMAs?

Or, should we simply turn every memory range that has
MADV_WIPEONFORK done to it into an anonymous VMA in the
child process?

[toc] | [prev] | [next] | [standalone]

#1708742

On Tue 08-08-17 14:45:14, Rik van Riel wrote:
> On Tue, 2017-08-08 at 09:52 -0700, Matthew Wilcox wrote:
> > On Tue, Aug 08, 2017 at 11:46:08AM -0400, Rik van Riel wrote:
> > > On Tue, 2017-08-08 at 08:19 -0700, Mike Kravetz wrote:
> > > > If the use case is fairly specific, then perhaps it makes sense
> > > > to
> > > > make MADV_WIPEONFORK not applicable (EINVAL) for mappings where
> > > > the
> > > > result is 'questionable'.
> > > 
> > > That would be a question for Florian and Colm.
> > > 
> > > If they are OK with MADV_WIPEONFORK only working on
> > > anonymous VMAs (no file mapping), that certainly could
> > > be implemented.
> > > 
> > > On the other hand, I am not sure that introducing cases
> > > where MADV_WIPEONFORK does not implement wipe-on-fork
> > > semantics would reduce user confusion...
> > 
> > It'll simply do exactly what it does today, so it won't introduce any
> > new fallback code.
> 
> Sure, but actually implementing MADV_WIPEONFORK in a
> way that turns file mapped VMAs into zero page backed
> anonymous VMAs after fork takes no more code than
> implementing it in a way that refuses to work on VMAs
> that have a file backing.
> 
> There is no complexity argument for or against either
> approach.
> 
> The big question is, what is the best for users?
> 
> Should we return -EINVAL when MADV_WIPEONFORK is called
> on a VMA that has a file backing, and only succeed on
> anonymous VMAs?

I would rather be conservative and implement the bare minimum until
there is a reasonable usecase to demand the feature for shared mappings
as well.
-- 
Michal Hocko
SUSE Labs

[toc] | [prev] | [standalone]

Page 2 of 2 — ← Prev page 1 [2]

Back to top | Article view | linux.kernel

csiph-web