From 40860dac20b02b5b471fb3c22cd543735123eb10 Mon Sep 17 00:00:00 2001 From: Martijn Dekker Date: Sun, 7 Mar 2021 17:01:17 +0000 Subject: [PATCH] job_init(): fix init on setpgid() permission denied (re: 41ebb55a) Symptoms of this bug below. These only seem to occur on Linux and only if you replace your initial login shell by ksh using 'exec'. 1. An erroneous 'Interrupt' message is printed after stopping the read builtin in a script. Reproducer: $ exec arch/*/bin/ksh $ cat ./reproducer.sh #!/bin/sh read foo $ ./reproducer.sh ^C$ [1] + Interrupt ../reproducer.sh 2. Ctrl+C fails to stop /bin/package make. Reproducer: $ exec arch/*/bin/ksh $ mv arch arch.old $ bin/package make # Press Ctrl+C multiple times Analysis: In 41ebb55a, I made an error in changing job_init() to work correctly on non-interactive shells. This line from before: 552| if(possible = (setpgid(0,job.mypgid)>=0) || errno==EPERM) was changed to: 555| possible = (setpgid(0,job.mypgid) >= 0); 556| if(sh_isoption(SH_INTERACTIVE) && (possible || errno==EPERM)) That is wrong. Before, 'possible' was set to 1 (true) if setpgid() either succeeded or failed with EPERM. After, it is only set to 1 if setpgid() succeeds. As a result, job control initialisation is aborted later on upon a test for non-zero 'possible'. src/cmd/ksh93/sh/jobs.c: job_init(): - Once again set possible to 1 even if setpgid() fails with EPERM. Thanks to @JohnoKing for the bug report and reproducers. Resolves: https://github.com/ksh93/ksh/issues/210 --- NEWS | 3 +++ src/cmd/ksh93/sh/jobs.c | 4 ++-- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/NEWS b/NEWS index 8ddc9ec84..5fccf2eee 100644 --- a/NEWS +++ b/NEWS @@ -13,6 +13,9 @@ Any uppercase BUG_* names are modernish shell bug IDs. if they match a normal (non-**) glob pattern. For example, if '/lnk' is a symlink to a directory, '/lnk/**' and '/l?k/**' now work as you would expect. +- Fixed a bug introduced on 2021-02-11 that caused job control on interactive + ksh sessions to misbehave if the login shell was replaced by ksh using 'exec'. + 2021-03-06: - Fixed an old expansion bug: expansions of type ${var=value} and ${var:=value} diff --git a/src/cmd/ksh93/sh/jobs.c b/src/cmd/ksh93/sh/jobs.c index 4c53204ea..e1c664b1d 100644 --- a/src/cmd/ksh93/sh/jobs.c +++ b/src/cmd/ksh93/sh/jobs.c @@ -552,8 +552,8 @@ void job_init(Shell_t *shp, int lflag) job.mypgid = shp->gd->pid; } #ifdef SIGTSTP - possible = (setpgid(0,job.mypgid) >= 0); - if(sh_isoption(SH_INTERACTIVE) && (possible || errno==EPERM)) + possible = (setpgid(0,job.mypgid) >= 0) || errno==EPERM; + if(sh_isoption(SH_INTERACTIVE) && possible) { /* wait until we are in the foreground */ while((job.mytgid=tcgetpgrp(JOBTTY)) != job.mypgid)