Skip to content

Commit 199d4d9

Browse files
committed
setpgid forms a real unit, kill(-n) reaches it, and a spawn can chdir
Three calls a shell runner makes, each of which used to succeed and change nothing a caller could observe --- which is the failure shape this port names in okm_opt.h and had three more instances of. `setpgid(0, 0)' answered 0 and formed nothing. True in a world with no groups, so it was not a lie; it was also not a unit, and the caller's next act --- `kill(-pid)' --- found nothing to kill. It is now `kal_process_job_enter'. `kill(-n)' answered ESRCH while a unit existed. It now names the unit: the one this program formed, or the one a start formed for a child. ⚠️ THE UNIT IS KEPT PAST THE WAIT, and that is the case a unit is used FOR --- a shell exits at once and the work it backgrounded is what a timeout has to reach. Clearing it on the wait made this answer ESRCH a fraction of a second before every caller that wants it; measured, the background work survived. `posix_spawn_file_actions_addchdir_np' was refused along with everything this file could not express. openkal 0.11 gives a spawn a second directory, so both chdir actions are now answered. ⚠️ `POSIX_SPAWN_SETPGROUP' is honoured only in the form this port can mean: a zero group asks for a unit of the program's own, which is exactly `kal_spawn.job'. A NAMED group stays refused --- openkal cannot put a program into somebody else's unit, and turning that request into a different one quietly is the defect above with a fourth instance. ⚠️⚠️ AND ONE IDIOM REMAINS OUT OF REACH, WHICH IS RECORDED AND NOT APPROXIMATED. `fork(); setpgid(0, 0); exec…' forms a unit led by the program the COPY starts, whose identity the original never learns --- so the original's `kill(-pid)' names a group that is not the one that exists. Closing it needs the copy and its parent to agree on a name before either exists, which nothing here can arrange. `posix_spawn' with the attribute above is the form that works, and it is what the consumer moved to.
1 parent 22ae529 commit 199d4d9

3 files changed

Lines changed: 153 additions & 12 deletions

File tree

port/src/okm_opt.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -170,6 +170,8 @@ static inline int okm_fs_list_next(struct kal_dir, kal_uintptr*, char*, kal_uint
170170
#if OKM_HAS_PROCESS
171171

172172
#define okm_process_spawn kal_process_spawn
173+
#define okm_process_job_enter kal_process_job_enter
174+
#define okm_process_job_terminate kal_process_job_terminate
173175
#define okm_process_wait kal_process_wait
174176
#define okm_process_terminate kal_process_terminate
175177
#define okm_process_close kal_process_close
@@ -208,6 +210,8 @@ static inline int okm_process_spawn(const struct kal_spawn*, const char*, kal_ui
208210
static inline int okm_process_wait(struct kal_process, int*,
209211
int*) { return kal_err_not_supported; }
210212
static inline int okm_process_terminate(struct kal_process) { return kal_err_not_supported; }
213+
static inline int okm_process_job_enter(struct kal_job*) { return kal_err_not_supported; }
214+
static inline int okm_process_job_terminate(struct kal_job) { return kal_err_not_supported; }
211215
static inline void okm_process_close(struct kal_process) {}
212216

213217
#endif /* OKM_HAS_PROCESS */

port/src/okm_spawn.c

Lines changed: 38 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,7 @@
7878
#define OKM_SPAWN_MAX_OPEN 8
7979

8080
int __okm_child_record(struct kal_process h);
81+
int __okm_child_record_job(struct kal_process h, struct kal_job j);
8182

8283
static size_t slen(const char* s) { size_t n = 0; while (s && s[n]) n++; return n; }
8384

@@ -417,6 +418,11 @@ int __okm_spawn_common(pid_t* restrict res, const char* restrict path,
417418
struct kal_file opened[OKM_SPAWN_MAX_OPEN];
418419
int opened_n = 0;
419420

421+
/* The directory the program is to run in, if a file action named one. It is
422+
* this call's and is released with the opened files below. */
423+
struct kal_dir where;
424+
int where_held = 0;
425+
420426
/* Resolving a file action's name needs one of these and it is four kilobytes.
421427
* Static, under this file's lock, for the same reason the argument vectors
422428
* above are: a second one on the stack would double the frame of a function
@@ -485,8 +491,33 @@ int __okm_spawn_common(pid_t* restrict res, const char* restrict path,
485491
if (op->fd > 2) break;
486492
refused = ENOSYS;
487493
break;
488-
case FDOP_CHDIR:
489-
case FDOP_FCHDIR:
494+
/* ⭐⭐ ANSWERED SINCE 0.12, BECAUSE openkal 0.11 GAVE A SPAWN A SECOND
495+
* DIRECTORY. Both of these say the same thing --- run the program
496+
* HERE --- and until there was a place to put it they were refused
497+
* along with everything else this file could not express. */
498+
case FDOP_CHDIR: {
499+
struct okm_at cat;
500+
const int r = okm_resolve(AT_FDCWD, op->path, &cat, 0);
501+
if (r) { refused = (int)-r; break; }
502+
struct kal_dir d;
503+
const int e = okm_fs_open_dir(cat.base, cat.rel, slen(cat.rel), &d);
504+
if (e != kal_ok) { refused = okm_errno(e); break; }
505+
if (where_held) okm_fs_close_dir(where);
506+
where = d; where_held = 1;
507+
break;
508+
}
509+
case FDOP_FCHDIR: {
510+
struct okm_desc* dd = okm_desc_of(op->fd);
511+
if (!dd || dd->kind != OKM_DIR) { refused = EBADF; break; }
512+
/* A directory of this program's own, opened again so that the
513+
* spawn holds one the caller cannot close underneath it. */
514+
struct kal_dir d;
515+
const int e = okm_fs_open_dir(dd->dir, ".", 1, &d);
516+
if (e != kal_ok) { refused = okm_errno(e); break; }
517+
if (where_held) okm_fs_close_dir(where);
518+
where = d; where_held = 1;
519+
break;
520+
}
490521
default:
491522
/* An action openkal cannot express. Performing the spawn
492523
* without it would start the program in a state the caller did
@@ -512,6 +543,7 @@ int __okm_spawn_common(pid_t* restrict res, const char* restrict path,
512543

513544
if (refused) {
514545
for (int i = 0; i < opened_n; i++) okm_fs_close_file(opened[i]);
546+
if (where_held) okm_fs_close_dir(where);
515547
okm_unlock();
516548
return refused;
517549
}
@@ -534,7 +566,8 @@ int __okm_spawn_common(pid_t* restrict res, const char* restrict path,
534566
* else entirely. Passing it now closes that, and it closes the `fork' route
535567
* as well: a copy that chdir'd has its own `okm_cwd_dir', and the `execve'
536568
* it then performs reaches this line. */
537-
int e = start_program(bound, want_unit, okm_cwd_dir, &at, a_ptr, a_len, argc,
569+
int e = start_program(bound, want_unit, where_held ? where : okm_cwd_dir,
570+
&at, a_ptr, a_len, argc,
538571
e_ptr, e_len, envc, &streams, &child);
539572

540573
/* ⭐ THE ONE ENVIRONMENT THAT SPELLS A PROGRAM WITH A SUFFIX IS ANSWERED
@@ -563,11 +596,12 @@ int __okm_spawn_common(pid_t* restrict res, const char* restrict path,
563596
* from a file, and openkal-musl asks the specification to say so for streams
564597
* in general rather than for one of them. */
565598
for (int i = 0; i < opened_n; i++) okm_fs_close_file(opened[i]);
599+
if (where_held) okm_fs_close_dir(where);
566600

567601
okm_unlock();
568602
if (e != kal_ok) return okm_errno(e);
569603

570-
const int pid = __okm_child_record(child);
604+
const int pid = __okm_child_record_job(child, unit);
571605
if (pid < 0) { okm_process_close(child); return EAGAIN; }
572606
*res = (pid_t)pid;
573607
return 0;

port/src/okm_syscall.c

Lines changed: 111 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -562,7 +562,15 @@ static void to_timespec(kal_duration ns, struct timespec* ts)
562562
* others --- which is the kind of program that meets it --- and because each
563563
* entry is three words. */
564564
#define OKM_MAX_CHILD 256
565-
static struct { int used; int pid; struct kal_process h; } g_child[OKM_MAX_CHILD];
565+
/* ⭐ AND THE UNIT THAT START FORMED, WHICH IS THE ONLY PLACE IT CAN LIVE.
566+
* A caller ends a group by naming a negative identifier; openkal names a unit by
567+
* a handle whose meaning is the implementation's. The two are related here and
568+
* nowhere else --- openkal deliberately offers no way to recover a unit from a
569+
* program, because an implementation that had to keep a table to answer that
570+
* would be the defect clause 7.1 describes. This library is not an
571+
* implementation of openkal; keeping what it was GIVEN is bookkeeping, not
572+
* recovery. */
573+
static struct { int used; int pid; struct kal_process h; struct kal_job job; int has_job; } g_child[OKM_MAX_CHILD];
566574
static int g_next_pid = 1000;
567575

568576
/* ⭐ WHAT THIS PROGRAM ANSWERS WHEN ASKED WHO IT IS.
@@ -578,6 +586,15 @@ static int g_next_pid = 1000;
578586
* `fork' returned it. */
579587
static int g_self_pid = 1;
580588

589+
/* The unit this program formed, if it did. ⚠️ KEPT BECAUSE THE NUMBER A CALLER
590+
* WILL LATER USE IS NOT THE HANDLE: `kill(-n)' names a group by a POSIX
591+
* identifier, and openkal's unit is a handle whose meaning belongs to the
592+
* implementation. One per program, which is what one `setpgid(0, 0)' forms, so
593+
* this is a word and not a table --- and a copy made by `fork' carries it, which
594+
* is what lets a copy form the unit and its parent end it. */
595+
static struct kal_job g_job;
596+
static int g_job_held;
597+
581598
void __okm_set_self_pid(int pid) { g_self_pid = pid; }
582599

583600
/* Takes an entry and settles its identifier WITHOUT a resource to put in it.
@@ -598,6 +615,8 @@ int __okm_child_reserve(int* pid_out)
598615
g_child[i].used = 1;
599616
g_child[i].pid = ++g_next_pid;
600617
g_child[i].h = (struct kal_process){ 0 };
618+
g_child[i].job = (struct kal_job){ 0 };
619+
g_child[i].has_job = 0;
601620
if (pid_out) *pid_out = g_child[i].pid;
602621
return i;
603622
}
@@ -609,22 +628,50 @@ void __okm_child_commit(int slot, struct kal_process h)
609628
if (slot >= 0 && slot < OKM_MAX_CHILD) g_child[slot].h = h;
610629
}
611630

631+
/* The unit a start formed for this child, recorded so that a later `kill(-n)'
632+
* can name it. Separate from the commit above because a start forms a unit only
633+
* when the caller asked for one. */
634+
void __okm_child_job(int slot, struct kal_job j)
635+
{
636+
if (slot >= 0 && slot < OKM_MAX_CHILD && j.h != 0) {
637+
g_child[slot].job = j;
638+
g_child[slot].has_job = 1;
639+
}
640+
}
641+
612642
void __okm_child_release(int slot)
613643
{
644+
/* ⚠️ THE UNIT IS NOT RELEASED WITH THE PROGRAM, AND THAT IS THE POINT OF IT.
645+
* A group outlives the program that formed it for exactly as long as it has
646+
* members --- which is the case a caller uses a unit FOR: the shell exits
647+
* immediately and the work it put in the background is what a timeout has to
648+
* reach. Clearing this on the wait made `kill(-n)' answer ESRCH the moment
649+
* the shell was reaped, which is a fraction of a second before every caller
650+
* that wants it. Measured: the background work survived.
651+
*
652+
* The slot is free for a new program; the number and its unit stay until
653+
* that slot is taken again. */
614654
if (slot >= 0 && slot < OKM_MAX_CHILD) g_child[slot].used = 0;
615655
}
616656

617-
int __okm_child_record(struct kal_process h)
657+
int __okm_child_record_job(struct kal_process h, struct kal_job j)
618658
{
619659
okm_lock();
620660
int pid = 0;
621661
const int slot = __okm_child_reserve(&pid);
622662
if (slot < 0) { okm_unlock(); return -EAGAIN; }
623663
__okm_child_commit(slot, h);
664+
__okm_child_job(slot, j);
624665
okm_unlock();
625666
return pid;
626667
}
627668

669+
int __okm_child_record(struct kal_process h)
670+
{
671+
const struct kal_job none = { 0 };
672+
return __okm_child_record_job(h, none);
673+
}
674+
628675
/* ⚠️ A COPY OF THE CALLING IMAGE INHERITS THIS TABLE AND MUST NOT KEEP IT.
629676
*
630677
* The entries name programs the ORIGINAL started, and POSIX is explicit that a
@@ -648,6 +695,15 @@ static int child_index(int pid)
648695
return -1;
649696
}
650697

698+
/* The unit a program formed, which is looked up WITHOUT requiring the program to
699+
* still be running --- see __okm_child_release. */
700+
static int job_index(int pid)
701+
{
702+
for (int i = 0; i < OKM_MAX_CHILD; i++)
703+
if (g_child[i].has_job && g_child[i].pid == pid) return i;
704+
return -1;
705+
}
706+
651707
/* --- what a signal aimed at this program means ------------------------------
652708
*
653709
* ⚠️⚠️ `abort' DID NOT END THE PROGRAM, AND WHAT ENDED IT WAS AN ILLEGAL
@@ -1824,6 +1880,31 @@ syscall_arg_t __okm_syscall(syscall_arg_t n, syscall_arg_t a1, syscall_arg_t a2,
18241880
#ifdef SYS_kill
18251881
case SYS_kill: {
18261882
const int pid = (int)a1, sig = (int)a2;
1883+
/* ⭐⭐ A NEGATIVE IDENTIFIER NAMES A UNIT, AND THIS IS THE OTHER HALF OF
1884+
* `setpgid(0, 0)' ABOVE.
1885+
*
1886+
* `kill(-n)' is how a caller ends a group, and the identifier it uses is
1887+
* the one `setpgid' gave it --- either this program's, or a child's whose
1888+
* copy formed the unit. Both reach the unit this library holds, because
1889+
* a program forms at most one and a copy carries its parent's.
1890+
*
1891+
* ⚠️ WITHOUT THIS THE FORMING WOULD BE INVISIBLE. That is the shape of
1892+
* the two defects before it: a call that succeeds and changes nothing
1893+
* observable is worse than one that refuses, because the caller proceeds.
1894+
* `kill(-n)' answered ESRCH here while the unit existed. */
1895+
if (pid < 0 && pid != -1) {
1896+
const int gi = job_index(-pid);
1897+
if (gi >= 0 && g_child[gi].has_job) {
1898+
if (sig == 0) return 0;
1899+
const int e = okm_process_job_terminate(g_child[gi].job);
1900+
return e == kal_ok ? 0 : -okm_errno(e);
1901+
}
1902+
if (g_job_held) {
1903+
if (sig == 0) return 0;
1904+
const int e = okm_process_job_terminate(g_job);
1905+
return e == kal_ok ? 0 : -okm_errno(e);
1906+
}
1907+
}
18271908
const int i = child_index(pid);
18281909
if (i >= 0) {
18291910
/* ⚠️ SIGNAL ZERO IS AN ENQUIRY AND USED TO TERMINATE THE CHILD.
@@ -2108,13 +2189,35 @@ syscall_arg_t __okm_syscall(syscall_arg_t n, syscall_arg_t a1, syscall_arg_t a2,
21082189
case SYS_getsid:
21092190
return (a1 == 0 || a1 == g_self_pid) ? g_self_pid : -ESRCH;
21102191

2111-
/* Naming this program, or naming nobody, asks for the arrangement that
2112-
* already holds. Naming anything else asks for a group this environment
2113-
* cannot name, which is EPERM rather than ENOSYS: the operation is here,
2114-
* the group is not. */
2192+
/* ⭐⭐ A REAL GROUP SINCE 0.12, AND IT IS THE CALL A SHELL RUNNER MAKES.
2193+
*
2194+
* `setpgid(0, 0)' asks that THIS program lead a unit of its own, which
2195+
* openkal 0.11 spells `kal_process_job_enter'. It used to answer 0 and form
2196+
* nothing --- true in a world with no groups, since a program is then
2197+
* trivially alone --- and the caller's next act, `kill(-pid)', found nothing
2198+
* to kill.
2199+
*
2200+
* ⚠️ THE UNIT IS KEPT, BECAUSE THE NUMBER THE CALLER WILL USE IS NOT ENOUGH.
2201+
* A caller ends a group by naming a negative identifier, and openkal's unit
2202+
* is a handle whose meaning is the implementation's --- a process group's
2203+
* identifier on one system, a job object on another. `g_job' is where this
2204+
* library remembers which unit `setpgid' formed, so that `kill(-n)' below can
2205+
* name it. One unit per program, which is what one `setpgid(0, 0)' forms.
2206+
*
2207+
* Naming another program's group is still refused: openkal cannot put a
2208+
* program into a unit it does not lead, and EPERM says the operation is here
2209+
* while the group is not. */
21152210
case SYS_setpgid:
2116-
if ((a1 == 0 || a1 == (syscall_arg_t)g_self_pid)
2117-
&& (a2 == 0 || a2 == (syscall_arg_t)g_self_pid)) return 0;
2211+
if (a1 != 0 && a1 != (syscall_arg_t)g_self_pid) return -EPERM;
2212+
if (a2 == 0 || a2 == (syscall_arg_t)g_self_pid) {
2213+
struct kal_job unit = { 0 };
2214+
const int e = okm_process_job_enter(&unit);
2215+
if (e == kal_ok) { g_job = unit; g_job_held = 1; return 0; }
2216+
/* An environment with no units leaves a program trivially alone in
2217+
* one, which is what this used to assert unconditionally. */
2218+
if (e == kal_err_not_supported) return 0;
2219+
return -okm_errno(e);
2220+
}
21182221
return -EPERM;
21192222
case SYS_setsid:
21202223
return -EPERM; /* already a process group leader; see above */

0 commit comments

Comments
 (0)