Description of problem: If there is a stuck NFS mount, and there are processes running from that mount point pidof utility may block forever. Since this utility is used in most stop scripts, machine becomes unbootable. This is expected to exist on all versions of Red Hat. (Tested 7.3, AS 2.1, AS 30, update 3). Steps to Reproduce: 1. mount a NFS partition (can be from a local machine) with default options. 2. put a sample prog on the mounted partition (the simples program will suffice). 3. start the prog. 4. stop nfs on the nfs server. 5. run `pidof xxx` or pidof `some_real_name`, provided some_real_name comes before your test prog in the /proc list. 6. Try `init 6` on the machine. Actual results: Since reboot uses pidof a lot, and since pidof scans the whole proc tree, and since pidof blocks forever scanning the /proc entry for a process which was run from NFS, the whole reboot blocks and the box is stuck. Expected results: Machine should be able to reboot, even if there is a stuck NFS partition. Additional info: The problem is in a brain-damaged code in killall5.c that tries to call stat() on /proc/pid/exe file, that will block for ever on a stuck NFS. I provide a patch that uses readlink() instead. It also fixes several memory leaks for races against exited processes.
Created attachment 106475 [details] use readlink instead of stat to solve blcking on NFS Also fixes some memory leaks when cleaning up for processes that exited during their proc scan.
In steps to reporduce, the step 5 should read: 5. run `pidof xxx` or pidof `some_real_name`, provided some_real_name comes AFTER your test prog in the /proc list. This when a real problem shows up -- when you try to look up the pid of the process that is listed in the proc tree after you.
Created attachment 106478 [details] instead of stat to solve blocking on NFS for AS3 SysVinit-2.85-4 Also fixes some memory leaks when cleaning up for processes that exited during their proc scan. Must be used as the last patch.
The first patch is for SysVinit-2.84.
The second patch obsoletes the first, correct?
The first patch is against version 2.84 of SysVinit and was tested on RedHat 7.3 platform. This is probably what will have to be used for advanced server 2.1. The second patch is against SysVinit-2.85-4 from RedHat advanced server 3.0 update 3. So, the second patch does not obsolete the first one, unless a new version of the package (2.85) will be provided for both AS 2.1 and AS 3.0 to fix the problem.
I'd be curious how this would work with namespaces and/or chroots(), when the same 'path' would be different.
When you perform pidof /sbin/my_prog or killall /sbin/my_prog, would you not expect it to take into account all processes that run on the machine, even if they run in the chrooted environment? Is it not perfectly logical? So, the new code is more consitent than the old, because with the old code you would have to do killall /path_to_chroot/sbin/myprog to get the chrooted progs. I doubt that this is sane for killall or compatible with other unixes for killall behavior. Also, even the unpatched code began to do just command name comparisons if it could not find any match based on the path inode and device info. So, with the old code, you would not get any chrooted programs when there were progs running from /. To get the chrooted stuff, you would have to call with /path_to_chroot/sbin/myprog as an argument. However, if there were no instances of /sbin/myprog running form /, you would get chrooted ones by simply using /sbin/myprog as an argument. Pretty inconsistent, I think! What do you think?
I think my previous comment is wrong, even though it would be nice to get the behavior I describe as desirable. With my changes, the code will behave exactly as before. Since /proc/pid/exe will show the symlink to full path of the executable (with chroot constraints decribed below), the readlink will return the full path of the executable. So, with the patch, the file locations comparisons will yield the same results as with the unpatched versions of the software. Even for the chrooted environments, if one starts a process from chroot, and does killall from chroot, then within the chroot, the exe link in proc will not contain the path to chroot. So, running the unitlity is OK from chroot and behaves exactly as with unpatched version of the utility. However, when running the utility from /, the comparison by file location will fail, since exe link will have the chroot_path, and only the comparsion by prog name may succeed (depending what is the command name that we get from proc). Thus, we get the same behavior as with unpatched version. For processes that are run from /, and we try to "get them" from chroot, the situation is reversed -- comparison by file location will succeed from /, and only comparison by name may succeed from chroot (since chrooted proc will have exe entries that will point outside of chroot, so we will get unresolved links). Again exactly the same behavior as with unpatched software. Do you concur?
It does not behave *exactly* as before. pidof /some/file/that/is/a/symlink will fail, as it relies on a dev/inode match to work. An example of this currently in use is the postmaster symlink in postgresql.
OK. Thank you for testing the patch. I have several comments: 1) In general, idea of pidof/killproc searching by filename (file location) and not by command name is bizzare to me. I always thought it searched by command name until I looked at the code. What it really does, it tries to take the "program" argument as file location and match the executables of running processes to that location. Only if this procedure fails, it will compare the "short" command line of running processes with the "program" argument passed to pidof/killproc. Such policy seems really strange to me, especially since most init scripts use it by passing the basename of the executable, and not its full path. However, at this point I am not saying the utility must be changed. 2) When I said "exactly as before", I addressed concerns of running pidof either form / or from chroot on the processes that have been started either from / or chroot. I still claim it will produce the same results as without the patch, provided the "program" argument passed to pidof is not a symlink. 3) You are correct that I have not thought about "program" arguments in the form of "/some_path/symlink". (My first item may explain why.) However, this can be very easily fixed. The fix is like this: Call readlink() on the "program" argument itself. If the argument is a valid symbolic link, then use the name returned by readlink() in further processing instead of the "program" argument. We also will have to use realpath() to further process the "program" name to resolve other symbolic links or relative paths. After we have the full path to the "program" argument, we can continue matching as before. (We need the "real" full path because exe symlink in /proc/pid/exe points to "real" full path of the executable). I can redo the patch if you wish or you can do it yourself. If realpath() blocks on the "stuck" NFS system, we may need to use an unblocking variant of it in order to allow calls like "pidof /stucked_nfs/bin/my_prog". BTW, AFAIK, such calls will still block even with my patch, since I have not removed the stat() call on the "program" argument. This still may need to be fixed. The main problem that I was trying to solve is "pidof sendmail" (or some other service name) blocks causing reboot to block because there is a process running on a NFS stuck partition.
Created attachment 108533 [details] Updated patch for SysVinit-2.85-4 to support symlinks as prog args This patch solves a problem with the previous patch -- symbolic links as "program" arguments work now as expected.
Created attachment 108534 [details] Updated patch for SysVinit-2.85-4 to support symlinks as prog args This patch solves a problem with the previous patch -- symbolic links as "program" arguments work now as expected. Initial replacement used the "old" patch. Sorry.
Created attachment 108535 [details] Updated patch for SysVinit-2.84 to support symlinks as prog args This patch solves a problem with the previous patch -- symbolic links as "program" arguments work now as expected.
With latest variants of the patches, symlink "program" argument work as expected. The change is trivial -- we call realpath() on the program argument instead of stat. The full name returned by realpath() is then used in comparisons against the proc entries. In the original code, stat call was used to decide whether the file was present and to get device and inode numbers. In the first version of the patch, the stat call was left just to decide whether the file is present. In this version of the patch, stat is replaced with realpath() -- we both get the full file name and the error code if the file is not present. Like stat(), realpath() blocks on stuck NFS since realpath() calls stat on each subdirectory entry. So, if there is a requirement to be able to call 'pidof /stuck_nfs/my_prog' without blocking, a non-blocking realpath() variant must be written. At lest with these patches, the system is able to reboot if there are processes running from a stuck nfs mount. Do you think ability to run 'pidof /stuck_nfs/my_prog' without blocking is important? I have realpath() like code that is well debugged. It does not call readlink() but it strips all dots and slashes from the path string correctly and it can be reused to write a non-blocking variant of realpath().
The use of readlink effectively breaks it for non-root usage entirely.
Created attachment 113650 [details] new patch This patch (against the devel tree) fixes the previous problem.
FWIW, this is fixed in 2.85-38 in the devel tree.
An advisory has been issued which should help the problem described in this bug report. This report is therefore being closed with a resolution of ERRATA. For more information on the solution and/or where to find the updated files, please follow the link below. You may reopen this bug report if the solution does not work for you. http://rhn.redhat.com/errata/RHBA-2005-510.html
An advisory has been issued which should help the problem described in this bug report. This report is therefore being closed with a resolution of ERRATA. For more information on the solution and/or where to find the updated files, please follow the link below. You may reopen this bug report if the solution does not work for you. http://rhn.redhat.com/errata/RHBA-2005-511.html