Skip to content

Report job number and PID for async commands in interactive shell#155

Merged
magicant merged 1 commit into
trunkfrom
async-report
Apr 13, 2025
Merged

Report job number and PID for async commands in interactive shell#155
magicant merged 1 commit into
trunkfrom
async-report

Conversation

@magicant

Copy link
Copy Markdown
Owner

The shell now reports the job number and process ID of asynchronous commands in an interactive shell, as required by POSIX.1-2024.

The shell now reports the job number and process ID of asynchronous
commands in an interactive shell, as required by POSIX.1-2024.
@magicant magicant added this to the 2.59 milestone Apr 13, 2025
@magicant magicant self-assigned this Apr 13, 2025
Comment thread exec.c
job->j_pgid = doing_job_control_now ? pgid : 0;
job->j_status = JS_RUNNING;
job->j_statuschanged = true;
job->j_statuschanged = false;

Check failure

Code scanning / CodeQL

Potential use after free

Memory may have been previously freed by [call to free](1).

Copilot Autofix

AI over 1 year ago

To fix the problem, we need to ensure that job is not accessed after it has been freed. One way to achieve this is to set the job pointer to NULL immediately after freeing it. This way, any subsequent access to job can be checked against NULL, preventing the use of a dangling pointer.

The best way to fix this without changing existing functionality is to set job to NULL right after the free(job) call on line 449. Additionally, we should add a check before accessing job on line 480 to ensure it is not NULL.

Suggested changeset 1
exec.c

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/exec.c b/exec.c
--- a/exec.c
+++ b/exec.c
@@ -449,2 +449,3 @@
             free(job);
+            job = NULL;
             connect_pipes(&pipe);
@@ -477,9 +478,11 @@
     /* establish the job and wait for it */
-    job->j_pgid = doing_job_control_now ? pgid : 0;
-    job->j_status = JS_RUNNING;
-    job->j_statuschanged = false;
-    job->j_legacy = false;
-    job->j_nonotify = false;
-    job->j_pcount = count;
-    set_active_job(job);
+    if (job != NULL) {
+        job->j_pgid = doing_job_control_now ? pgid : 0;
+        job->j_status = JS_RUNNING;
+        job->j_statuschanged = false;
+        job->j_legacy = false;
+        job->j_nonotify = false;
+        job->j_pcount = count;
+        set_active_job(job);
+    }
     if (type != E_ASYNC) {
EOF
@@ -449,2 +449,3 @@
free(job);
job = NULL;
connect_pipes(&pipe);
@@ -477,9 +478,11 @@
/* establish the job and wait for it */
job->j_pgid = doing_job_control_now ? pgid : 0;
job->j_status = JS_RUNNING;
job->j_statuschanged = false;
job->j_legacy = false;
job->j_nonotify = false;
job->j_pcount = count;
set_active_job(job);
if (job != NULL) {
job->j_pgid = doing_job_control_now ? pgid : 0;
job->j_status = JS_RUNNING;
job->j_statuschanged = false;
job->j_legacy = false;
job->j_nonotify = false;
job->j_pcount = count;
set_active_job(job);
}
if (type != E_ASYNC) {
Copilot is powered by AI and may make mistakes. Always verify output.
@magicant
magicant merged commit 4f00c6b into trunk Apr 13, 2025
@magicant
magicant deleted the async-report branch April 13, 2025 07:53
@magicant magicant mentioned this pull request Apr 13, 2025
56 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants