Rework slam function

[Narrat]

Two things get in this PR addressed:

Commit 1:

slam_tomb(): don't parse process output and rework

In https://github.com/dyne/tomb/pull/504 list_processes() got reworked in a way to avoid parsing process output as this had interesting side-effects. Back then I mentioned the same behaviour existing in slam_tomb() which should probably be changed too. This PR addresses that. Firstly it will use list_processes() from within slam_tomb(), as this is in principal overlapping functionality. For this list_processes() needed to be adjusted. It now has a return value which can indicate if there were processes. Secondly the order of execution was changed in slam_tomb(). Before it would process one process and work through the signals until this process was killed. Now it will take a signal and issue a kill for all processes found.

Commit 2:

slam_tomb: simplify and rename to _kill_processes

In general umount_tomb and slam_tomb shared a lot of similar code. Main difference being, that the latter additionally searched for processes and would still call umount_tomb if the processes could be killed. umount_tomb would then again search with the provided name for the relevant tomb in list_tomb_mounts, which should be obsolete at this point. Therefore the decision to reduce slam_tomb in functionality. It would only work on a supplied tombname and tombmount, look if there are processes and is called from within umount_tomb. (Theoretical tombname could be removed) Calling tomb with slam or close sets a flag, which will decide if that part in umount_tomb will be executed.

---

And the final PR for the time being :)

[Narrat]

Need to check if there are time restrictions for the CI (reached tests regarding steganography)

2024-08-08T18:03:02.9560868Z ##[error]The operation was canceled.

Didn't fail on a test and they run without error locally.

[jaromil]

@Narrat I need some help after solving conflicts with master. Maybe there is something I've done wrong?

Beware here I force pushed my conflict resolution over your old branch, so better save your own local copy of it for reference.

I like very much that we have a test now and also the ;& in the main() switch case gliding from slam to umount is very elegant!

[Narrat]

Roger. I will take a look

[Narrat]

I'm a bit puzzled :D What happens is kill_processes() gets the tombname with [ ] around. Therefore it will never run the kill loop and when checking if there are still processes at the mount location those will get detected. Why that happens I'm not sure yet, as this part of code didn't get touched in years. And it works if I go back to the original PR. https://github.com/dyne/tomb/blob/7ea0a3de59516292b81892699526c8b25a6f292e/tomb#L2939

# strip square parens from tombname
tombname=${t[(ws:;:)5]}

doesn't do its job. Or is the comment a reminder, that it needs to be done if one wants to use $tombname? Still curious that it worked originally. But I didn't see that $tombname got modified afterwards

Just rebased onto loop handling:

$  ./tomb -D slam test
tomb [D] Identified caller: testo (1000:100)
tomb [D] Tomb command: slam test
tomb [D] Caller: uid[1000], gid[100], tty[/dev/pts/5].
tomb [D] Temporary directory: /tmp
tomb [D] /dev/mapper/tomb.test.31362975dd289875e658e11829f42517bde608dd96736fcf33b008f930b7962b.loop0;/run/media/testo/test;ext4;(rw,nodev,noatime);[test] <--- complete line stored in $t
tomb [D] Name: [test] <--- tombname still(?) with []
tomb [D] Mount: /run/media/testo/test
tomb [D] Loop: loop0
tomb [D] Mapper: tomb.test.31362975dd289875e658e11829f42517bde608dd96736fcf33b008f930b7962b.loop0
tomb  .  Slamming tomb [test] mounted on /run/media/testo/test
tomb  .  What name? [test] <--- not the tomb name, therefore list_processes doesn't find anything and mount location seems clear
tomb [W] Couldn't kill 9971 <--- output from the check
tomb [W] Couldn't kill 10119
tomb [E] Still active processes for [test], cannot close tomb.

Original state of the PR: Still the wrong name, but working

$  ./tomb slam test                      
tomb  .  Slamming tomb [test] mounted on /run/media/testo/test
tomb  .  What name? [test]
tomb  .  Looking for processes running inside tomb '[test]'... <--- I know I added ' ' to accentuate the name because of the missing []. Well not so missing
COMMAND   PID   USER FD   TYPE DEVICE SIZE/OFF NODE NAME
less    14129 testo 4r   REG  254,0        5   18 /run/media/testo/test/hello
tomb  .  [test] sending TERM to open process 14129

Two ways: 1) tombname should really be without [] for the whole run

• that may have sideeffects for every function using tombname 2) just remove the [ ] when needed.

To get it working I will go with 2 for the time being. But what is your take on what should be done with $tombname in general?

[Narrat]

Force pushed the adjusted branch. Now the checks are happy again:

2024-09-01T16:24:46.6692141Z tomb [D] Tomb command: slam
2024-09-01T16:24:46.6709650Z tomb [D] Caller: uid[1001], gid[127], tty[::3 tty::].
2024-09-01T16:24:46.6726000Z tomb [D] Temporary directory: /tmp
2024-09-01T16:24:46.6791268Z tomb [D] /dev/mapper/tomb.test.ccffcb34977c2f4aad5a9405f6b9c77d909cf6ca53d9c8783bb6985a36f9b78a.loop3;/tmp/tomb/testmount;ext4;(rw,nodev,noatime);[test]
2024-09-01T16:24:46.6806556Z tomb [D] Name: [test]
2024-09-01T16:24:46.6822760Z tomb [D] Mount: /tmp/tomb/testmount
2024-09-01T16:24:46.6838771Z tomb [D] Loop: loop3
2024-09-01T16:24:46.6855213Z tomb [D] Mapper: tomb.test.ccffcb34977c2f4aad5a9405f6b9c77d909cf6ca53d9c8783bb6985a36f9b78a.loop3
2024-09-01T16:24:46.6872752Z tomb  .  Slamming tomb [test] mounted on /tmp/tomb/testmount
2024-09-01T16:24:46.6888858Z tomb [D] Trying to determine if there are open processes in [test]
2024-09-01T16:24:46.6937667Z tomb  .  Looking for processes running inside tomb [test]...
2024-09-01T16:24:46.6955469Z tomb [D] scanning tomb: /dev/mapper/tomb.test.ccffcb34977c2f4aad5a9405f6b9c77d909cf6ca53d9c8783bb6985a36f9b78a.loop3;/tmp/tomb/testmount;ext4;(rw,nodev,noatime);[test]
2024-09-01T16:24:46.6990644Z tomb [D] Super user execution skipped (SUID caller)
2024-09-01T16:24:46.7691284Z COMMAND    PID USER   FD   TYPE DEVICE SIZE/OFF NODE NAME
2024-09-01T16:24:46.7692385Z close_blo 5705 root    7w   REG  252,0        0   16 /tmp/tomb/testmount/occupied
2024-09-01T16:24:46.7709298Z tomb [D] Found open processes in [test]
2024-09-01T16:24:46.7749129Z tomb [D] Super user execution skipped (SUID caller)
2024-09-01T16:24:46.8304483Z tomb  .  [test] sending TERM to open process 5705
2024-09-01T16:24:46.8336336Z tomb [D] Super user execution skipped (SUID caller)
2024-09-01T16:24:47.3392776Z tomb [D] Super user execution skipped (SUID caller)
2024-09-01T16:24:47.4136784Z tomb [D] Super user execution skipped (SUID caller)
2024-09-01T16:24:47.4680267Z tomb  .  Closing tomb [test] mounted on /tmp/tomb/testmount
2024-09-01T16:24:47.4725157Z tomb [D] Performing umount of /tmp/tomb/testmount
2024-09-01T16:24:47.4767172Z tomb [D] Super user execution skipped (SUID caller)
2024-09-01T16:24:47.4843764Z tomb [D] Super user execution skipped (SUID caller)
2024-09-01T16:24:47.5059822Z tomb (*) Tomb [test] closed: your bones will rest in peace.
2024-09-01T16:24:47.5176764Z loop device usage change to 3
2024-09-01T16:24:47.5177239Z      Tomb command returns 0
2024-09-01T16:24:47.5178938Z ok 5 - Testing functionality of the slam operation (use of lsof)

[Narrat]

The additional force push because I forgot to remove a temporary output

[jaromil]

many thanks! looks good even with the small hack on tombname, still less invasive and feasible for now. I agree we could look into making the [] disappear, I don't remember anymore why those chars slipped in, most likely because it was easier to move forward.