sai: give rs274 its own tool mmap instead of truncating $HOME/.tool.mmap by tzuohann · Pull Request #4385 · LinuxCNC/linuxcnc

tzuohann · 2026-08-16T06:21:07Z

Running rs274 while a LinuxCNC session is up replaces that session's tool
table with the sample one (shipped with lcnc), until restart.

A lucky user will find that conflicting MDI or gcode stops running. A less
lucky user will find code running with offsets from sample tools - potentially
disastrous.

tool_mmap_creator() opens $HOME/.tool.mmap O_RDWR|O_CREAT|O_TRUNC
(tooldata_mmap.cc:33, :135) and sai calls it at driver.cc:570 — before
getopt() at :578, so it happens on every invocation, --help included, and
-t (read at :583) cannot prevent it.

io, milltask, halui and the Python bindings all map that same inode
MAP_SHARED. O_TRUNC keeps the inode, so nothing re-maps and nothing
errors — the running session simply starts using the sample table.

Reproduce:

rm -rf /tmp/rsx && mkdir -p /tmp/rsx
HOME=/tmp/rsx rs274 -g /dev/null
# /tmp/rsx/.tool.mmap, last_index=4, sample table

Patch:

Gives sai its own pid-qualified file under /tmp, unlinked on exit, so an
offline check can never touch a running session's table.
tool_mmap_close() already unlinks and tool_mmap_fname() already honours a
preset name; this only adds the setter to reach it. io and milltask are
unchanged.

Seen on 2.9.8, identical at 2.9.10.

tool_mmap_creator() opens the file O_RDWR|O_CREAT|O_TRUNC (tooldata_mmap.cc:33, used at :135) and sai calls it at driver.cc:570 -- before getopt() at :578. Every rs274 invocation therefore empties $HOME/.tool.mmap, including `rs274 --help` and including one that supplies -t, since -t is not read until :583. That file is not scratch space: tool_mmap_fname() builds it from secure_getenv("HOME") with a fixed name, and io, milltask, halui and the Python bindings all map that same inode MAP_SHARED. An offline parse run beside a live session therefore replaces the running machine's tool table with the compiled-in sample table. O_TRUNC preserves the inode, so nothing re-maps and nothing is notified: the session simply observes its tools change. Observed on a machine using [EMCIO]DB_PROGRAM -- 15 tools became the 4 sample entries mid-session, G43 applied 0.0000 for a tool that was no longer in the table, and the tool-number/drawbar guard inhibited jog and feed. DB_PROGRAM neither prevents nor repairs it: ioControl.cc creates the mmap before the DB_ACTIVE branch, and io does not re-read afterwards. Give sai its own file and unlink it on exit. tool_mmap_close() already unlinks and tool_mmap_fname() already honours a preset filename -- this only adds the setter to reach it. io and milltask are untouched and remain the only creators of the shared file. Two points from review, both addressed here: mkstemp(), not a name built from the pid (grandixximo). TMPDIR is world-writable and a pid is guessable, and the creator opens without O_EXCL or O_NOFOLLOW, so a predictable name can be pre-created as a symlink and the victim's rs274 then truncates the attacker's chosen file -- and an attacker can blanket a pid range in advance. mkstemp() creates it atomically with O_EXCL and mode 0600, and TOOL_MMAP_CREATOR_OPEN_FLAGS gains O_NOFOLLOW so the creator refuses a symlink at that path even if one appears in the gap. tool_mmap_close() is now safe as an atexit handler (BsAtHome). It called exit(EXIT_FAILURE) when munmap failed, and calling exit() from within an atexit handler is undefined behaviour; _exit would skip the remaining handlers, so that is not the answer either. It now reports the failure, closes the fd and returns. Reproduce before the change: rm -rf /tmp/rsx && mkdir -p /tmp/rsx HOME=/tmp/rsx rs274 -g /dev/null # /tmp/rsx/.tool.mmap, last_index=4, holding # T1 z0.511 d0.125 / T2 z0.100 d0.0625 / T3 z1.273 d0.201 / T99999 P123 After: rs274 writes $TMPDIR/rs274.tool.mmap.XXXXXX, removes it on exit, and does not open $HOME/.tool.mmap. Verified on a live machine: an rs274 run with the real $HOME left last_index=15 and every tool untouched.

BsAtHome · 2026-08-18T12:16:34Z

+    // close, returns at the tool_mmap_base guard above.
+    tool_mmap_base = (char*)0;


Please use NULL or nullptr instead of manual casts of the value zero (0).

Done with nullptr at 241 and also at 200 (another one).

BsAtHome · 2026-08-18T12:22:27Z

BTW, I see this is targeted at 2.9 branch. The same problem in master, I guess?

BsAtHome, review 2026-08-18: `(char*)0` is a hand-written null pointer. Both occurrences in tooldata_mmap.cc now use nullptr. No behaviour change.

tzuohann · 2026-08-19T00:22:10Z

BTW, I see this is targeted at 2.9 branch. The same problem in master, I guess?

Yes. Its in master. I wasn't sure if you want to retarget this, or do a new PR. Just LMK, or DIY.

BsAtHome · 2026-08-19T08:40:49Z

Yes, then you need to retarget this PR to master.

grandixximo · 2026-08-20T02:58:05Z

The bug is identical in master, so the fix is wanted in both. The question is just direction.

I asked tzuohann to point this at 2.9 since it was reported there (2.9.8, still in 2.9.10). Our usual flow is master-first, then backport. Would the reverse work here: keep this PR on 2.9 and merge 2.9 into master to carry it up? I checked the patch against both branches; it applies cleanly to 2.9, and the master differences in these files are only cosmetic, so the merge-up should be near-trivial.

If you would rather have master-first, no problem, tzuohann can retarget and we backport.

BsAtHome · 2026-08-20T08:58:41Z

No, by all means, go for it. If this needs to be applied in both places, then it does not really matter in which order.

BsAtHome reviewed Aug 16, 2026

View reviewed changes

Comment thread src/emc/sai/driver.cc

grandixximo reviewed Aug 16, 2026

View reviewed changes

Comment thread src/emc/sai/driver.cc

tzuohann force-pushed the rs274-private-toolmmap branch from 3c37f31 to 74522c6 Compare August 16, 2026 14:44

BsAtHome reviewed Aug 16, 2026

View reviewed changes

Comment thread src/emc/tooldata/tooldata_mmap.cc

tzuohann force-pushed the rs274-private-toolmmap branch from 74522c6 to 8c8e048 Compare August 17, 2026 02:31

BsAtHome reviewed Aug 17, 2026

View reviewed changes

Comment thread src/emc/tooldata/tooldata_mmap.cc

BsAtHome reviewed Aug 18, 2026

View reviewed changes

tooldata: use nullptr rather than casting 0 to a pointer

f61e3fa

BsAtHome, review 2026-08-18: `(char*)0` is a hand-written null pointer. Both occurrences in tooldata_mmap.cc now use nullptr. No behaviour change.

grandixximo merged commit 5a7a31a into LinuxCNC:2.9 Aug 20, 2026
17 checks passed

grandixximo mentioned this pull request Aug 20, 2026

Merge branch '2.9' into master #4427

Open

Navigation Menu

Uh oh!

sai: give rs274 its own tool mmap instead of truncating $HOME/.tool.mmap - #4385

sai: give rs274 its own tool mmap instead of truncating $HOME/.tool.mmap#4385
grandixximo merged 2 commits into
LinuxCNC:2.9from
tzuohann:rs274-private-toolmmap

tzuohann commented Aug 16, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

BsAtHome Aug 18, 2026

Uh oh!

tzuohann Aug 19, 2026

Uh oh!

Uh oh!

BsAtHome commented Aug 18, 2026

Uh oh!

tzuohann commented Aug 19, 2026

Uh oh!

BsAtHome commented Aug 19, 2026

Uh oh!

grandixximo commented Aug 20, 2026

Uh oh!

BsAtHome commented Aug 20, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

		// close, returns at the tool_mmap_base guard above.
		tool_mmap_base = (char*)0;

Uh oh!

Conversation

tzuohann commented Aug 16, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

BsAtHome Aug 18, 2026

Choose a reason for hiding this comment

Uh oh!

tzuohann Aug 19, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

BsAtHome commented Aug 18, 2026

Uh oh!

tzuohann commented Aug 19, 2026

Uh oh!

BsAtHome commented Aug 19, 2026

Uh oh!

grandixximo commented Aug 20, 2026

Uh oh!

BsAtHome commented Aug 20, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants