How implementation details become ABI: a case study [LWN.net]

How implementation details become ABI: a case study

Posted Oct 5, 2014 3:14 UTC (Sun) by viro (subscriber, #7872)
In reply to: How implementation details become ABI: a case study by magcius
Parent article: How implementation details become ABI: a case study

Yes, it would (not that they were in any real danger of stepping into that one - seriously, 32-character filename for a daemon binary? And that would not include the pathname - just the last component that long).

But yes, that's one of the reasons why their use of that trick had been bogus.

to post comments

How implementation details become ABI: a case study

Posted Oct 5, 2014 3:26 UTC (Sun) by Cyberax (✭ supporter ✭, #52523) [Link] (4 responses)

One of our clients used to have a "paracetomoxyfrysebendroneomycind" daemon. Doing some drug discovery calculations, appropriately.

How implementation details become ABI: a case study

Posted Oct 5, 2014 4:47 UTC (Sun) by viro (subscriber, #7872) [Link] (2 responses)

*snort*

OK, I stand corrected - their script definitely breaks on that one... for 64bit host. Anyway, I think it's bloody obvious that what they are doing is an awful kludge.

Note that keeping the daemon running through the update is a very dubious approach; sure, the binary itself will stick around until we exit, but the rest of files in the package will get replaced, so if that daemon ever rereads them (e.g. in response to incoming request of some sort, or being asked to stop), the old binary will find new config/data/helper binaries/whatnot, with potentially spectacular results. Much safer to stop the sucker before replacing any files and restart it afterwards. And anything that does something more fancy (e.g. re-exec itself and transfer the internal state across that in one way or another) has no business using start-stop-daemon anyway.

IOW, the entire "replace files, then stop the old processes" is a bad idea. And prior to replacements there's no need whatsoever for that kind of kludges. Mind you, I'd rather do stat() on the binary we are after, then looked for /proc/*/exe with stat() giving the matching st_dev/st_ino. Without bothering with readlink() on those guys. Has an extra benefit of doing the right thing when you have multiple links to the same binary, with different processes using different names...

What they were doing is awful for a lot of reasons; sadly, that's not the criterion used in such situation ;-/ It worked for a long time, there's real-world userland code relying on it, so we get to keep it working. It's not quite the same as bug-for-bug compatibility - if nothing breaks when we fix inconsistent behaviour, we can go for it even if it was possible to write something that would break. Ditto if the code being broken is a rootkit or rootkit equivalent (i.e. relies on exploiting a security hole, by accident or not). But "the code we broke would've broken in a lot of other cases anyway" isn't an acceptable excuse.

How implementation details become ABI: a case study

Posted Oct 5, 2014 11:10 UTC (Sun) by JGR (subscriber, #93631) [Link] (1 responses)

Presumably a daemon designed to be upgraded whilst running in this way would load configuration and other files at startup or on HUP specifically to avoid these issues.

Stopping the daemon, upgrading, then restarting it introduces a small period of downtime, which can be undesirable.

How implementation details become ABI: a case study

Posted Oct 5, 2014 16:08 UTC (Sun) by viro (subscriber, #7872) [Link]

As I said, something more fancy shouldn't be using start-stop-daemon in the first place. I really wonder how many common daemons take care about the races around the upgrade; stop/move new one in place/start is more robust...
It's not just config; anything from helper binaries to permissions on directories, etc. can become a surprise for old daemon binary.

How implementation details become ABI: a case study

Posted Oct 12, 2014 8:11 UTC (Sun) by jzbiciak (guest, #5246) [Link]

Ok, I have nothing witty or insightful to add. But I have to say that's the most awesome executable name I've seen in a long time. Bravo!

How implementation details become ABI: a case study

Posted Oct 5, 2014 3:46 UTC (Sun) by magcius (guest, #85280) [Link] (1 responses)

Ah, from the "/tmp/foo (deleted)" string, I thought it was the full file path stored.

How implementation details become ABI: a case study

Posted Oct 5, 2014 5:05 UTC (Sun) by viro (subscriber, #7872) [Link]

Nope - in that case there are 3 or 4 dentries involved (depending on whether /tmp is a mountpoint). There's root directory of root filesystem. Name's empty, parent - itself. There's /tmp on root filesystem. Name being "tmp", parent - root of root filesystem. There's (possibly) root directory of whatever is mounted on /tmp. Again, name is empty, parent - itself. And there's /tmp/foo - name is "foo", parent - /tmp on root fs or root of filesystem mounted on /tmp. And it's unhashed since that rename.

We start with that dentry and vfsmount of either root fs or that mounted on /tmp and walk towards root. When we reach the root of current vfsmount we just proceed to dentry and vfsmount of mountpoint and continue from there. When we reach the process' root (sensu chroot(2)) or global root (vfsmount that isn't mounted on anything) we stop.

In this case the names we see along the way are "foo" and "tmp", and or dentry is unhashed, so d_path() produces "/tmp/foo (deleted)". And that's what readlink() on these guys returns. See fs/dcache.c:d_path() and the stuff next to it.

The name of dentry is just a single pathname component. That "/tmp/foo (deleted)" isn't stored anywhere - it's calculated on demand.