JianyuWang0623 opened a new pull request, #3496:
URL: https://github.com/apache/nuttx-apps/pull/3496

   ## Summary
   
   Fixes an infinite loop in `nsh_catfile` (used by `cat` / `dmesg`) when 
stdout is broken — for example when a PTY master is closed while the shell is 
dumping `/dev/log`.
   
   Failure mode:
   
   1. `nsh_catfile` reads a chunk from the input file (e.g. `/dev/log`).
   2. `nsh_consolewrite` calls `write(OUTFD, ...)`, which fails 
(EPIPE/EIO/ENOSPC/…).
   3. The old code logged the failure via `_err()`, which the syslog backend 
turned into more bytes on `/dev/log`.
   4. `nsh_catfile` ignored the write error and looped back to `read()`, 
picking up the bytes that step 3 just produced.
   5. Goto 2 — the shell spins forever at the highest priority.
   
   Two independent fixes, either of which would break the loop, both applied 
for defense in depth:
   
   - **`nshlib/nsh_console.c`**: drop the `_err()` in `nsh_consolewrite` 
entirely. The hazard is errno-agnostic: any logging at this layer can be 
re-injected when `OUTFD` is bound to the syslog backend. Callers already get 
the failure via the negative return value and `errno`, so on-failure logging 
here is redundant.
   - **`nshlib/nsh_fsutils.c`**: on inner-loop write failure in `nsh_catfile`, 
jump straight to a single `errout:` cleanup label instead of using a two-level 
`break` and falling through to another `read()`. A failed write can no longer 
be followed by another read of the same fd.
   
   ## Impact
   
   - **User-visible behavior**: `cat /dev/log`, `dmesg`, and similar commands 
now exit cleanly with `ERROR` when stdout is broken, instead of hanging the 
shell. No change on the success path or for non-syslog outputs.
   - **Logging**: write failures inside `nsh_consolewrite` are no longer logged 
at this layer. The information is still available to callers through the return 
value and `errno`, and other layers (e.g. command implementations) remain free 
to report errors as appropriate.
   - **API / build / config**: none. No public API, Kconfig, or build changes.
   - **Security**: removes a denial-of-service style hang triggered by closing 
the controlling PTY while NSH is streaming the syslog device.
   - **Compatibility**: backward compatible. Behavior only changes on the 
previously broken error path.
   - **Documentation**: no doc changes required.
   
   ## Testing
   Regression checks on the happy path:
   
   - `cat <regular file>` — unchanged output, exit status 0.
   - `dmesg` to a healthy console — unchanged output.
   - `cat` of a non-existent / unreadable file — still reports the original 
error via the existing `nsh_error()` paths in callers; no behavioral change 
beyond the dropped `_err()` line in `nsh_consolewrite`.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to