Re: KAUTH_SYSTEM_UNENCRYPTED_SWAP

2020-05-16 Thread Greg Troxel
Kamil Rytarowski  writes:

> Is it possible to avoid negation in the name?
>
> KAUTH_SYSTEM_ENABLE_SWAP_ENCRYPTION

I think the point is to have one permission to enable it, which is
perhaps just regular root, and another to disable it if securelevel is
elevated.

So perhaps there should be two names, one to enable, one to disable.


Re: KAUTH_SYSTEM_UNENCRYPTED_SWAP

2020-05-16 Thread Kamil Rytarowski
Is it possible to avoid negation in the name?

KAUTH_SYSTEM_ENABLE_SWAP_ENCRYPTION

On 17.05.2020 00:51, Paul Goyette wrote:
> I'm not sure I like the name!
> 
> Can you call it KAUTH_SYSTEM_DISABLE_SWAPENCRYPT ?  That more
> closely describes the action which is being controlled.
> 
> 
> On Sat, 16 May 2020, Alexander Nasonov wrote:
> 
>> Attached patch adds KAUTH_SYSTEM_UNENCRYPTED_SWAP and
>> it forbids changing vm.swap_encrypt from 1 to 0 when
>> securelevel > 0.
>>
>> If there are no objections, I'm going to commit it tomorrow.
>>
>> -- 
>> Alex
>>
>>
>> !DSPAM:5ec06ddb24841398742664!
>>
> 
> ++--+---+
> | Paul Goyette   | PGP Key fingerprint: | E-mail addresses: |
> | (Retired)  | FA29 0E3B 35AF E8AE 6651 | p...@whooppee.com |
> | Software Developer | 0786 F758 55DE 53BA 7731 | pgoye...@netbsd.org   |
> ++--+---+




signature.asc
Description: OpenPGP digital signature


Re: KAUTH_SYSTEM_UNENCRYPTED_SWAP

2020-05-16 Thread Alexander Nasonov
m...@netbsd.org wrote:
> No objections from me, but I feel like "will commit unless objected"
> should be done on longer time scales. I spend way too much time on
> netbsd and I still have some days I dont get to reading email for
> whatever reason.

It's a small change, we discussed it on source-changes-d and
current-users already. I could have committed it without sending
the patch but I introduce a new public constant and I wasn't
very sure how to best name it.

-- 
Alex


Re: KAUTH_SYSTEM_UNENCRYPTED_SWAP

2020-05-16 Thread maya
On Sat, May 16, 2020 at 11:53:02PM +0100, Alexander Nasonov wrote:
> Attached patch adds KAUTH_SYSTEM_UNENCRYPTED_SWAP and
> it forbids changing vm.swap_encrypt from 1 to 0 when
> securelevel > 0.
> 
> If there are no objections, I'm going to commit it tomorrow.

No objections from me, but I feel like "will commit unless objected"
should be done on longer time scales. I spend way too much time on
netbsd and I still have some days I dont get to reading email for
whatever reason.


Re: KAUTH_SYSTEM_UNENCRYPTED_SWAP

2020-05-16 Thread Paul Goyette

I'm not sure I like the name!

Can you call it KAUTH_SYSTEM_DISABLE_SWAPENCRYPT ?  That more
closely describes the action which is being controlled.


On Sat, 16 May 2020, Alexander Nasonov wrote:


Attached patch adds KAUTH_SYSTEM_UNENCRYPTED_SWAP and
it forbids changing vm.swap_encrypt from 1 to 0 when
securelevel > 0.

If there are no objections, I'm going to commit it tomorrow.

--
Alex


!DSPAM:5ec06ddb24841398742664!



++--+---+
| Paul Goyette   | PGP Key fingerprint: | E-mail addresses: |
| (Retired)  | FA29 0E3B 35AF E8AE 6651 | p...@whooppee.com |
| Software Developer | 0786 F758 55DE 53BA 7731 | pgoye...@netbsd.org   |
++--+---+


KAUTH_SYSTEM_UNENCRYPTED_SWAP

2020-05-16 Thread Alexander Nasonov
Attached patch adds KAUTH_SYSTEM_UNENCRYPTED_SWAP and
it forbids changing vm.swap_encrypt from 1 to 0 when
securelevel > 0.

If there are no objections, I'm going to commit it tomorrow.

-- 
Alex
Index: share/man/man9/kauth.9
===
RCS file: /cvsroot/src/share/man/man9/kauth.9,v
retrieving revision 1.112
diff -p -u -u -r1.112 kauth.9
--- share/man/man9/kauth.9  15 Jul 2018 05:16:41 -  1.112
+++ share/man/man9/kauth.9  16 May 2020 19:22:46 -
@@ -25,7 +25,7 @@
 .\" (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF
 .\" THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 .\"
-.Dd July 14, 2018
+.Dd May 17, 2020
 .Dt KAUTH 9
 .Os
 .Sh NAME
@@ -488,6 +488,8 @@ Check if changing the RTC offset is allo
 .It Dv KAUTH_REQ_SYSTEM_TIME_TIMECOUNTERS
 Check if manipulating timecounters is allowed.
 .El
+.It Dv KAUTH_SYSTEM_UNENCRYPTED_SWAP
+Check if encrypted swap can be degraded to unencrypted.
 .It Dv KAUTH_SYSTEM_VERIEXEC
 Check if operations on the
 .Xr veriexec 8
Index: share/man/man9/secmodel_securelevel.9
===
RCS file: /cvsroot/src/share/man/man9/secmodel_securelevel.9,v
retrieving revision 1.19
diff -p -u -u -r1.19 secmodel_securelevel.9
--- share/man/man9/secmodel_securelevel.9   18 May 2019 10:21:03 -  
1.19
+++ share/man/man9/secmodel_securelevel.9   16 May 2020 19:22:46 -
@@ -26,7 +26,7 @@
 .\" (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF
 .\" THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 .\"
-.Dd May 18, 2019
+.Dd May 17, 2020
 .Dt SECMODEL_SECURELEVEL 9
 .Os
 .Sh NAME
@@ -129,6 +129,11 @@ calls are denied.
 .It
 Access to unmanaged memory is denied.
 .It
+The
+.Va vm.swap_encrypt
+.Xr sysctl 8
+variable may not be changed to 0.
+.It
 Only GPIO pins that have been set at
 .Em securelevel
 0 can be accessed.
Index: sys/secmodel/securelevel/secmodel_securelevel.c
===
RCS file: /cvsroot/src/sys/secmodel/securelevel/secmodel_securelevel.c,v
retrieving revision 1.35
diff -p -u -u -r1.35 secmodel_securelevel.c
--- sys/secmodel/securelevel/secmodel_securelevel.c 11 May 2020 19:36:39 
-  1.35
+++ sys/secmodel/securelevel/secmodel_securelevel.c 16 May 2020 19:22:47 
-
@@ -343,6 +343,11 @@ secmodel_securelevel_system_cb(kauth_cre
result = KAUTH_RESULT_DENY;
break;
 
+   case KAUTH_SYSTEM_UNENCRYPTED_SWAP:
+   if (securelevel > 0)
+   result = KAUTH_RESULT_DENY;
+   break;
+
case KAUTH_SYSTEM_DEBUG:
default:
break;
Index: sys/secmodel/suser/secmodel_suser.c
===
RCS file: /cvsroot/src/sys/secmodel/suser/secmodel_suser.c,v
retrieving revision 1.54
diff -p -u -u -r1.54 secmodel_suser.c
--- sys/secmodel/suser/secmodel_suser.c 16 May 2020 19:12:38 -  1.54
+++ sys/secmodel/suser/secmodel_suser.c 16 May 2020 19:22:47 -
@@ -397,6 +397,11 @@ secmodel_suser_system_cb(kauth_cred_t cr
 
break;
 
+   case KAUTH_SYSTEM_UNENCRYPTED_SWAP:
+   if (isroot)
+   result = KAUTH_RESULT_ALLOW;
+   break;
+
case KAUTH_SYSTEM_VERIEXEC:
switch (req) {
case KAUTH_REQ_SYSTEM_VERIEXEC_ACCESS:
Index: sys/sys/kauth.h
===
RCS file: /cvsroot/src/sys/sys/kauth.h,v
retrieving revision 1.84
diff -p -u -u -r1.84 kauth.h
--- sys/sys/kauth.h 29 Apr 2020 05:54:37 -  1.84
+++ sys/sys/kauth.h 16 May 2020 19:22:47 -
@@ -152,6 +152,7 @@ enum {
KAUTH_SYSTEM_FS_SNAPSHOT,
KAUTH_SYSTEM_INTR,
KAUTH_SYSTEM_KERNADDR,
+   KAUTH_SYSTEM_UNENCRYPTED_SWAP,
 };
 
 /*
Index: sys/uvm/uvm_swap.c
===
RCS file: /cvsroot/src/sys/uvm/uvm_swap.c,v
retrieving revision 1.189
diff -p -u -u -r1.189 uvm_swap.c
--- sys/uvm/uvm_swap.c  10 May 2020 02:38:10 -  1.189
+++ sys/uvm/uvm_swap.c  16 May 2020 19:22:47 -
@@ -2078,12 +2078,34 @@ uvm_swap_decryptpage(struct swapdev *sdp
explicit_memset(, 0, sizeof aes);
 }
 
+static int
+sysctl_swap_encrypt(SYSCTLFN_ARGS)
+{
+   struct sysctlnode node;
+   int newval, error;
+
+   newval = *(int *)rnode->sysctl_data;
+
+   node = *rnode;
+   node.sysctl_data = 
+   error = sysctl_lookup(SYSCTLFN_CALL());
+   if (error || newp == NULL)
+   return error;
+
+   if (!newval && kauth_authorize_system(l->l_cred,
+   KAUTH_SYSTEM_UNENCRYPTED_SWAP, 0, NULL, NULL, NULL))
+   return EPERM;
+
+   *(int *)rnode->sysctl_data = newval;
+   return 0;
+}
+
 

Re: vmspace refcnt refactor patch

2020-05-16 Thread Kamil Rytarowski
On 16.05.2020 18:52, Nick Hudson wrote:
> 
> On 16/05/2020 12:45, Kamil Rytarowski wrote:
>> On 16.05.2020 07:48, Nick Hudson wrote:
>>> On 15/05/2020 17:35, Kamil Rytarowski wrote:
 I propose the following patch:

 http://netbsd.org/~kamil/patch-00253-refactor-vmspace-refcnt.txt

 Does it look good?

 Nick, does it fix the hppa locking problem?

>>> Thanks for working on this.
>>>
>>> Unfortunately, it doesn't fix all the problems...
>>>
>> Please check this:
>>
>> http://netbsd.org/~kamil/patch-00254-remove-lwp_lock_in_sstep.txt
> 
> I can run the ptrace tests now without triggering LOCKDEBUG asserts.
> 
> 
> Thanks,
> 
> Nick
> 

OK. I'm waiting now for the feedback from Andew before committing.



signature.asc
Description: OpenPGP digital signature


Re: vmspace refcnt refactor patch

2020-05-16 Thread Nick Hudson



On 16/05/2020 12:45, Kamil Rytarowski wrote:

On 16.05.2020 07:48, Nick Hudson wrote:

On 15/05/2020 17:35, Kamil Rytarowski wrote:

I propose the following patch:

http://netbsd.org/~kamil/patch-00253-refactor-vmspace-refcnt.txt

Does it look good?

Nick, does it fix the hppa locking problem?


Thanks for working on this.

Unfortunately, it doesn't fix all the problems...


Please check this:

http://netbsd.org/~kamil/patch-00254-remove-lwp_lock_in_sstep.txt


I can run the ptrace tests now without triggering LOCKDEBUG asserts.


Thanks,

Nick



Re: uvm object page remove question

2020-05-16 Thread Manuel Bouyer
On Sat, May 16, 2020 at 05:24:32AM -0700, Chuck Silvers wrote:
> On Wed, May 13, 2020 at 08:20:15PM +0200, Manuel Bouyer wrote:
> > Hello,
> > for Xen I need some non-standard VM operation: the tools want to map
> > some Xen objects for which we don't have a physical address.
> > The map/unmap operations are done with hypercalls which does the
> > page table update. In my implementation the tools ask the kernel to
> > do this via a ioctl on /kern/xen/privcmds
> > 
> > When a tool wants to map one of these Xen object, it first does a
> > mmap() to get some virtual space, and then an ioctl to map it.
> > The ioctl allocates a uvm_object and uvm_map() it for the range.
> > The pgo_fault() handler will map the VA to the Xen object using the
> > dedictated hypercall; this works fine.
> > 
> > But I have a problem for unmap: when uvm wants to remove the mapping
> > (either because the process called munmap() or because it exited),
> > pmap_remove() will try to clear the page table entries for this
> > special mapping, and Xen will kill the VM. This has to be done with
> > the right hypercall.
> > 
> > I see 2 ways to fix this: add a pgo_remove() and have UVM call it,
> > or intercept the pmap_remove() calls at the pmap level, and check
> > the VA against our uvm_objects.
> > 
> > Is there something I missed to get a custom page remove for uvm ojbects ?
> > what would be the best way to handle this ?
> 
> I don't think you're missing anything...
> currently the kernel is always able to clear a PTE by simply writing a zero.
> 
> There are more cases to consider, eg. pmap_protect() on this magic mapping
> would probably kill the VM too, since that will also try to modify the PTE.

Not sure; maybe changing the protection bits would be allowed.

We're already more or less in this world with pmap_protect_ma() which takes
the remote domain as (optional) argument, just for this.

> 
> Are all of these mappings single pages?

Some are single pages, but some also contains several pages (I guess for e.g.
framebuffer, or loading the initial code).

> If not, does xen allow removing
> one page of a magic mapping and leaving the rest in place?

Yes

> 
> Why does xen feel it necessary to kill the VM in these cases
> where the guest is reducing its access to these magic pages?
> This would be a lot easier to deal with if xen just let the guest
> operate on these PTEs the same way it operates on other PTEs.

Sure. But I think Xen wants to track the usage, and eventually we
have to take some extra action on unmap too (like a notification sent to
the remote domain).

Also, exact way to do the mapping depends on the type of the remote domain
(PV vs HVM)

> 
> What are these magic mappings for, exactly?

Various memory mapped structures (like the xenstore), emulated devices
(e.g. framebuffer).


> 
> Would it be practical to just map the magic mappings in the kernel
> and then have the tools use ioctls to access the magic mappings?
> That would avoid all of these problems.

This would impose very depth modifications in the tools.


For now I implemented this re-using the hooks in x86/pmap.c for NVMM
and ept tables (pm_remove and pm_data) and it's enough for my needs.

I have another question: when we implment a umv_object, it is possible to
map the whole range at uvm_map() time ?
If you look at the IOCTL_PRIVCMD_MMAPBATCH code in sys/arch/xen/xen/privcmd.c
we validate the machine addresses using a temporaty mapping in kernel space.
The mapping in userland itself is done when a fault occurs.
It would be easier to map the whole range at ioctl() time, so that a
fault never occurs.
I guess I would need to allocate virtual pages registers them in the map ?

-- 
Manuel Bouyer 
 NetBSD: 26 ans d'experience feront toujours la difference
--


Re: uvm object page remove question

2020-05-16 Thread Chuck Silvers
On Wed, May 13, 2020 at 08:20:15PM +0200, Manuel Bouyer wrote:
> Hello,
> for Xen I need some non-standard VM operation: the tools want to map
> some Xen objects for which we don't have a physical address.
> The map/unmap operations are done with hypercalls which does the
> page table update. In my implementation the tools ask the kernel to
> do this via a ioctl on /kern/xen/privcmds
> 
> When a tool wants to map one of these Xen object, it first does a
> mmap() to get some virtual space, and then an ioctl to map it.
> The ioctl allocates a uvm_object and uvm_map() it for the range.
> The pgo_fault() handler will map the VA to the Xen object using the
> dedictated hypercall; this works fine.
> 
> But I have a problem for unmap: when uvm wants to remove the mapping
> (either because the process called munmap() or because it exited),
> pmap_remove() will try to clear the page table entries for this
> special mapping, and Xen will kill the VM. This has to be done with
> the right hypercall.
> 
> I see 2 ways to fix this: add a pgo_remove() and have UVM call it,
> or intercept the pmap_remove() calls at the pmap level, and check
> the VA against our uvm_objects.
> 
> Is there something I missed to get a custom page remove for uvm ojbects ?
> what would be the best way to handle this ?

I don't think you're missing anything...
currently the kernel is always able to clear a PTE by simply writing a zero.

There are more cases to consider, eg. pmap_protect() on this magic mapping
would probably kill the VM too, since that will also try to modify the PTE.

Are all of these mappings single pages?  If not, does xen allow removing
one page of a magic mapping and leaving the rest in place?

Why does xen feel it necessary to kill the VM in these cases
where the guest is reducing its access to these magic pages?
This would be a lot easier to deal with if xen just let the guest
operate on these PTEs the same way it operates on other PTEs.

What are these magic mappings for, exactly?

Would it be practical to just map the magic mappings in the kernel
and then have the tools use ioctls to access the magic mappings?
That would avoid all of these problems.

-Chuck


Re: vmspace refcnt refactor patch

2020-05-16 Thread Kamil Rytarowski
On 16.05.2020 07:48, Nick Hudson wrote:
> On 15/05/2020 17:35, Kamil Rytarowski wrote:
>> I propose the following patch:
>>
>> http://netbsd.org/~kamil/patch-00253-refactor-vmspace-refcnt.txt
>>
>> Does it look good?
>>
>> Nick, does it fix the hppa locking problem?
>>
> 
> Thanks for working on this.
> 
> Unfortunately, it doesn't fix all the problems...
> 

Please check this:

http://netbsd.org/~kamil/patch-00254-remove-lwp_lock_in_sstep.txt

In general I don't like the locking model in ptrace(2), but this change
is not making anything worse.

> 
>     setstep1: [ 209.5497837] Mutex error: assert_sleepable,78: spin
> lock held
> 
> [ 209.5497837] lock address : 0x3ffccf00 type :  spin
> [ 209.5497837] initialized  : 0x007d6f78
> [ 209.5497837] shared holds :  0 exclusive: 1
> [ 209.5497837] shares wanted:  0 exclusive: 0
> [ 209.5497837] relevant cpu :  0 last held: 0
> [ 209.5497837] relevant lwp : 0x3fc36d00 last held:
> 0x3fc36d00
> [ 209.5497837] last locked* : 0x0084a760 unlocked :
> 0x007d5134
> [ 209.5497837] owner field  : 0xff10 wait/spin:   0/1
> 
> [ 209.5497837] panic: LOCKDEBUG: Mutex error: assert_sleepable,78: spin
> lock held
> [ 209.5497837] cpu0: Begin traceback...
> [ 209.5497837] vpanic() at netbsd:vpanic+0x1b8
> [ 209.5497837] panic() at netbsd:panic+0x38
> [ 209.5497837] lockdebug_abort1() at netbsd:lockdebug_abort1+0x170
> [ 209.5497837] lockdebug_barrier() at netbsd:lockdebug_barrier+0x114
> [ 209.5497837] assert_sleepable() at netbsd:assert_sleepable+0xa0
> [ 209.5497837] pool_cache_get_paddr() at netbsd:pool_cache_get_paddr+0x254
> [ 209.5497837] uvm_mapent_alloc() at netbsd:uvm_mapent_alloc+0x150
> [ 209.5497837] uvm_map_enter() at netbsd:uvm_map_enter+0x848
> [ 209.5497837] uvm_map() at netbsd:uvm_map+0xd4
> [ 209.5497837] uvm_map_reserve() at netbsd:uvm_map_reserve+0x240
> [ 209.5497837] uvm_map_extract() at netbsd:uvm_map_extract+0x6fc
> [ 209.5497837] uvm_io() at netbsd:uvm_io+0xf8
> [ 209.5497837] process_domem() at netbsd:process_domem+0x94
> [ 209.5497837] ss_get_value() at netbsd:ss_get_value+0x74
> [ 209.5497837] process_sstep() at netbsd:process_sstep+0x74
> [ 209.5497837] do_ptrace() at netbsd:do_ptrace+0xe00
> [ 209.5497837] sys_ptrace() at netbsd:sys_ptrace+0x4c
> [ 209.5497837] syscall() at netbsd:syscall+0x480
> [ 209.5497837] -- syscall #26(7, 141a, 1, 0, ...) (0xd3131000)
> [ 209.5497837] cpu0: End traceback...
> 
> nick@zoom:/netbsd/hppa/obj.hppa/sys/arch/hppa/compile/GENERIC$
> hppa--netbsd-addr2line -e netbsd.gdb  -f 0x007d6f78
> sched_cpuattach
> /netbsd/hppa/src/sys/kern/kern_runq.c:147
> nick@zoom:/netbsd/hppa/obj.hppa/sys/arch/hppa/compile/GENERIC$
> hppa--netbsd-addr2line -e netbsd.gdb  -f 0x0084a760
> lwp_lock
> /netbsd/hppa/src/sys/sys/lwp.h:412
> nick@zoom:/netbsd/hppa/obj.hppa/sys/arch/hppa/compile/GENERIC$
> hppa--netbsd-addr2line -e netbsd.gdb  -f 0x007d5134
> calcru
> /netbsd/hppa/src/sys/kern/kern_resource.c:506 (discriminator 2)
> nick@zoom:/netbsd/hppa/obj.hppa/sys/arch/hppa/compile/GENERIC$
> 
> Nick





signature.asc
Description: OpenPGP digital signature