Bug 232222 - rpm-4.4.2-debugpaths.patch broken
rpm-4.4.2-debugpaths.patch broken
Status: CLOSED RAWHIDE
Product: Fedora
Classification: Fedora
Component: rpm (Show other bugs)
6
All Linux
medium Severity medium
: ---
: ---
Assigned To: Paul Nasrat
: Reopened
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2007-03-14 10:03 EDT by Jakub Jelinek
Modified: 2007-11-30 17:11 EST (History)
1 user (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2007-04-23 06:57:21 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:


Attachments (Terms of Use)
rpm-4.4.2-debugedit-canonicalize-path.patch (1.37 KB, patch)
2007-03-14 10:47 EDT, Jakub Jelinek
no flags Details | Diff
rpm-debugedit-incremental-fix.patch (777 bytes, patch)
2007-04-20 17:12 EDT, Jakub Jelinek
no flags Details | Diff

  None (edit)
Description Jakub Jelinek 2007-03-14 10:03:54 EDT
debugedit with rpm-4.4.2-debugpaths.patch breaks libgcj.so.8rh.0.0.debug.
Self-contained testcase:

#!/bin/sh
rm -rf /tmp/debugedit-test
mkdir -p /tmp/debugedit-test/{a/b/c,d/e/f}
cd /tmp/debugedit-test/a/b/c
echo 'int main (void) { return 0; }' > ../../../d/e/f/a.c
gcc -g ../../../d/e/f/a.c -o a -O2
cd ../../../
readelf -wi -wl a/b/c/a | sed -n '/DW_AT_\(name.*\.c\|comp_dir\)/p;/^ The \(Directory\|File Name\) Table:/,/^$/p'
cp a/b/c/a{,.orig}
/usr/lib/rpm/debugedit -b /tmp/debugedit-test/ -d /usr/src/debug/ -l a/b/c/a.list a/b/c/a
readelf -wi -wl a/b/c/a | sed -n '/DW_AT_\(name.*\.c\|comp_dir\)/p;/^ The \(Directory\|File Name\) Table:/,/^$/p'

As you can see, it messes up the directory table entries, ../../../d/e/f
is changed into ../d/e/f and thus after relocation it obviously can't be found,
/usr/src/debug/a/b/c/../d/e/f/a.c is not where the file was stored -
/usr/src/debug/d/e/f/a.c aka /usr/src/debug/a/b/c/../../../d/e/f/a.c.
Comment 2 Jakub Jelinek 2007-03-14 10:47:03 EDT
Created attachment 150040 [details]
rpm-4.4.2-debugedit-canonicalize-path.patch

It seems canonicalize_path doesn't handle ../../ right (and messes other things
as well).
Try:
int
main (void)
{
  char buf[64];
  canonicalize_path("/a/b/././//c/../../d/e/", buf);
  __builtin_puts (buf);
  canonicalize_path("/a/b/././//c/..////../////d/e/", buf);
  __builtin_puts (buf);
  canonicalize_path("../a/b", buf);
  __builtin_puts (buf);
  canonicalize_path("../../a/b", buf);
  __builtin_puts (buf);
  canonicalize_path("../../../a/b", buf);
  __builtin_puts (buf);
  canonicalize_path("..///..///..///a/b", buf);
  __builtin_puts (buf);
  return 0;
}

with canonicalize_path from debugedit.c, it will print:
/a/d/e
/a//d/e
../a/b
a/b
../a/b
/..//a/b

Note, ../../ was killed altogether, ../../../ shrunk into ../ and there are
several cases where multiple //'s are left.
With this patch it prints
/a/d/e
/a/d/e
../a/b
../../a/b
../../../a/b
../../../a/b
Comment 3 Jeff Johnson 2007-03-15 10:22:32 EDT
Fixed in rpm cvs, will be in rpm-4.4.9-0.2 when built.
Comment 4 Paul Nasrat 2007-03-15 11:53:59 EDT
Thanks for the testcase (added to RHTS) and the patch.  Built for rawhide.

Do we need this for fc6 and RHEL5?  If so can you clone appropriately
Comment 5 Jakub Jelinek 2007-04-20 16:31:38 EDT
Unfortunately this patch causes regressions, e.g. glibc doesn't build any longer.
In particular, canonicalize_path on say "./netinet" canonicalizes it to
"/netinet" rather than "netinet".  Working on a fix.
Comment 6 Jakub Jelinek 2007-04-20 17:12:09 EDT
Created attachment 153216 [details]
rpm-debugedit-incremental-fix.patch

Incremental patch to fix this bug.
Comment 8 Jeff Johnson 2007-04-20 19:48:36 EDT
Fixed in rpm cvs. Thanks for the patch.

(aside) I still think that path canonicalization on strings needs to be in glibc.
I personally have boogered up path canonicalization several times, clearly you have too ;-)
Comment 9 Jakub Jelinek 2007-04-21 02:58:21 EDT
Path canonicalization is in glibc, see realpath(3) and canonicalize_file_name(3).
But that doesn't simplify just ., .. and multiple //s, but also symlinks, so it
is not useful for debugedit purposes.
Comment 10 Jeff Johnson 2007-04-21 23:34:40 EDT
I knew realpath(3), whose behavior is dependent on the path existing on the file system
because of getcwd and stat.

A quick look at canonicalize_file_name indicates and alternative entry to __realpath, and
so has the same behavior. Thanks for the ptr.

A pure string canonicalization on path would be useful imho.

Note You need to log in before you can comment on or make changes to this bug.