Re: [Acl-devel] libattr 2.4.48 change in syscalls.c breaks fakechroot

[Dmitry V. Levin]

On Wed, Aug 15, 2018 at 11:09:26PM +0200, Andreas Grünbacher wrote:
> Am Mi., 15. Aug. 2018 um 22:16 Uhr schrieb Dmitry V. Levin <address@hidden>:
> > On Wed, Aug 15, 2018 at 09:50:14PM +0200, Andreas Grünbacher wrote:
> > > Am Mi., 15. Aug. 2018 um 18:36 Uhr schrieb Filipe Brandenburger 
> > > <address@hidden>:
> > > > On Tue, Aug 14, 2018 at 11:00 AM Dmitry V. Levin <address@hidden> wrote:
> > > > > On Mon, Aug 13, 2018 at 10:12:57PM -0700, Filipe Brandenburger wrote:
> > > > > > See here for more details:
> > > > > > https://github.com/dex4er/fakechroot/issues/57
> > > > >
> > > > > I suppose fakechroot has to be fixed to remain useful.
> > > >
> > > > I'm proposing using explicit symbol versions (dlvsym) in fakechroot to 
> > > > fix this:
> > > > https://github.com/dex4er/fakechroot/pull/58
> > > >
> > > > > > Any chance you'd consider going back to an implementation directly
> > > > > > using SYSCALL(...) in libattr?
> > > > >
> > > > > I believe commit v2.4.48~17 was the right change in this respect.
> > > > > Note that software has been using glibc functions to access acl 
> > > > > syscalls
> > > > > for quite a long time, so changing libacl won't help fakechroot to 
> > > > > handle
> > > > > that software.
> > > >
> > > > I somewhat agree, but the way v2.4.48~11 reintroduces them is not
> > > > ideal, since it interacts with LD_PRELOAD and dlsym() in unexpected
> > > > ways. (Again, I'm not saying libattr is at fault here, just that
> > > > LD_PRELOAD and dlsym is a somewhat incomplete API and leads to this
> > > > kind of issues.)
> > > >
> > > > In a perfect world (or, at least, slightly better than this own),
> > > > instead of libattr exporting wrappers (like libattr_lsetxattr) which
> > > > call the glibc function (lsetxattr), I think it would be good if they
> > > > could import the glibc symbol (address@hidden) and re-export that
> > > > (as address@hidden, non-default symbol.)
> > > >
> > > > I don't really know if it's even possible to do this with linker
> > > > scripts or, if it is, how to do that... But I think that would be an
> > > > improvement.
> > >
> > > Hmm, commit efa0b1ea (v2.4.48~11) was not such a good idea.
> > >
> > > I wonder if there's a way to leave older versions of a symbol around
> > > in a library, but to un-export it in the default version so that
> > > programs newly built against the library will skip it. I couldn't
> > > figure out a way to do that though.
> >
> > This is exactly what commit v2.4.48~11 does: it brings versioned symbols
> > back but does not allow to link with them.  As result, every program
> > or library linked with libattr >= v2.4.48~11 will automatically switch
> > to *attr symbols provided by glibc.
> 
> It does seem that fakeroot ends up calling the libattr_*xattr
> trampoline functions.

I bet the bug report was against fakechroot, but fakeroot must have
the same issue with *attr functions.

Both fakechroot and fakeroot implement the following naive algorithm:
- add their own *attr functions;
- each of these functions in turn calls dlsym(RTLD_NEXT) and obtains
  a reference to another function with the same name implemented somewhere else;
- when libattr is loaded, these references point to functions in libattr,
  otherwise they point to functions in glibc;
- when libattr >= v2.4.48~11 is loaded, these referenced functions invoke
  their sister functions from glibc;
- these invocations get intercepted by fakechroot, causing a loop.

> > > Alternatively, we could add a second libattr with a new major version.
> >
> > Just to strip these compatibility symbols?  Does it worth the trouble
> > caused by a soname change?
> 
> To fix something we've screwed up instead of forcing a second hack on
> top of our hack into fakeroot?

I don't think we've screwed up anything on the libacl side.
There was no guarantee that libacl functions will not call glibc
functions.  fakechroot used to rely on a hack, and 3 years ago
this hack stopped working.

I had patched fakeroot to use dlvsym back in 2005, this is probably
the reason why I haven't noticed any regressions 3 years ago.


-- 
ldv

signature.asc
Description: PGP signature