I didn't do a careful analysis, but glancing at the ARMv7-A code, it looks to me that none of the things needed to create stack frames are available for the kernel stack.  It looks like the system call entry logic just sets the SP to the top of the kernel stack and does very little more.  The address stack base and size of the stack do not get set up in the TCB on a system call so creating a frame with arm_stackframe() would not work.

Am I missing something?

On 4/26/2023 2:14 PM, Ville Juven wrote:
Hi,

But, why need involve kheap here? If the security is your concern, you
should enable per-thread kernel stack in the first place. It's safe to
allocate waitlist/holder from this stack directly, since userspace code
can't touch this area too.
Not from a security perspective, and of course I have the per-thread kstack
enabled (kernel mode won't work without it) but how do you keep the frame
from being corrupted / freed until it is not needed any more? Do you
suggest the frame is allocated for each waiting thread separately ? How
does that work ?

I was thinking the first one who waits in sem_wait() allocates the kernel
structure, puts a reference to it into e.g. the user semaphore, so other
waiters and the kernel itself can find it later (reference being ID, not
pointer!).

But, which scenario needs to access kernel struct through user semaphore?
It is not necessary to make the access via user semaphore, but somehow the
kernel side structure needs to be tied to the semaphore, so the kernel can
find it. Linux does this by a hash function, if I remember correctly. I'm
not sure duplicating this is a good approach, my concern is the real-time
aspect.

sem_waitirq() is the problem I have, and fixing it is nasty. The semaphore
is accessed via tcb->waitobj and this access can happen from interrupt or
signal dispatch. This means the context of sem_wait() / sem_post() is _not_
the only place the access happens. sem_waitirq() currently also modifies
the semaphore counter value, so it needs access to the userspace counter.
This I will do by mapping the page where the counter exists to kernel
(implement something like kmap() / vmap()).

If you have an alternative suggestion on how to fix sem_waitirq() I will
gladly implement it.

Br,
Ville Juven

On Wed, Apr 26, 2023 at 10:34 PM Xiang Xiao <xiaoxiang781...@gmail.com>
wrote:

On Thu, Apr 27, 2023 at 2:18 AM Ville Juven <ville.ju...@gmail.com> wrote:

Hi,

Yes exactly, this is the basic idea. Like I said, the waitlist/holder
structure is needed only when sem_wait needs to block / wait for the
semaphore to become available.

How to protect the integrity of the stack allocated structure is still a
bit open but one option is to use kheap as well. Semantics to be figured
out, the solution should be feasible.

But, why need involve kheap here? If the security is your concern, you
should enable per-thread kernel stack in the first place. It's safe to
allocate waitlist/holder from this stack directly, since userspace code
can't touch this area too.


My idea was to put the handle to this data into the user semaphore,
however
a pointer must not be used, a handle / integer id is needed, which then
holds the pointer (much like files etc). As the user can spoof / destroy
the pointer it is unsafe to do that. Spoofing the id can cause the user
process to crash, but the kernel integrity remains.


But, which scenario needs to access kernel struct through user semaphore?


Br,
Ville Juven

On Wed, Apr 26, 2023 at 9:09 PM Xiang Xiao <xiaoxiang781...@gmail.com>
wrote:

But why not allocate list entry and holder temporally inside the kernel
thread stack? Both data is just used when the calling thread can't
hold,
but wait the semphare, this trick is widely used inside Linux kernel.

On Thu, Apr 27, 2023 at 1:32 AM Ville Juven <ville.ju...@gmail.com>
wrote:
Hi,

Thanks for the explanation, it makes sense. I had an idea to move
struct
sem_s entirely to kernel and make the user use it via a handle (in
KERNEL
build) but due to the ubiquitous init macros, that is not a feasible
solution.

I will explore allocating the kernel structures on-demand next.

What does this mean ? The kernel side structures (dq_waitlist and
priority
inheritance holders) are only needed when the semaphore is locked and
someone is actually waiting for it to be released. This means that
when
sem_wait() results in the thread getting blocked, the kernel part can
be
allocated here and a reference placed into the user sem_t structure.
When
the semaphore unlocks, the kernel parts can be freed.

This should be completely regression free for FLAT/PROTECTED, which
is
very
important for me because I don't want to be responsible for breaking
such a
fundamental OS API for current users ;)

Br,
Ville

On Wed, Apr 26, 2023 at 5:26 PM Gregory Nutt <spudan...@gmail.com>
wrote:
I have a question about using mutex_t / struct mutex_s inside
libs/libc. The mutex_t definition is kernel internal but some
modules
inside the libs folder use it directly. My question is, is this
intentional ? I don't think such modules should be using mutex_t.

...

My question ? Should these be sem_t or perhaps pthread_mutex_t ?
Both
of which are valid user APIs.
It was sem_t before it was changed to the non-standard mutex_t. See
commit d1d46335df6:

commit d1d46335df6cc1bc63f1cd7e7bffe3735b8c275d
Author: anjiahao <anjia...@xiaomi.com>
Date:   Tue Sep 6 14:18:45 2022 +0800

      Replace nxsem API when used as a lock with nxmutex API

      Signed-off-by: anjiahao <anjia...@xiaomi.com>
      Signed-off-by: Xiang Xiao <xiaoxi...@xiaomi.com>

Parts of the above which introduce inappropriate use of mutex_t
could
be
reverted back to sem_t.  Any use of any non-standard OS function or
structure should be prohibited in application code.

However, nuttx/libs is a special creature.  It is not confined by
the
rules of POSIX interface because user-space libraries are a
non-privileged extension of the operating system.  They have
intimate
knowledge of OS internals and can do non-standard things to
accomplish
what they need to do.  So although I think it is wrong
architecturally
for applications to use OS internals, I don't think it is wrong for
nuttx/libs to do so.  It is a friend of the OS.

But it is should not break the KERNEL build either.

Perhaps we will need a user-space function to allocate (and
initialize)
kernel memory.  Use of such functions should not performed outside
of
nuttx/libs, although we have no way to prohibit that use:  Such an
API
would be a security hole. user-space code cannot access kernel
memory,
but it can use the kernel memory address like a "handle" to
interact
with the OS.



Reply via email to