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

Groups > linux.kernel > #1712508 > unrolled thread

Re: [PATCH 2/2] mm,fork: introduce MADV_WIPEONFORK

Started byAndrew Morton <akpm@linux-foundation.org>
First post2017-08-16 01:00 +0200
Last post2017-08-19 02:10 +0200
Articles 6 — 2 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH 2/2] mm,fork: introduce MADV_WIPEONFORK Andrew Morton <akpm@linux-foundation.org> - 2017-08-16 01:00 +0200
    Re: [PATCH 2/2] mm,fork: introduce MADV_WIPEONFORK Rik van Riel <riel@redhat.com> - 2017-08-16 04:20 +0200
      Re: [PATCH 2/2] mm,fork: introduce MADV_WIPEONFORK Andrew Morton <akpm@linux-foundation.org> - 2017-08-18 01:00 +0200
        Re: [PATCH 2/2] mm,fork: introduce MADV_WIPEONFORK Rik van Riel <riel@redhat.com> - 2017-08-18 18:30 +0200
          Re: [PATCH 2/2] mm,fork: introduce MADV_WIPEONFORK Andrew Morton <akpm@linux-foundation.org> - 2017-08-18 20:20 +0200
            Re: [PATCH 2/2] mm,fork: introduce MADV_WIPEONFORK Rik van Riel <riel@redhat.com> - 2017-08-19 02:10 +0200

#1712508 — Re: [PATCH 2/2] mm,fork: introduce MADV_WIPEONFORK

FromAndrew Morton <akpm@linux-foundation.org>
Date2017-08-16 01:00 +0200
SubjectRe: [PATCH 2/2] mm,fork: introduce MADV_WIPEONFORK
Message-ID<ueSYx-7mo-1@gated-at.bofh.it>
On Fri, 11 Aug 2017 17:28:29 -0400 riel@redhat.com wrote:

> From: Rik van Riel <riel@redhat.com>
> 
> 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.
> 
> If a child process accesses memory that was MADV_WIPEONFORK, it
> will get zeroes. The address ranges are still valid, they are just empty.
> 
> If a child process accesses memory that was MADV_DONTFORK, it will
> get a segmentation fault, since those address ranges are no longer
> valid in the child after fork.
> 
> Since MADV_DONTFORK also seems to be used to allow very large
> programs to fork in systems with strict memory overcommit restrictions,
> changing the semantics of MADV_DONTFORK might break existing programs.
> 
> MADV_WIPEONFORK only works on private, anonymous VMAs.
> 
> 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.
> 
> Examples of this would be:
> - systemd/pulseaudio API checks (fail after fork)
>   (replacing a getpid check, which is too slow without a PID cache)
> - PKCS#11 API reinitialization check (mandated by specification)
> - glibc's upcoming PRNG (reseed after fork)
> - OpenSSL PRNG (reseed after fork)
> 
> The security benefits of a forking server having a re-inialized
> PRNG in every child process are pretty obvious. However, due to
> libraries having all kinds of internal state, and programs getting
> compiled with many different versions of each library, it is
> unreasonable to expect calling programs to re-initialize everything
> manually after fork.
> 
> A further complication is the proliferation of clone flags,
> programs bypassing glibc's functions to call clone directly,
> and programs calling unshare, causing the glibc pthread_atfork
> hook to not get called.
> 
> It would be better to have the kernel take care of this automatically.

I'll add "The patch also adds MADV_KEEPONFORK, to undo the effects of a
prior MADV_WIPEONFORK." here.

I guess it isn't worth mentioning that these things can cause VMA
merges and splits. 

> --- a/mm/madvise.c
> +++ b/mm/madvise.c
> @@ -80,6 +80,17 @@ static long madvise_behavior(struct vm_area_struct *vma,
>  		}
>  		new_flags &= ~VM_DONTCOPY;
>  		break;
> +	case MADV_WIPEONFORK:
> +		/* MADV_WIPEONFORK is only supported on anonymous memory. */
> +		if (vma->vm_file || vma->vm_flags & VM_SHARED) {
> +			error = -EINVAL;
> +			goto out;
> +		}
> +		new_flags |= VM_WIPEONFORK;
> +		break;
> +	case MADV_KEEPONFORK:
> +		new_flags &= ~VM_WIPEONFORK;
> +		break;
>  	case MADV_DONTDUMP:
>  		new_flags |= VM_DONTDUMP;
>  		break;

It seems odd to permit MADV_KEEPONFORK against other-than-anon vmas?

[toc] | [next] | [standalone]

#1712597

FromRik van Riel <riel@redhat.com>
Date2017-08-16 04:20 +0200
Message-ID<ueW66-17t-11@gated-at.bofh.it>
In reply to#1712508
On Tue, 2017-08-15 at 15:51 -0700, Andrew Morton wrote:
> On Fri, 11 Aug 2017 17:28:29 -0400 riel@redhat.com wrote:
> 
> > A further complication is the proliferation of clone flags,
> > programs bypassing glibc's functions to call clone directly,
> > and programs calling unshare, causing the glibc pthread_atfork
> > hook to not get called.
> > 
> > It would be better to have the kernel take care of this
> > automatically.
> 
> I'll add "The patch also adds MADV_KEEPONFORK, to undo the effects of
> a
> prior MADV_WIPEONFORK." here.
> 
> I guess it isn't worth mentioning that these things can cause VMA
> merges and splits. 

That's the same as every other Linux specific madvise operation.

> > --- a/mm/madvise.c
> > +++ b/mm/madvise.c
> > @@ -80,6 +80,17 @@ static long madvise_behavior(struct
> > vm_area_struct *vma,
> >  		}
> >  		new_flags &= ~VM_DONTCOPY;
> >  		break;
> > +	case MADV_WIPEONFORK:
> > +		/* MADV_WIPEONFORK is only supported on anonymous
> > memory. */
> > +		if (vma->vm_file || vma->vm_flags & VM_SHARED) {
> > +			error = -EINVAL;
> > +			goto out;
> > +		}
> > +		new_flags |= VM_WIPEONFORK;
> > +		break;
> > +	case MADV_KEEPONFORK:
> > +		new_flags &= ~VM_WIPEONFORK;
> > +		break;
> >  	case MADV_DONTDUMP:
> >  		new_flags |= VM_DONTDUMP;
> >  		break;
> 
> It seems odd to permit MADV_KEEPONFORK against other-than-anon vmas?

Given that the only way to set VM_WIPEONFORK is through
MADV_WIPEONFORK, calling MADV_KEEPONFORK on an
other-than-anon vma would be equivalent to a noop.

If new_flags == vma->vm_flags, madvise_behavior() will
immediately exit.

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

#1714475

FromAndrew Morton <akpm@linux-foundation.org>
Date2017-08-18 01:00 +0200
Message-ID<ufBVD-2VV-3@gated-at.bofh.it>
In reply to#1712597
On Tue, 15 Aug 2017 22:18:19 -0400 Rik van Riel <riel@redhat.com> wrote:

> > > --- a/mm/madvise.c
> > > +++ b/mm/madvise.c
> > > @@ -80,6 +80,17 @@ static long madvise_behavior(struct
> > > vm_area_struct *vma,
> > > __		}
> > > __		new_flags &= ~VM_DONTCOPY;
> > > __		break;
> > > +	case MADV_WIPEONFORK:
> > > +		/* MADV_WIPEONFORK is only supported on anonymous
> > > memory. */
> > > +		if (vma->vm_file || vma->vm_flags & VM_SHARED) {
> > > +			error = -EINVAL;
> > > +			goto out;
> > > +		}
> > > +		new_flags |= VM_WIPEONFORK;
> > > +		break;
> > > +	case MADV_KEEPONFORK:
> > > +		new_flags &= ~VM_WIPEONFORK;
> > > +		break;
> > > __	case MADV_DONTDUMP:
> > > __		new_flags |= VM_DONTDUMP;
> > > __		break;
> > 
> > It seems odd to permit MADV_KEEPONFORK against other-than-anon vmas?
> 
> Given that the only way to set VM_WIPEONFORK is through
> MADV_WIPEONFORK, calling MADV_KEEPONFORK on an
> other-than-anon vma would be equivalent to a noop.
> 
> If new_flags == vma->vm_flags, madvise_behavior() will
> immediately exit.

Yes, but calling MADV_WIPEONFORK against an other-than-anon vma is
presumably a userspace bug.  A bug which will probably result in
userspace having WIPEONFORK memory which it didn't want.  The kernel
can trivially tell userspace that it has this bug so why not do so?

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

#1715305

FromRik van Riel <riel@redhat.com>
Date2017-08-18 18:30 +0200
Message-ID<ufSjL-6dn-3@gated-at.bofh.it>
In reply to#1714475
On Thu, 2017-08-17 at 15:50 -0700, Andrew Morton wrote:
> On Tue, 15 Aug 2017 22:18:19 -0400 Rik van Riel <riel@redhat.com>
> wrote:
> 
> > > > --- a/mm/madvise.c
> > > > +++ b/mm/madvise.c
> > > > @@ -80,6 +80,17 @@ static long madvise_behavior(struct
> > > > vm_area_struct *vma,
> > > > __		}
> > > > __		new_flags &= ~VM_DONTCOPY;
> > > > __		break;
> > > > +	case MADV_WIPEONFORK:
> > > > +		/* MADV_WIPEONFORK is only supported on
> > > > anonymous
> > > > memory. */
> > > > +		if (vma->vm_file || vma->vm_flags & VM_SHARED)
> > > > {
> > > > +			error = -EINVAL;
> > > > +			goto out;
> > > > +		}
> > > > +		new_flags |= VM_WIPEONFORK;
> > > > +		break;
> > > > +	case MADV_KEEPONFORK:
> > > > +		new_flags &= ~VM_WIPEONFORK;
> > > > +		break;
> > > > __	case MADV_DONTDUMP:
> > > > __		new_flags |= VM_DONTDUMP;
> > > > __		break;
> > > 
> > > It seems odd to permit MADV_KEEPONFORK against other-than-anon
> > > vmas?
> > 
> > Given that the only way to set VM_WIPEONFORK is through
> > MADV_WIPEONFORK, calling MADV_KEEPONFORK on an
> > other-than-anon vma would be equivalent to a noop.
> > 
> > If new_flags == vma->vm_flags, madvise_behavior() will
> > immediately exit.
> 
> Yes, but calling MADV_WIPEONFORK against an other-than-anon vma is
> presumably a userspace bug.  A bug which will probably result in
> userspace having WIPEONFORK memory which it didn't want.  The kernel
> can trivially tell userspace that it has this bug so why not do so?

Uh, what?

Calling MADV_WIPEONFORK on an other-than-anon vma results
in NOT getting VM_WIPEONFORK semantics on that VMA.

The code you are commenting on is the bit that CLEARS
the VM_WIPEONFORK code, not the bit where it gets set.

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

#1715363

FromAndrew Morton <akpm@linux-foundation.org>
Date2017-08-18 20:20 +0200
Message-ID<ufU2e-7lN-11@gated-at.bofh.it>
In reply to#1715305
On Fri, 18 Aug 2017 12:28:29 -0400 Rik van Riel <riel@redhat.com> wrote:

> On Thu, 2017-08-17 at 15:50 -0700, Andrew Morton wrote:
> > On Tue, 15 Aug 2017 22:18:19 -0400 Rik van Riel <riel@redhat.com>
> > wrote:
> > 
> > > > > --- a/mm/madvise.c
> > > > > +++ b/mm/madvise.c
> > > > > @@ -80,6 +80,17 @@ static long madvise_behavior(struct
> > > > > vm_area_struct *vma,
> > > > > __		}
> > > > > __		new_flags &= ~VM_DONTCOPY;
> > > > > __		break;
> > > > > +	case MADV_WIPEONFORK:
> > > > > +		/* MADV_WIPEONFORK is only supported on
> > > > > anonymous
> > > > > memory. */
> > > > > +		if (vma->vm_file || vma->vm_flags & VM_SHARED)
> > > > > {
> > > > > +			error = -EINVAL;
> > > > > +			goto out;
> > > > > +		}
> > > > > +		new_flags |= VM_WIPEONFORK;
> > > > > +		break;
> > > > > +	case MADV_KEEPONFORK:
> > > > > +		new_flags &= ~VM_WIPEONFORK;
> > > > > +		break;
> > > > > __	case MADV_DONTDUMP:
> > > > > __		new_flags |= VM_DONTDUMP;
> > > > > __		break;
> > > > 
> > > > It seems odd to permit MADV_KEEPONFORK against other-than-anon
> > > > vmas?
> > > 
> > > Given that the only way to set VM_WIPEONFORK is through
> > > MADV_WIPEONFORK, calling MADV_KEEPONFORK on an
> > > other-than-anon vma would be equivalent to a noop.
> > > 
> > > If new_flags == vma->vm_flags, madvise_behavior() will
> > > immediately exit.
> > 
> > Yes, but calling MADV_WIPEONFORK against an other-than-anon vma is
> > presumably a userspace bug.____A bug which will probably result in
> > userspace having WIPEONFORK memory which it didn't want.____The kernel
> > can trivially tell userspace that it has this bug so why not do so?
> 
> Uh, what?
> 

Braino.  I meant MADV_KEEPONFORK.  Calling MADV_KEEPONFORK against an
other-than-anon vma is a presumptive userspace bug and the kernel
should report that.

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

#1715516

FromRik van Riel <riel@redhat.com>
Date2017-08-19 02:10 +0200
Message-ID<ufZuV-2rm-5@gated-at.bofh.it>
In reply to#1715363
On Fri, 2017-08-18 at 11:15 -0700, Andrew Morton wrote:
> On Fri, 18 Aug 2017 12:28:29 -0400 Rik van Riel <riel@redhat.com>
> wrote:
> 
> > On Thu, 2017-08-17 at 15:50 -0700, Andrew Morton wrote:
> > > On Tue, 15 Aug 2017 22:18:19 -0400 Rik van Riel <riel@redhat.com>
> > > wrote:
> > > 
> > > > > > --- a/mm/madvise.c
> > > > > > +++ b/mm/madvise.c
> > > > > > @@ -80,6 +80,17 @@ static long madvise_behavior(struct
> > > > > > vm_area_struct *vma,
> > > > > > __		}
> > > > > > __		new_flags &= ~VM_DONTCOPY;
> > > > > > __		break;
> > > > > > +	case MADV_WIPEONFORK:
> > > > > > +		/* MADV_WIPEONFORK is only supported on
> > > > > > anonymous
> > > > > > memory. */
> > > > > > +		if (vma->vm_file || vma->vm_flags &
> > > > > > VM_SHARED)
> > > > > > {
> > > > > > +			error = -EINVAL;
> > > > > > +			goto out;
> > > > > > +		}
> > > > > > +		new_flags |= VM_WIPEONFORK;
> > > > > > +		break;
> > > > > > +	case MADV_KEEPONFORK:
> > > > > > +		new_flags &= ~VM_WIPEONFORK;
> > > > > > +		break;
> > > > > > __	case MADV_DONTDUMP:
> > > > > > __		new_flags |= VM_DONTDUMP;
> > > > > > __		break;
> > > > > 
> > > > > It seems odd to permit MADV_KEEPONFORK against other-than-
> > > > > anon
> > > > > vmas?
> > > > 
> > > > Given that the only way to set VM_WIPEONFORK is through
> > > > MADV_WIPEONFORK, calling MADV_KEEPONFORK on an
> > > > other-than-anon vma would be equivalent to a noop.
> > > > 
> > > > If new_flags == vma->vm_flags, madvise_behavior() will
> > > > immediately exit.
> > > 
> > > Yes, but calling MADV_WIPEONFORK against an other-than-anon vma
> > > is
> > > presumably a userspace bug.____A bug which will probably result
> > > in
> > > userspace having WIPEONFORK memory which it didn't want.____The
> > > kernel
> > > can trivially tell userspace that it has this bug so why not do
> > > so?
> > 
> > Uh, what?
> > 
> 
> Braino.  I meant MADV_KEEPONFORK.  Calling MADV_KEEPONFORK against an
> other-than-anon vma is a presumptive userspace bug and the kernel
> should report that.

All MADV_KEEPONFORK does is clear the flag set by
MADV_WIPEONFORK. Since there is no way to set the
WIPEONFORK flag on other-than-anon VMAs, that means
MADV_KEEPONFORK is always a noop for those VMAs.

You remind me that I should send in a man page
addition, though, when this code gets sent to
Linus.

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web