umm_map returns unaligned address?

Discussion:

Alessandro Pistocchi

2021-04-24 00:11:14 UTC

---------- Forwarded message ---------
From: Alessandro Pistocchi <***@gmail.com>
Date: Fri, Apr 23, 2021 at 1:55 PM
Subject: umm_map returns unaligned address?
To: <***@openbsd.org>

Hi all,

I am fairly new to openbsd so if this is something obvious that I missed
please be understanding.

I am adding a syscall to openbsd 6.8. I am working on a raspberry pi.

During the syscall I allocate some memory that I want to share between the
kernel
and the calling process.

When it's time to wrap up and unmap the memory, I unmap it both from the
kernel
map and from the process map.

The unmapping from the process map goes fine, the unmapping from the kernel
map
fails by saying that the virtual address in kernel map is not aligned to
the page size
( it's actually 4 bytes off ).

What have I missed? I assumed that umm_map would return a page aligned
virtual
address for the kernel mapping as well.

Here is my code for creating the shared memory chunk:

--------------------------------------------------------------------------------
// memory_size is a multiple of page size
uvm_object = uao_create(memory_size, 0);
if(!uvm_object) return;

// TODO(ale): make sure that this memory cannot be swapped out

uao_reference(uvm_object)
if(uvm_map(kernel_map, (vaddr_t *)&memory, round_page(memory_size),
uvm_object,
0, 0, UVM_MAPFLAG(PROT_READ | PROT_WRITE, PROT_READ | PROT_WRITE,
MAP_INHERIT_SHARED, MADV_NORMAL, 0))) {
uao_detach(uvm_object);
uvm_object = 0;
return;
}

uao_reference(uvm_object);
if(uvm_map(&p->p_vmspace->vm_map, &memory_in_proc_space,
round_page(memory_size), uvm_object,
0, 0, UVM_MAPFLAG(PROT_READ | PROT_WRITE, PROT_READ | PROT_WRITE,
MAP_INHERIT_NONE, MADV_NORMAL, 0))) {
memory = 0;
uao_detach(uvm_object);
uao_detach(uvm_object);
uvm_object = 0;
return;
}
--------------------------------------------------------------------------------

Thanks,
A

Philip Guenther

2021-04-24 00:24:24 UTC

Permalink

Post by Alessandro Pistocchi
---------- Forwarded message ---------
Date: Fri, Apr 23, 2021 at 1:55 PM
Subject: umm_map returns unaligned address?
Hi all,
I am fairly new to openbsd so if this is something obvious that I missed
please be understanding.
I am adding a syscall to openbsd 6.8. I am working on a raspberry pi.
During the syscall I allocate some memory that I want to share between the
kernel
and the calling process.
When it's time to wrap up and unmap the memory, I unmap it both from the
kernel
map and from the process map.
The unmapping from the process map goes fine, the unmapping from the kernel
map
fails by saying that the virtual address in kernel map is not aligned to
the page size
( it's actually 4 bytes off ).
What have I missed? I assumed that umm_map would return a page aligned
virtual
address for the kernel mapping as well.

Stop sending summaries and just send diffs that compile: you don't know
everything that is relevant and keep leaving out stuff that is. I'm the
third person to say this.

Post by Alessandro Pistocchi
--------------------------------------------------------------------------------
// memory_size is a multiple of page size
uvm_object = uao_create(memory_size, 0);
if(!uvm_object) return;
// TODO(ale): make sure that this memory cannot be swapped out
uao_reference(uvm_object)
if(uvm_map(kernel_map, (vaddr_t *)&memory, round_page(memory_size),
uvm_object,
0, 0, UVM_MAPFLAG(PROT_READ | PROT_WRITE, PROT_READ | PROT_WRITE,
MAP_INHERIT_SHARED, MADV_NORMAL, 0))) {

The cast of &memory is wrong: it's either unnecessary (if memory is of the
correct type) or totally broken (if it isn't). Why did you think it was
unnecessary to show how you declared your variables?

You also fail to show your initialization of 'memory'. If you didn't then
that's ABSOLUTELY wrong and not in line with the existing uses of uvm_map()
in the kernel. Please consult the uvm_map(9) manpage for what the incoming
value means.

...

Post by Alessandro Pistocchi
uao_reference(uvm_object);
if(uvm_map(&p->p_vmspace->vm_map, &memory_in_proc_space,
round_page(memory_size), uvm_object,
0, 0, UVM_MAPFLAG(PROT_READ | PROT_WRITE, PROT_READ | PROT_WRITE,
MAP_INHERIT_NONE, MADV_NORMAL, 0))) {
memory = 0;

This error handling is incomplete, lacking an unmap.

Philip Guenther

Alessandro Pistocchi

2021-04-24 00:54:44 UTC

Permalink

Sorry, I didn't see previous replies because the emails went to the spam
folder
for some reason.

I didn't mean to waste anybody's time. Apologies. I replied to your
subsequent points below
and I'll send the diff in a new email.

On Fri, Apr 23, 2021 at 3:13 PM Alessandro Pistocchi <

Stop sending summaries and just send diffs that compile: you don't know
everything that is relevant and keep leaving out stuff that is. I'm the
third person to say this.

The cast of &memory is wrong: it's either unnecessary (if memory is of the
correct type) or totally broken (if it isn't). Why did you think it was
unnecessary to show how you declared your variables?

You're right.
memory is a void * which is zero at call time, vadd_t an unsigned long. I
believed it was ok since I am expecting to receive the address of the start
of the mapping.

You also fail to show your initialization of 'memory'. If you didn't then
that's ABSOLUTELY wrong and not in line with the existing uses of uvm_map()
in the kernel. Please consult the uvm_map(9) manpage for what the incoming
value means.
...

This error handling is incomplete, lacking an unmap.

Yep, I had noticed that and corrected it in my code. However that should
not be related to the problem I get.

Philip Guenther

Theo de Raadt

2021-04-24 00:32:26 UTC

Permalink

Post by Alessandro Pistocchi
During the syscall I allocate some memory that I want to share between the
kernel and the calling process.

When you get the mapping working, it will not work as well as you like.

Compiler re-ordering of writes & reads, caches, write buffers, and other
details such as non-TSO will creat problems, meaning you'll need very careful
data-structure, and great care with co-dependent fields, otherwise one side
can fool the other.

That is why the copyin/copyout approach is used as a high-level serializing-forcing
primitive in most operating systems.

But since you don't show your whole diff.. I suspect this is a waste of time.

Alessandro Pistocchi

2021-04-24 01:50:12 UTC

Permalink

Hi,
apologies again.

I am not familiar with cvs so I have used the diff command between an
original sys folder and mine.

I am attaching the diff file and two files to be put in sys/kern. Sorry but
the diff
command did not include the content of the two files in the diff itself.

Please notice that this is not code that is intended to be put into the
openbsd kernel,
as it would probably not be interesting to the general users.

What I am doing here is to have a syscall that stops the scheduler on 3 of
the 4 cores,
runs my own code on those while the calling process runs on the other core
and restarts
scheduling on the 3 cores when the process terminates.
The shared memory is used for the 3 cores to communicate with the process.
I know that it might sound crazy to you but it does make a lot of sense for
what
I am trying to achieve.

Regarding the mapping, I hope that it works well enough even with
reordering issues and other stuff.
It does for the example I am sending and I don't need much more than that.

What I was flagging is just that sometimes uvm_map returns an address that
is not
aligned to PAGE_SIZE ( I printed it out and it has 0x004 in the lower 12
bits). On the
other hand uvm_unmap has an assertion that panics if the address passed to
it is not
page aligned. I believe that there could be a bug somewhere.

In previous runs I was getting constantly an address which is not aligned
but now
I get an address that is aligned.

Best :-)
Alessandro

Post by Theo de Raadt

Post by Alessandro Pistocchi
During the syscall I allocate some memory that I want to share between

the

Post by Alessandro Pistocchi
kernel and the calling process.

When you get the mapping working, it will not work as well as you like.
Compiler re-ordering of writes & reads, caches, write buffers, and other
details such as non-TSO will creat problems, meaning you'll need very careful
data-structure, and great care with co-dependent fields, otherwise one side
can fool the other.
That is why the copyin/copyout approach is used as a high-level serializing-forcing
primitive in most operating systems.
But since you don't show your whole diff.. I suspect this is a waste of time.

Philip Guenther

2021-04-24 03:38:54 UTC

Permalink

On Fri, Apr 23, 2021 at 4:50 PM Alessandro Pistocchi <***@gmail.com>
wrote:
...

Post by Alessandro Pistocchi
What I was flagging is just that sometimes uvm_map returns an address that
is not
aligned to PAGE_SIZE ( I printed it out and it has 0x004 in the lower 12
bits).On the

other hand uvm_unmap has an assertion that panics if the address passed to

Post by Alessandro Pistocchi
it is not
page aligned. I believe that there could be a bug somewhere.

You apparently didn't print out the value directly after return from
uvm_map() but rather later after a bunch of your other code had run. Yes,
there's a bug, in your game_mode_start_audio_thread(), where you advance
the pointer from uvm_map() by four.

Philip Guenther