* strip looses original file ownership and file permissions @ 1999-05-06 3:48 Franz Sirl 1999-05-06 4:45 ` Franz Sirl 0 siblings, 1 reply; 7+ messages in thread From: Franz Sirl @ 1999-05-06 3:48 UTC (permalink / raw) To: gas2 Hi, I just verified that strip out of gas-990418 looses original ownership and permissions of a file. This is on glibc-2.1.1pre2, Linux-2.2.6 (PPC). Is this platform specific or does anybody else notice this? Franz. ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: strip looses original file ownership and file permissions 1999-05-06 3:48 strip looses original file ownership and file permissions Franz Sirl @ 1999-05-06 4:45 ` Franz Sirl 1999-05-06 9:27 ` Ian Lance Taylor 0 siblings, 1 reply; 7+ messages in thread From: Franz Sirl @ 1999-05-06 4:45 UTC (permalink / raw) To: gas2 At 12:48 06.05.99 , Franz Sirl wrote: >Hi, > >I just verified that strip out of gas-990418 looses original ownership and >permissions of a file. >This is on glibc-2.1.1pre2, Linux-2.2.6 (PPC). > >Is this platform specific or does anybody else notice this? After a quick browse through the source I came up with the following untested patch. Does it look right? Franz. Index: rename.c =================================================================== RCS file: /cvs/binutils/binutils/binutils/rename.c,v retrieving revision 1.1.1.1 diff -u -p -r1.1.1.1 rename.c --- rename.c 1999/05/03 07:29:10 1.1.1.1 +++ rename.c 1999/05/06 11:36:14 @@ -163,29 +163,26 @@ smart_rename (from, to, preserve_dates) #else /* Use rename only if TO is not a symbolic link and has only one hard link. */ - if (exists < 0 || (!S_ISLNK (s.st_mode) && s.st_nlink == 1)) + if (exists == 0 && !S_ISLNK (s.st_mode) && s.st_nlink == 1) { ret = rename (from, to); if (ret == 0) { - if (exists) - { - /* Try to preserve the permission bits and ownership of - TO. First get the mode right except for the setuid - bit. Then change the ownership. Then fix the setuid - bit. We do the chmod before the chown because if the - chown succeeds, and we are a normal user, we won't be - able to do the chmod afterward. We don't bother to - fix the setuid bit first because that might introduce - a fleeting security problem, and because the chown - will clear the setuid bit anyhow. We only fix the - setuid bit if the chown succeeds, because we don't - want to introduce an unexpected setuid file owned by - the user running objcopy. */ - chmod (to, s.st_mode & 0777); - if (chown (to, s.st_uid, s.st_gid) >= 0) - chmod (to, s.st_mode & 07777); - } + /* Try to preserve the permission bits and ownership of + TO. First get the mode right except for the setuid + bit. Then change the ownership. Then fix the setuid + bit. We do the chmod before the chown because if the + chown succeeds, and we are a normal user, we won't be + able to do the chmod afterward. We don't bother to + fix the setuid bit first because that might introduce + a fleeting security problem, and because the chown + will clear the setuid bit anyhow. We only fix the + setuid bit if the chown succeeds, because we don't + want to introduce an unexpected setuid file owned by + the user running objcopy. */ + chmod (to, s.st_mode & 0777); + if (chown (to, s.st_uid, s.st_gid) >= 0) + chmod (to, s.st_mode & 07777); } else { ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: strip looses original file ownership and file permissions 1999-05-06 4:45 ` Franz Sirl @ 1999-05-06 9:27 ` Ian Lance Taylor 1999-05-06 9:54 ` Franz Sirl 0 siblings, 1 reply; 7+ messages in thread From: Ian Lance Taylor @ 1999-05-06 9:27 UTC (permalink / raw) To: Franz.Sirl-kernel; +Cc: gas2 Date: Thu, 06 May 1999 13:44:47 +0200 From: Franz Sirl <Franz.Sirl-kernel@lauterbach.com> >I just verified that strip out of gas-990418 looses original ownership and >permissions of a file. >This is on glibc-2.1.1pre2, Linux-2.2.6 (PPC). > >Is this platform specific or does anybody else notice this? After a quick browse through the source I came up with the following untested patch. Does it look right? It doesn't look right to me. We need to rename the file FROM to TO. In the normal case of strip, FROM is a temporary file, and TO is the original file which we are stripping. However, this function is also used in other cases. If TO does not exist, we should just use rename. This is not the normal case of strip, but it happens in other cases. Your patch breaks that. That seems to be only significant change in your patch. Perhaps I am missing something. I think the only way to reliably preserve ownership is to avoid using rename. Perhaps the code should be changed to call simple_copy when the owner of the file differs from the effective uid. Ian ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: strip looses original file ownership and file permissions 1999-05-06 9:27 ` Ian Lance Taylor @ 1999-05-06 9:54 ` Franz Sirl 1999-05-06 11:03 ` Ian Lance Taylor 0 siblings, 1 reply; 7+ messages in thread From: Franz Sirl @ 1999-05-06 9:54 UTC (permalink / raw) To: Ian Lance Taylor; +Cc: gas2 At 18:27 06.05.99 , Ian Lance Taylor wrote: > Date: Thu, 06 May 1999 13:44:47 +0200 > From: Franz Sirl <Franz.Sirl-kernel@lauterbach.com> > > >I just verified that strip out of gas-990418 looses original > ownership and > >permissions of a file. > >This is on glibc-2.1.1pre2, Linux-2.2.6 (PPC). > > > >Is this platform specific or does anybody else notice this? > > After a quick browse through the source I came up with the following > untested patch. Does it look right? > >It doesn't look right to me. > >We need to rename the file FROM to TO. In the normal case of strip, >FROM is a temporary file, and TO is the original file which we are >stripping. However, this function is also used in other cases. > >If TO does not exist, we should just use rename. This is not the >normal case of strip, but it happens in other cases. Your patch >breaks that. That seems to be only significant change in your patch. >Perhaps I am missing something. > >I think the only way to reliably preserve ownership is to avoid using >rename. Perhaps the code should be changed to call simple_copy when >the owner of the file differs from the effective uid. I think my patch produces the behavior that the comments are suggesting. The old code was bogus anyway, cause it tried to chmod/chown "to" after rename with values from a non-existing (exists != 0, this is really a bad-named variable, it's named the opposite of it's meaning) file. Franz. ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: strip looses original file ownership and file permissions 1999-05-06 9:54 ` Franz Sirl @ 1999-05-06 11:03 ` Ian Lance Taylor 1999-05-06 11:43 ` Franz Sirl 0 siblings, 1 reply; 7+ messages in thread From: Ian Lance Taylor @ 1999-05-06 11:03 UTC (permalink / raw) To: Franz.Sirl-kernel; +Cc: gas2 Date: Thu, 06 May 1999 18:54:16 +0200 From: Franz Sirl <Franz.Sirl-kernel@lauterbach.com> At 18:27 06.05.99 , Ian Lance Taylor wrote: > Date: Thu, 06 May 1999 13:44:47 +0200 > From: Franz Sirl <Franz.Sirl-kernel@lauterbach.com> > > >I just verified that strip out of gas-990418 looses original > ownership and > >permissions of a file. > >This is on glibc-2.1.1pre2, Linux-2.2.6 (PPC). > > > >Is this platform specific or does anybody else notice this? > > After a quick browse through the source I came up with the following > untested patch. Does it look right? > >It doesn't look right to me. > >We need to rename the file FROM to TO. In the normal case of strip, >FROM is a temporary file, and TO is the original file which we are >stripping. However, this function is also used in other cases. > >If TO does not exist, we should just use rename. This is not the >normal case of strip, but it happens in other cases. Your patch >breaks that. That seems to be only significant change in your patch. >Perhaps I am missing something. > >I think the only way to reliably preserve ownership is to avoid using >rename. Perhaps the code should be changed to call simple_copy when >the owner of the file differs from the effective uid. I think my patch produces the behavior that the comments are suggesting. The old code was bogus anyway, cause it tried to chmod/chown "to" after rename with values from a non-existing (exists != 0, this is really a bad-named variable, it's named the opposite of it's meaning) file. You're right. I applied the appended patch, which I think should fix the problem. Ian Index: rename.c =================================================================== RCS file: /cvs/binutils/binutils/binutils/rename.c,v retrieving revision 1.1.1.1 diff -u -r1.1.1.1 rename.c --- rename.c 1999/05/03 07:29:10 1.1.1.1 +++ rename.c 1999/05/06 18:02:05 @@ -139,17 +139,17 @@ const char *to; int preserve_dates; { - int exists; + boolean exists; struct stat s; int ret = 0; - exists = lstat (to, &s); + exists = lstat (to, &s) == 0; #if defined (_WIN32) && !defined (__CYGWIN32__) /* Win32, unlike unix, will not erase `to' in `rename(from, to)' but fail instead. Also, chown is not present. */ - if (exists == 0) + if (exists) remove (to); ret = rename (from, to); @@ -163,7 +163,7 @@ #else /* Use rename only if TO is not a symbolic link and has only one hard link. */ - if (exists < 0 || (!S_ISLNK (s.st_mode) && s.st_nlink == 1)) + if (! exists || (!S_ISLNK (s.st_mode) && s.st_nlink == 1)) { ret = rename (from, to); if (ret == 0) ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: strip looses original file ownership and file permissions 1999-05-06 11:03 ` Ian Lance Taylor @ 1999-05-06 11:43 ` Franz Sirl 1999-05-06 13:57 ` Ian Lance Taylor 0 siblings, 1 reply; 7+ messages in thread From: Franz Sirl @ 1999-05-06 11:43 UTC (permalink / raw) To: Ian Lance Taylor; +Cc: gas2 At 20:03 06.05.99 , Ian Lance Taylor wrote: > I think my patch produces the behavior that the comments are suggesting. > The old code was bogus anyway, cause it tried to chmod/chown "to" after > rename with values from a non-existing (exists != 0, this is really a > bad-named variable, it's named the opposite of it's meaning) file. > >You're right. I applied the appended patch, which I think should fix >the problem. Ok, this works fine, thanks. Btw, have you ever looked at the patch in http://sourceware.cygnus.com/ml/gas2/1999/msg00041.html ? Is it the correct fix for the make check FAIL on powerpc-linux-gnu? Franz. ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: strip looses original file ownership and file permissions 1999-05-06 11:43 ` Franz Sirl @ 1999-05-06 13:57 ` Ian Lance Taylor 0 siblings, 0 replies; 7+ messages in thread From: Ian Lance Taylor @ 1999-05-06 13:57 UTC (permalink / raw) To: Franz.Sirl-kernel; +Cc: gas2 Date: Thu, 06 May 1999 20:42:53 +0200 From: Franz Sirl <Franz.Sirl-kernel@lauterbach.com> Btw, have you ever looked at the patch in http://sourceware.cygnus.com/ml/gas2/1999/msg00041.html ? Is it the correct fix for the make check FAIL on powerpc-linux-gnu? No, I don't think it is. I think this is a problem in the glibc dynamic linker. (I looked at glibc 2.0.7; I don't know about other versions.) The GNU linker does not ordinarily create a PT_PHDR program segment for a shared library. This follows the lead of the Solaris linker. When the glibc 2.0.7 dynamic linker does not find a PT_PHDR program segment, it assumes that the program headers may be found at the start of the first PT_LOAD segment. However, ELF does not require this. The comment in the glibc sources (in dl-load.c) says this: /* There was no PT_PHDR specified. We need to find the phdr in the load image ourselves. We assume it is in fact in the load image somewhere, and that the first load command starts at the beginning of the file and thus contains the ELF file header. */ However, I believe the dynamic linker should be able to operate without having the program headers in the load segment at all. What your patch changes is whether the linker puts the program headers at the start of the first PT_LOAD segment. Without your patch, the linker does not put the program headers in loadable memory. With your patch, it does. Since with the current script the program headers are not loaded, the dynamic linker will wind up looking at nonsensical memory for the program headers, and it will fail. However, the dynamic linker does know how to find the correct program headers by reading the file. However, after reading the program headers, it throws them away, and uses the copy which it expects to find in the file. If there is no PT_PHDR segment, it should instead keep and use its own copy. Ian ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~1999-05-06 13:57 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 1999-05-06 3:48 strip looses original file ownership and file permissions Franz Sirl 1999-05-06 4:45 ` Franz Sirl 1999-05-06 9:27 ` Ian Lance Taylor 1999-05-06 9:54 ` Franz Sirl 1999-05-06 11:03 ` Ian Lance Taylor 1999-05-06 11:43 ` Franz Sirl 1999-05-06 13:57 ` Ian Lance Taylor
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).