dyne / tomb

the Crypto Undertaker
https://dyne.org/software/tomb
GNU General Public License v3.0
1.31k stars 151 forks source link

slam command don't slam #77

Closed davinerd closed 11 years ago

davinerd commented 12 years ago

slam_tomb() should be revised: it don't slam tomb. I'm unsure about fuser syntax; also, we don't implement an -f option, but the code above mentions it.

Anyone got this issue? Please check it out.

boyska commented 12 years ago

Works for me. But you're right, the "-f" option is not recognized by slam subcommand, we should add it. I tried with vim being the bad process, and it exited on USR1. then I tried with dd, which does NOT exit on USR1 (instead, it displays stats), and it exited on HUP (after 3 seconds)

davinerd commented 12 years ago

On 13/01/2012 21:55, BoySka wrote:

Works for me. But you're right, the "-f" option is not recognized by slam subcommand, we should add it. I tried with vim being the bad process, and it exited on USR1. then I tried with dd, which does NOT exit on USR1 (instead, it displays stats), and it exited on HUP (after 3 seconds) Yeah, I'had to be more specific. The tomb won't slam if it isn't busy. I mean: open a tomb, and immediatly slam the tomb, without holding it with processes.

The close command works, slam don't.

Anathema

+--------------------------------------------------------------------+ GPG/PGP KeyID: 0F26965C available on http://pgpkeys.mit.edu:11371/ Fingerprint: F808 18A2 2E7D 6E7A 7A18 4062 0AA3 7BF2 0F26 965C
http://www.msack.org
https://tboxes.tracciabi.li/anathema

+--------------------------------------------------------------------+

boyska commented 12 years ago

again, works for me.

Password: [_] Commanded to open tomb testing.tomb
 .  mountpoint not specified, using default: /media/testing.tomb
[_] mounting testing.tomb on mountpoint /media/testing.tomb
 .  check for a valid LUKS encrypted device
[_] Password is required for key testing
 .  encrypted storage filesystem check
fsck da util-linux 2.20.1
testing: clean, 13/4608 files, 1768/18432 blocks
[_] encrypted storage testing.tomb succesfully mounted on /media/testing.tomb
davide ~/Desktop % tomb slam
[_] Slamming tomb testing mounted on /media/testing.tomb
 .  Kill all processes busy inside the tomb
[_] Tomb testing closed: your bones will rest in peace.
davide ~/Desktop % tomb list
[!] I can't see any open tomb, may they all rest in peace.
davinerd commented 12 years ago

Not working for me, either with an holding proc. I'm doing some debug, and I would to ask somethings:

slam_tomb() {

$1 = tomb mount point

for s in INT TERM HUP KILL; do
fuser -s -m "$1" || return 0
fuser -s -m "$1" -k -M -$s && { option_is_set -f || sleep 3 }
done
return 1

}

To successfully slam the tomb, that function must return 0. Ok. Than:

fuser -s -m "$1" || return 0

this should be true, but fuser -s -m "$1" always return 0. Than that code will never be executed.

Another point: if the for loop kills my pids on KILL (last for loop run), then the last command executed will be

fuser -s -m "$1" -k -M -$s && { option_is_set -f || sleep 3 }

the for loop returns and then

return 1

will be executed, meaning the failure. I don't know how this can be works on your distro...I'm on gentoo.

Anathema

+--------------------------------------------------------------------+ GPG/PGP KeyID: 0F26965C available on http://pgpkeys.mit.edu:11371/ Fingerprint: F808 18A2 2E7D 6E7A 7A18 4062 0AA3 7BF2 0F26 965C
http://www.msack.org
https://tboxes.tracciabi.li/anathema

+--------------------------------------------------------------------+

boyska commented 12 years ago

You seem to be right. Indeed, fuser -s -m $1 always returns 0. BUT fuser -m $1 &> /dev/null has the behaviour we need. Try if this change make it work fine.

jaromil commented 12 years ago

nope, it still returns 0

I've done some tries, but fuser seems to me a non-well formed commandline command.

why we changed to it? not for reliability, just for it being elegant?

I vote for reliability and revert to lsof parsing unless someone comes with a reliable use of fuser. I couldn't.

jaromil commented 12 years ago

just for reference, incriminated commit: https://github.com/dyne/Tomb/commit/d53c028f6b

davinerd commented 12 years ago

On 16/01/2012 11:08, BoySka wrote:

You seem to be right. Indeed, fuser -s -m $1 always returns 0. BUT fuser -m $1 &> /dev/null has the behaviour we need. Try if this change make it work fine. This works for me too. Changing the two lines makes slam_tomb() works as expected.

But we need to test the

fuser -m "$1" -k -M -$s &>/dev/null && { option_is_set -f || sleep 3 }

that needs the -f flag. Do we need it?

Anathema

+--------------------------------------------------------------------+ GPG/PGP KeyID: 0F26965C available on http://pgpkeys.mit.edu:11371/ Fingerprint: F808 18A2 2E7D 6E7A 7A18 4062 0AA3 7BF2 0F26 965C
http://www.msack.org
https://tboxes.tracciabi.li/anathema

+--------------------------------------------------------------------+

jaromil commented 12 years ago

no the -f flag is not needed. actually I'm wondering now if we need the 'slam' command at all. we can start activating slam also when 'close -f' is called.

davinerd commented 12 years ago

On 21/01/2012 16:09, Jaromil wrote:

no the -f flag is not needed. actually I'm wondering now if we need the 'slam' command at all. we can start activating slam also when 'close -f' is called. yeah: we can recycle slam_tomb(), calling it when using 'close -f'. BUT: the -f flag? Why we havn't it yet?

It seems to be a great thing, maybe 'limited' for now to 'close' and some others.

Anathema

+--------------------------------------------------------------------+ GPG/PGP KeyID: 0F26965C available on http://pgpkeys.mit.edu:11371/ Fingerprint: F808 18A2 2E7D 6E7A 7A18 4062 0AA3 7BF2 0F26 965C
http://www.msack.org
https://tboxes.tracciabi.li/anathema

+--------------------------------------------------------------------+

boyska commented 12 years ago

mh, there is a problem. Atm, slam use the -f flag to control whether or not to sleep between the kills. We need to find another name, for this, like: tomb close #normal tomb close -f # same as tomb slam tomb close -f --brutal # same as tomb slam -f

davinerd commented 12 years ago

On 22/01/2012 13:33, BoySka wrote:

mh, there is a problem. Atm, slam use the -f flag to control whether or not to sleep between the kills. We need to find another name, for this, like: tomb close #normal tomb close -f # same as tomb slam tomb close -f --brutal # same as tomb slam -f Maybe

tomb close --brutal # without -f

can be very nice. Who do this?

I'm busy working on tomb filesystem choose, and I can be ready for that after 3-4 days.

Anathema

+--------------------------------------------------------------------+ GPG/PGP KeyID: 0F26965C available on http://pgpkeys.mit.edu:11371/ Fingerprint: F808 18A2 2E7D 6E7A 7A18 4062 0AA3 7BF2 0F26 965C
http://www.msack.org
https://tboxes.tracciabi.li/anathema

+--------------------------------------------------------------------+

jaromil commented 12 years ago

mmm, wait a sec. we need to have LESS flags, not more. they complicate things, give users more documentation to read and often introduce ambiguity. I personally dream of a world without flags! (voluntary double-sense)

said that, there is no problem because:

close -f -> slam (wait) slam -f -> slam (nowait)

davinerd commented 12 years ago

On 23/01/2012 10:48, Jaromil wrote:

mmm, wait a sec. we need to have LESS flags, not more. they complicate things, give users more documentation to read and often introduce ambiguity. I personally dream of a world without flags! (voluntary double-sense)

said that, there is no problem because:

close -f -> slam (wait) slam -f -> slam (nowait)

Well, we can do: close -f -> slam (wait) slam -> slam (nowait)

agreed?

Anathema

+--------------------------------------------------------------------+ GPG/PGP KeyID: 0F26965C available on http://pgpkeys.mit.edu:11371/ Fingerprint: F808 18A2 2E7D 6E7A 7A18 4062 0AA3 7BF2 0F26 965C
http://www.msack.org
https://tboxes.tracciabi.li/anathema

+--------------------------------------------------------------------+

boyska commented 12 years ago

davinerd,jaromil: is fuser -m $1 &> /dev/null a valid solution for the problem? shall we commit it? please test it

jaromil commented 12 years ago

I've tested it again and same result: it does return zero all the time (I've tested on common Ubuntu and Debian systems)

We need something else there to return non-zero if the directory is not being used....

boyska commented 12 years ago

uff. we could do something like fuser|wc -w (that checks if there is any result), and checking it being zero/non-zero. It do what expected, here, and it should work flawlessy everywhere. Can you test it, please? I want to get rid of this issue quickly.

jaromil commented 12 years ago

yes we need to solve this issue quick.

on the return code I get a different behaviour on debian and ubuntu (!?)

instead, your approach seems to work. i'm trying only on debian now assuming '/' is in use and '/mnt' is not in use on my comp now, I tried

% caz=fuser /mnt 2> /dev/null ; { test "$caz" = "" } && { echo vuoto } vuoto % caz=fuser / 2> /dev/null ; { test "$caz" = "" } && { echo vuoto } (return 1)

seems to be viable

boyska commented 12 years ago

your simplification is fine: so the code is

if [[ -n `fuser -m /mount/point 2> /dev/null` ]]; then #there are processes
boyska commented 12 years ago

I tried to fix it with this new commit. Hope it works, please everyone test it.

tomb-git package for archlinux has been updated. arch testers, no excuses for you :)

jaromil commented 11 years ago

It works for me, I've been using it for a year now. Thanks :^)