Hi,
Thanks for the responses. The scalar read / write functions will work when
accessing singular fields from the semaphore, and such an access will (or
at least should be) always be correctly aligned so there is no risk in the
scalar value being split by a page boundary -> no extra work is required,
simply obtaining the kernel virtual address for the page + offset will
suffice.
However, as I said, there are some things in the sem_t structure whose
usage is ubiquitous and I don't know if the scalar access macros will be
enough, i.e. I don't fully understand how they work / are supposed to work.
struct sem_s
{
volatile int16_t semcount;
uint8_t flags;
dq_queue_t waitlist; // This is accessed by the system scheduler when
traversing TSTATE_WAIT_SEM-list
// These are also accessed from all over
#ifdef CONFIG_PRIORITY_INHERITANCE
# if CONFIG_SEM_PREALLOCHOLDERS > 0
FAR struct semholder_s *hhead;
# else
struct semholder_s holder[2];
# endif
#endif
};
The waitlist doubly linked queue is used by the scheduler to implement the
TSTATE_WAIT_SEM list. This means the semaphore object is sporadically
accessed when the scheduler accesses tasks in that list. This means that
ANY user semaphore from any process must be accessible by the kernel to
traverse the list. Ok, we can access dqueue->head via pointer aligned
access, so that can be patched.
The next is the semholder list. I'm not even sure how much patching that
will need to work with MMU. The static holder[] allocations come from user
memory, while the "hhead" allocations come from a kernel pool. Ok, maybe
the test for CONFIG_SEM_PREALLOCHOLDERS can also test for MMU, if MMU is in
use, do not use the static holder[] list.
Finally, we have a reference to a (I don't know which, yet) semaphore in
struct semholder_s too. Maybe someone can help me understand what this
reference is for ?
A more detailed description of my findings is in the github issue I created
for this: https://github.com/apache/nuttx/issues/8917
Also, what Jukka said above about the security aspect is a perfectly valid
point. Now the dq waitlist and the semaphore holder list are in user
memory, which means the user process can mess those lists up, crashing the
kernel.
Br,
Ville Juven
PS. Whatever solution is implemented shall in no way increase code size or
modify functionality in the flat addressing mode(s). This is a requirement
for us as well, we have flat targets that have very limited memory as well.
On Tue, Apr 18, 2023 at 4:23 PM Gregory Nutt <spudan...@gmail.com> wrote:
> On 4/18/2023 6:58 AM, Nathan Hartman wrote:
> > On Tue, Apr 18, 2023 at 8:52 AM spudaneco <spudan...@gmail.com> wrote:
> >
> >> Perhaps you could use accessor functions to read/write 8, 16 32, 64 bit
> >> values. Each scalar value will be properly aligned and, hence, will lie
> >> within a page.
> >
> >
> > And those accessor functions could be macros in FLAT builds where
> functions
> > are not required, so no memory or code size increase.
> >
> > Cheers
> > Nathan
> >
> The inefficiency is that each access would require a virtual to physical
> address look-up. That could be alleviated by providing the base
> physical address of the the page and the offset of the sem_t iin the
> page as a parameter, maybe like:
>
> struct paginfo_t pageinfo;
>
> get_pageinfo(sem, &pageinfo);
>
> value = get_pagevalue16(&pageinfo, &sem->field);
>
> If the field like in the following page, then get_pagevalue16 would have
> to work harder. But the usual case, where the sem_t lies wholly in the
> page would be relatively fast.
>
> This, however, would not macro-ize as well.
>
> Not that no special alignment of the sem_t is required if you access by
> scalar field.
>
>
>
