On Wed, 2013-12-18 at 20:13 +0100, Jan Kratochvil wrote: > jankratochvil/deleted
Cute name :) > with the new /proc/PID/map_files/ feature it is possible to fully access even > deleted executable + shared_libraries. Current Linux kernels (3.11.10) permit > on root user as the tracer but (1) hopefully it gets fixed in the future and > (2) at least for the use in ABRT it is OK as backtracer is run from core > dumping hook. Do you have a reference to do documentation of /proc/PID/map_files? It seems simple enough, a directory with files named after the /proc/PID/maps ranges symlinked to the actual file/inode (you don't seem to get the file offset, so you have to get that from /proc/PID/maps I assume, or in case of ELF files match their segment load addresses). But it would be good to see the "official" documentation if there is any to make sure we are using the kernel interface/contract as intended. > Linux kernels have always supported also /proc/PID/exe which works for deleted > main executable. It even works for non-root users. But it works only for the > executable, not for shared libraries. I find more useful to push for Linux > kernel /proc/PID/map_files/ permissions fix than to implement /proc/PID/exe. Did you discuss this with the kernel hackers? I don't think /proc/PID/map_files is like /proc/PID/exe. It is more like /proc/PID/mem. Because with /proc/PID/exe the kernel can be sure the user had access to that file on disk at least when the program started, but /proc/PID/map_files contains mappings that might only have temporarily existed as files (maybe a mapped tmp file that was explicitly removed to only have an in memory copy). That doesn't mean we shouldn't try to get access to /proc/PID/map_files, it looks really, really useful. But it might be harder than just a simple directory permission fix, it might need at least the permission characteristics that /proc/PID/mem has. Or it might have to distinguish those on-disk files mapped executable from those that aren't. I think it is useful to implement this for /proc/PID/exe first and if that works extend it to /proc/PID/map_files if the permission and security concerns can be addressed by the kernel people. /proc/PID/exe seems to be available everywhere, while /proc/PID/map_files looks very recent. > If the running binary has been upgraded while it is still running its separate > debug info files in /usr/lib/debug/ will no longer match. But still > .eh_frame-based unwind is possible using /proc/PID/map_files/ that time. > > Testcase is skipped when /proc/PID/map_files/ gives permission error. > > OK for check-in? I am slightly concerned by the fact that gdb doesn't seem to use this mechanism yet. We should at least coordinate usage of this kernel feature with them. Ideally gdb experiments a bit with it and when they have found all the quirks we can use it properly :) And we should first use the more common and more usable /proc/PID/exe approach even if that only gives us part of the benefits. That would at least make testing the feature much easier. It is slightly more work since you don't get a range with it, but it should be the first mapping in /proc/PID/maps and/or you could match on the /proc/PID/comm or /proc/PID/stat exe command field. > I am not completely sure setting just mod->main.name is right but it looks so. I think this works because main.name is used as fallback when no other filename is available. But it would be good to know for sure. > > libdwfl/ > > 2013-12-18 Jan Kratochvil <[email protected]> > > > > Support live PIDs with deleted files. > > * linux-proc-maps.c (PROCMAPFILESFMT): New #defined. > > (proc_maps_report): New comment for low and high. New variable > > high_start, set it everywhere. > > (proc_maps_report) (map_files_name): New inlined function. > > (proc_maps_report) (report): Call it and set mod->main.NAME. So this works by reading the /proc/PID/maps file as normal. Accumulating all ranges of a file (low-start). And keeping track of the last mapped range (high_start-high). When you got a full mapping then you call Then after you called dwfl_report_module to create the Module and then you see whether you can find the /proc/PID/map_files/ symlink for the last range and use that to prime mod->main.NAME. Why not first lookup the symlink for that last mapped range and use that for the call to dwfl_report_module if it exists? And is there any reason to pick the last range? I assume it could have been any range for the file. > > diff --git a/libdwfl/linux-proc-maps.c b/libdwfl/linux-proc-maps.c > > index 8863cc8..b9604d0 100644 > > --- a/libdwfl/linux-proc-maps.c > > +++ b/libdwfl/linux-proc-maps.c > > @@ -43,6 +43,7 @@ > > > > > > #define PROCMAPSFMT "/proc/%d/maps" > > +#define PROCMAPFILESFMT "/proc/%d/map_files/%" PRIx64 "-%" PRIx64 > > #define PROCMEMFMT "/proc/%d/mem" > > #define PROCAUXVFMT "/proc/%d/auxv" > > #define PROCEXEFMT "/proc/%d/exe" > > @@ -176,7 +177,26 @@ proc_maps_report (Dwfl *dwfl, FILE *f, GElf_Addr > > sysinfo_ehdr, pid_t pid) > > unsigned int last_dmajor = -1, last_dminor = -1; > > uint64_t last_ino = -1; > > char *last_file = NULL; > > - Dwarf_Addr low = 0, high = 0; > > + // LOW-HIGH is cumulative range for all lines of the current filename. > > + // HIGH_START-HIGH is range of only the very last PROCMAPSFMT line. > > + Dwarf_Addr low = 0, high_start = 0, high = 0; > > + > > + inline char *map_files_name (void) > > + { > > + static int map_files_failed = -1; > > + if (map_files_failed > 0) > > + return NULL; > > + char *map_fname; > > + if (asprintf (&map_fname, PROCMAPFILESFMT, pid, high_start, high) < > > 0) > > + return NULL; > > + if (map_files_failed == 0) > > + return map_fname; > > + map_files_failed = access (map_fname, R_OK) != 0; This seems too drastic. Why not do a check when map_files_name is called first to see if the /proc/PID/map_files/ directory exists, can be read and isn't empty? > > + if (! map_files_failed) > > + return map_fname; > > + free (map_fname); > > + return NULL; > > + } > > > > inline bool report (void) > > { > > @@ -188,6 +208,9 @@ proc_maps_report (Dwfl *dwfl, FILE *f, GElf_Addr > > sysinfo_ehdr, pid_t pid) > > last_file = NULL; > > if (unlikely (mod == NULL)) > > return true; > > + char *map_fname = map_files_name (); > > + if (map_fname != NULL) > > + mod->main.name = map_fname; Yeah, so this is a little mysterious. As said before I would have done this lookup first and then used it for the dwfl_report_module call. Cheers, Mark
