[PATCH v3] cobalt: Sort out virtual vs. real pids

2021-08-02 Thread Jan Kiszka via Xenomai 
From: Jan Kiszka 

Make sure that we consider all pids taken from or sent to userspace via
syscalls as virtual. That means converting them to global pids before
processing them or converting them back to virtual pids in the caller's
namespace before reporting them. We also need to switch thread hashing
from vpids to real pids.

Not converted is the procfs interface as this one continues to provide
only a global view on cobalt objects and associated pids.

A few syscall tracepoints that report the vpid as passed down from
userspace get adjusted in their output so that it is clear that no
global pid is shown.

Signed-off-by: Jan Kiszka 
---

Changes in v3:
 - handle process-local thread lookups correctly (hopefully)

This is in next for a while and ran several test cycles. Even if we 
should have remaining issues under pid namespaces, we should not have 
regressions in the normal cases.

 include/cobalt/kernel/thread.h |  2 ++
 kernel/cobalt/posix/event.c|  2 +-
 kernel/cobalt/posix/mqueue.c   |  2 +-
 kernel/cobalt/posix/process.c  | 14 ++
 kernel/cobalt/posix/process.h  |  2 ++
 kernel/cobalt/posix/sched.c| 10 ++
 kernel/cobalt/posix/sem.c  |  2 +-
 kernel/cobalt/posix/signal.c   | 10 ++
 kernel/cobalt/posix/thread.c   | 18 ++
 kernel/cobalt/posix/thread.h   |  2 +-
 kernel/cobalt/thread.c | 10 ++
 kernel/cobalt/trace/cobalt-posix.h |  4 ++--
 12 files changed, 56 insertions(+), 22 deletions(-)

diff --git a/include/cobalt/kernel/thread.h b/include/cobalt/kernel/thread.h
index b79cb84296..4c8a0b9f2a 100644
--- a/include/cobalt/kernel/thread.h
+++ b/include/cobalt/kernel/thread.h
@@ -450,6 +450,8 @@ char *xnthread_format_status(unsigned long status,
 
 pid_t xnthread_host_pid(struct xnthread *thread);
 
+pid_t xnthread_host_vpid(struct xnthread *thread);
+
 int xnthread_set_clock(struct xnthread *thread,
   struct xnclock *newclock);
 
diff --git a/kernel/cobalt/posix/event.c b/kernel/cobalt/posix/event.c
index 3712154f53..5871106301 100644
--- a/kernel/cobalt/posix/event.c
+++ b/kernel/cobalt/posix/event.c
@@ -352,7 +352,7 @@ COBALT_SYSCALL(event_inquire, current,
xnsynch_for_each_sleeper(thread, &event->synch) {
if (nrwait >= nrpids)
break;
-   t[nrwait++] = xnthread_host_pid(thread);
+   t[nrwait++] = xnthread_host_vpid(thread);
}
}
 
diff --git a/kernel/cobalt/posix/mqueue.c b/kernel/cobalt/posix/mqueue.c
index dd8acd55b0..fcde87a077 100644
--- a/kernel/cobalt/posix/mqueue.c
+++ b/kernel/cobalt/posix/mqueue.c
@@ -720,7 +720,7 @@ mq_notify(struct cobalt_mqd *mqd, unsigned index, const 
struct sigevent *evp)
 * receiver's namespaces. We pass the receiver's creds
 * into the init namespace instead.
 */
-   mq->si.si_pid = task_pid_nr(current);
+   mq->si.si_pid = task_pid_vnr(current);
mq->si.si_uid = get_current_uuid();
}
 
diff --git a/kernel/cobalt/posix/process.c b/kernel/cobalt/posix/process.c
index 1f059ad830..98b7df33fb 100644
--- a/kernel/cobalt/posix/process.c
+++ b/kernel/cobalt/posix/process.c
@@ -975,6 +975,20 @@ int cobalt_handle_cleanup_event(struct mm_struct *mm)
return KEVENT_PROPAGATE;
 }
 
+pid_t cobalt_vpid2pid_nr(pid_t vpid)
+{
+   struct pid *pid;
+   pid_t nr = -1;
+
+   rcu_read_lock();
+   pid = find_vpid(vpid);
+   if (pid)
+   nr = pid_nr(pid);
+   rcu_read_unlock();
+
+   return nr;
+}
+
 static int attach_process(struct cobalt_process *process)
 {
struct cobalt_ppd *p = &process->sys_ppd;
diff --git a/kernel/cobalt/posix/process.h b/kernel/cobalt/posix/process.h
index 279707a680..46dff43b7a 100644
--- a/kernel/cobalt/posix/process.h
+++ b/kernel/cobalt/posix/process.h
@@ -175,4 +175,6 @@ int cobalt_handle_cleanup_event(struct mm_struct *mm);
 
 int cobalt_handle_user_return(struct task_struct *task);
 
+pid_t cobalt_vpid2pid_nr(pid_t vpid);
+
 #endif /* !_COBALT_POSIX_PROCESS_H */
diff --git a/kernel/cobalt/posix/sched.c b/kernel/cobalt/posix/sched.c
index 4fd9c2b46e..26259fbd3f 100644
--- a/kernel/cobalt/posix/sched.c
+++ b/kernel/cobalt/posix/sched.c
@@ -736,7 +736,7 @@ COBALT_SYSCALL(sched_weightprio, current,
return __cobalt_sched_weightprio(policy, ¶m_ex);
 }
 
-int cobalt_sched_setscheduler_ex(pid_t pid,
+int cobalt_sched_setscheduler_ex(pid_t vpid,
 int policy,
 const struct sched_param_ex *param_ex,
 __u32 __user *u_winoff,
@@ -747,9 +747,11 @@ int cobalt_sched_setscheduler_ex(pid_t pid,
int ret, promoted = 0;
spl_t s;
 
-   trace_cobalt_sched_setscheduler(pid, policy, param_ex);
+   trace_cobalt_sched_setscheduler(vpid, policy,

RE: [PATCH v3] cobalt: Sort out virtual vs. real pids

2021-08-04 Thread MONTET Julien via Xenomai 
Hi Jan,

I did some new tests this morning with your new patch.
It doesn't work yet, but I have some debug info that may help you.

At first, I looked at the output of "cat /proc/xenomai/sched/stat" directly 
inside the docker and on the target where the app is running.
PID, MSW, CSW and XSC are the same (maybe it is just normal). I can also find 
the same with the command "top".

Then I tried to compare the output of the Docker executable with the classic 
target executable (which works).
You will find in the link below the output of  "cat /proc/xenomai/sched/stat" 
where the docker AND the classic executable are running.
The result is the same if the executables work separately
I renamed the thread and added comments with in ().
https://paste.ubuntu.com/p/PSsqTct7xV/
What does MSW, CSW and XSC mean here ? Values remain the same with Docker while 
they are different with a normal executable.

Regards,

De : Jan Kiszka 
Envoyé : lundi 2 août 2021 18:56
À : Xenomai 
Cc : MONTET Julien ; Marco Barletta 

Objet : [PATCH v3] cobalt: Sort out virtual vs. real pids

From: Jan Kiszka 

Make sure that we consider all pids taken from or sent to userspace via
syscalls as virtual. That means converting them to global pids before
processing them or converting them back to virtual pids in the caller's
namespace before reporting them. We also need to switch thread hashing
from vpids to real pids.

Not converted is the procfs interface as this one continues to provide
only a global view on cobalt objects and associated pids.

A few syscall tracepoints that report the vpid as passed down from
userspace get adjusted in their output so that it is clear that no
global pid is shown.

Signed-off-by: Jan Kiszka 
---

Changes in v3:
 - handle process-local thread lookups correctly (hopefully)

This is in next for a while and ran several test cycles. Even if we
should have remaining issues under pid namespaces, we should not have
regressions in the normal cases.

 include/cobalt/kernel/thread.h |  2 ++
 kernel/cobalt/posix/event.c|  2 +-
 kernel/cobalt/posix/mqueue.c   |  2 +-
 kernel/cobalt/posix/process.c  | 14 ++
 kernel/cobalt/posix/process.h  |  2 ++
 kernel/cobalt/posix/sched.c| 10 ++
 kernel/cobalt/posix/sem.c  |  2 +-
 kernel/cobalt/posix/signal.c   | 10 ++
 kernel/cobalt/posix/thread.c   | 18 ++
 kernel/cobalt/posix/thread.h   |  2 +-
 kernel/cobalt/thread.c | 10 ++
 kernel/cobalt/trace/cobalt-posix.h |  4 ++--
 12 files changed, 56 insertions(+), 22 deletions(-)

diff --git a/include/cobalt/kernel/thread.h b/include/cobalt/kernel/thread.h
index b79cb84296..4c8a0b9f2a 100644
--- a/include/cobalt/kernel/thread.h
+++ b/include/cobalt/kernel/thread.h
@@ -450,6 +450,8 @@ char *xnthread_format_status(unsigned long status,

 pid_t xnthread_host_pid(struct xnthread *thread);

+pid_t xnthread_host_vpid(struct xnthread *thread);
+
 int xnthread_set_clock(struct xnthread *thread,
struct xnclock *newclock);

diff --git a/kernel/cobalt/posix/event.c b/kernel/cobalt/posix/event.c
index 3712154f53..5871106301 100644
--- a/kernel/cobalt/posix/event.c
+++ b/kernel/cobalt/posix/event.c
@@ -352,7 +352,7 @@ COBALT_SYSCALL(event_inquire, current,
 xnsynch_for_each_sleeper(thread, &event->synch) {
 if (nrwait >= nrpids)
 break;
-   t[nrwait++] = xnthread_host_pid(thread);
+   t[nrwait++] = xnthread_host_vpid(thread);
 }
 }

diff --git a/kernel/cobalt/posix/mqueue.c b/kernel/cobalt/posix/mqueue.c
index dd8acd55b0..fcde87a077 100644
--- a/kernel/cobalt/posix/mqueue.c
+++ b/kernel/cobalt/posix/mqueue.c
@@ -720,7 +720,7 @@ mq_notify(struct cobalt_mqd *mqd, unsigned index, const 
struct sigevent *evp)
  * receiver's namespaces. We pass the receiver's creds
  * into the init namespace instead.
  */
-   mq->si.si_pid = task_pid_nr(current);
+   mq->si.si_pid = task_pid_vnr(current);
 mq->si.si_uid = get_current_uuid();
 }

diff --git a/kernel/cobalt/posix/process.c b/kernel/cobalt/posix/process.c
index 1f059ad830..98b7df33fb 100644
--- a/kernel/cobalt/posix/process.c
+++ b/kernel/cobalt/posix/process.c
@@ -975,6 +975,20 @@ int cobalt_handle_cleanup_event(struct mm_struct *mm)
 return KEVENT_PROPAGATE;
 }

+pid_t cobalt_vpid2pid_nr(pid_t vpid)
+{
+   struct pid *pid;
+   pid_t nr = -1;
+
+   rcu_read_lock();
+   pid = find_vpid(vpid);
+   if (pid)
+   nr = pid_nr(pid);
+   rcu_read_unlock();
+
+   return nr;
+}
+
 static int attach_process(struct cobalt_process *process)
 {
 struct cobalt_ppd *p = &process->sys_ppd;
diff

Re: [PATCH v3] cobalt: Sort out virtual vs. real pids

2021-08-08 Thread Jan Kiszka via Xenomai 
On 02.08.21 18:56, Jan Kiszka via Xenomai wrote:
> From: Jan Kiszka 
> 
> Make sure that we consider all pids taken from or sent to userspace via
> syscalls as virtual. That means converting them to global pids before
> processing them or converting them back to virtual pids in the caller's
> namespace before reporting them. We also need to switch thread hashing
> from vpids to real pids.
> 
> Not converted is the procfs interface as this one continues to provide
> only a global view on cobalt objects and associated pids.
> 
> A few syscall tracepoints that report the vpid as passed down from
> userspace get adjusted in their output so that it is clear that no
> global pid is shown.
> 
> Signed-off-by: Jan Kiszka 
> ---
> 
> Changes in v3:
>  - handle process-local thread lookups correctly (hopefully)
> 
> This is in next for a while and ran several test cycles. Even if we 
> should have remaining issues under pid namespaces, we should not have 
> regressions in the normal cases.
> 
>  include/cobalt/kernel/thread.h |  2 ++
>  kernel/cobalt/posix/event.c|  2 +-
>  kernel/cobalt/posix/mqueue.c   |  2 +-
>  kernel/cobalt/posix/process.c  | 14 ++
>  kernel/cobalt/posix/process.h  |  2 ++
>  kernel/cobalt/posix/sched.c| 10 ++
>  kernel/cobalt/posix/sem.c  |  2 +-
>  kernel/cobalt/posix/signal.c   | 10 ++
>  kernel/cobalt/posix/thread.c   | 18 ++
>  kernel/cobalt/posix/thread.h   |  2 +-
>  kernel/cobalt/thread.c | 10 ++
>  kernel/cobalt/trace/cobalt-posix.h |  4 ++--
>  12 files changed, 56 insertions(+), 22 deletions(-)
> 
> diff --git a/include/cobalt/kernel/thread.h b/include/cobalt/kernel/thread.h
> index b79cb84296..4c8a0b9f2a 100644
> --- a/include/cobalt/kernel/thread.h
> +++ b/include/cobalt/kernel/thread.h
> @@ -450,6 +450,8 @@ char *xnthread_format_status(unsigned long status,
>  
>  pid_t xnthread_host_pid(struct xnthread *thread);
>  
> +pid_t xnthread_host_vpid(struct xnthread *thread);
> +
>  int xnthread_set_clock(struct xnthread *thread,
>  struct xnclock *newclock);
>  
> diff --git a/kernel/cobalt/posix/event.c b/kernel/cobalt/posix/event.c
> index 3712154f53..5871106301 100644
> --- a/kernel/cobalt/posix/event.c
> +++ b/kernel/cobalt/posix/event.c
> @@ -352,7 +352,7 @@ COBALT_SYSCALL(event_inquire, current,
>   xnsynch_for_each_sleeper(thread, &event->synch) {
>   if (nrwait >= nrpids)
>   break;
> - t[nrwait++] = xnthread_host_pid(thread);
> + t[nrwait++] = xnthread_host_vpid(thread);
>   }
>   }
>  
> diff --git a/kernel/cobalt/posix/mqueue.c b/kernel/cobalt/posix/mqueue.c
> index dd8acd55b0..fcde87a077 100644
> --- a/kernel/cobalt/posix/mqueue.c
> +++ b/kernel/cobalt/posix/mqueue.c
> @@ -720,7 +720,7 @@ mq_notify(struct cobalt_mqd *mqd, unsigned index, const 
> struct sigevent *evp)
>* receiver's namespaces. We pass the receiver's creds
>* into the init namespace instead.
>*/
> - mq->si.si_pid = task_pid_nr(current);
> + mq->si.si_pid = task_pid_vnr(current);
>   mq->si.si_uid = get_current_uuid();
>   }
>  
> diff --git a/kernel/cobalt/posix/process.c b/kernel/cobalt/posix/process.c
> index 1f059ad830..98b7df33fb 100644
> --- a/kernel/cobalt/posix/process.c
> +++ b/kernel/cobalt/posix/process.c
> @@ -975,6 +975,20 @@ int cobalt_handle_cleanup_event(struct mm_struct *mm)
>   return KEVENT_PROPAGATE;
>  }
>  
> +pid_t cobalt_vpid2pid_nr(pid_t vpid)
> +{
> + struct pid *pid;
> + pid_t nr = -1;
> +
> + rcu_read_lock();

This is not a good idea, at least for I-pipe which is not RCU-compatible
(Dovetail just gained that).

I think we need to rest this approach for now, either under I-pipe
gained support for RCU read-side access or we switched to Dovetail
completely. Pulling it from next.

Jan

> + pid = find_vpid(vpid);
> + if (pid)
> + nr = pid_nr(pid);
> + rcu_read_unlock();
> +
> + return nr;
> +}
> +
>  static int attach_process(struct cobalt_process *process)
>  {
>   struct cobalt_ppd *p = &process->sys_ppd;
> diff --git a/kernel/cobalt/posix/process.h b/kernel/cobalt/posix/process.h
> index 279707a680..46dff43b7a 100644
> --- a/kernel/cobalt/posix/process.h
> +++ b/kernel/cobalt/posix/process.h
> @@ -175,4 +175,6 @@ int cobalt_handle_cleanup_event(struct mm_struct *mm);
>  
>  int cobalt_handle_user_return(struct task_struct *task);
>  
> +pid_t cobalt_vpid2pid_nr(pid_t vpid);
> +
>  #endif /* !_COBALT_POSIX_PROCESS_H */
> diff --git a/kernel/cobalt/posix/sched.c b/kernel/cobalt/posix/sched.c
> index 4fd9c2b46e..26259fbd3f 100644
> --- a/kernel/cobalt/posix/sched.c
> +++ b/kernel/cobalt/posix/sched.c
> @@ -736,7 +736,7 @@ COBALT_SYSCALL(sched_weightprio, current,
>   return __cobalt_sched_weightprio(po