This is the mail archive of the elfutils-devel@sourceware.org mailing list for the elfutils project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [patch] Support live PIDs with deleted files


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  <jan.kratochvil@redhat.com>
> > 
> > 	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


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]