Date: Tue, 6 Jun 2006 18:42:54 +0200 From: Martin Kammerhofer <dada@pluto.tugraz.at> To: freebsd-bugs@FreeBSD.org Subject: Re: bin/88365: resubmission of MTA mangled PR Message-ID: <200606061642.k56Ggslg027123@pluto.tugraz.at>
next in thread | raw e-mail | index | archive | help
>Submitter-Id: current-users
>Originator: Martin Kammerhofer
>Organization: Graz Uni
>Confidential: no
>Synopsis: [patch] /bin/rm has problems with symbolic link flags
>Severity: non-critical
>Priority: medium
>Category: bin
>Class: sw-bug
>Release: FreeBSD 6.1-STABLE i386
>Environment:
System: FreeBSD Martin.liebt.Susi 6.1-STABLE FreeBSD 6.1-STABLE #2: Fri Jun 2 14:53:34 CEST 2006 toor@Martin.liebt.Susi:/usr/obj/usr/src/sys/P2B-S i386
>Description:
The FreeBSD /bin/rm silently overrides the UF_APPEND|UF_IMMUTABLE
flags when running as super user. This should be documented in the
rm(1) manual page.
Since FreeBSD 5.X and later have gotten lchflags(2) support, semantics
are slightly broken. While standards dictate that rm(1), unlink(2) and
rename(2) never follow symbolic links, the FreeBSD /bin/rm _does_
sometimes follow symbolic links inside the implicit chflags(2) call.
This leads to the following behaviour:
(1) /bin/rm operating as super user cannot remove a symbolic link
which has UF_APPEND|UF_IMMUTABLE != 0.
(2) /bin/rm when running as super user and failing to unlink a
UF_APPEND|UF_IMMUTABLE protected symbolic link will reset the
UF_APPEND and UF_IMMUTABLE flags on the symbolic links target
- an object that /bin/rm must not change!
Imho (1) is a nuisance while I consider (2) broken behaviour.
/usr/bin/find ... -delete behaves like /bin/rm.
pkg_add has a similar issue when creating backup files.
Finally /bin/cp -Rp should be made aware of lchflags(2) (Patch
fragment below is similar to the slightly more verbose patch in PR
bin/90823)
>How-To-Repeat:
su
touch myfile
ln -s myfile myslink
chflags -h uchg myfile myslink
ls -lo myfile myslink
rm -f myslink # fails and modifies myfile instead
ls -lo myfile myslink
cp -Rp myslink myslink2 # fails to copy uchg flag (ENOSYS!)
ls -lo myslink*
>Fix:
Note: When creating the patch below I was not aware of PR kern/29355.
It turns out that my patch is a small subset of the PR 29355 userland
part. 29355 is 4 years old now and most of its functionality is
already in the tree. My patch catches up on the still missing
"s/chflags/lchflags/"-parts. It is against --current.
Index: bin/cp/utils.c
===================================================================
RCS file: /home/ncvs/src/bin/cp/utils.c,v
retrieving revision 1.46
diff -u -t -r1.46 utils.c
--- bin/cp/utils.c 5 Sep 2005 04:36:08 -0000 1.46
+++ bin/cp/utils.c 5 Jun 2006 10:55:29 -0000
@@ -321,7 +321,7 @@
if (!gotstat || fs->st_flags != ts.st_flags)
if (fdval ?
fchflags(fd, fs->st_flags) :
- (islink ? (errno = ENOSYS) :
+ (islink ? lchflags(to.p_path, fs->st_flags) :
chflags(to.p_path, fs->st_flags))) {
warn("chflags: %s", to.p_path);
rval = 1;
Index: bin/rm/rm.c
===================================================================
RCS file: /home/ncvs/src/bin/rm/rm.c,v
retrieving revision 1.54
diff -u -t -r1.54 rm.c
--- bin/rm/rm.c 15 Apr 2006 09:26:23 -0000 1.54
+++ bin/rm/rm.c 5 Jun 2006 10:47:38 -0000
@@ -231,7 +231,7 @@
else if (!uid &&
(p->fts_statp->st_flags & (UF_APPEND|UF_IMMUTABLE)) &&
!(p->fts_statp->st_flags & (SF_APPEND|SF_IMMUTABLE)) &&
- chflags(p->fts_accpath,
+ lchflags(p->fts_accpath,
p->fts_statp->st_flags &= ~(UF_APPEND|UF_IMMUTABLE)) < 0)
goto err;
continue;
@@ -250,7 +250,7 @@
if (!uid &&
(p->fts_statp->st_flags & (UF_APPEND|UF_IMMUTABLE)) &&
!(p->fts_statp->st_flags & (SF_APPEND|SF_IMMUTABLE)))
- rval = chflags(p->fts_accpath,
+ rval = lchflags(p->fts_accpath,
p->fts_statp->st_flags &= ~(UF_APPEND|UF_IMMUTABLE));
if (rval == 0) {
/*
@@ -350,7 +350,7 @@
if (!uid &&
(sb.st_flags & (UF_APPEND|UF_IMMUTABLE)) &&
!(sb.st_flags & (SF_APPEND|SF_IMMUTABLE)))
- rval = chflags(f, sb.st_flags & ~(UF_APPEND|UF_IMMUTABLE));
+ rval = lchflags(f, sb.st_flags & ~(UF_APPEND|UF_IMMUTABLE));
if (rval == 0) {
if (S_ISWHT(sb.st_mode))
rval = undelete(f);
Index: usr.bin/find/function.c
===================================================================
RCS file: /home/ncvs/src/usr.bin/find/function.c,v
retrieving revision 1.55
diff -u -t -r1.55 function.c
--- usr.bin/find/function.c 3 Apr 2006 20:36:37 -0000 1.55
+++ usr.bin/find/function.c 5 Jun 2006 11:05:26 -0000
@@ -439,7 +439,7 @@
if ((entry->fts_statp->st_flags & (UF_APPEND|UF_IMMUTABLE)) &&
!(entry->fts_statp->st_flags & (SF_APPEND|SF_IMMUTABLE)) &&
geteuid() == 0)
- chflags(entry->fts_accpath,
+ lchflags(entry->fts_accpath,
entry->fts_statp->st_flags &= ~(UF_APPEND|UF_IMMUTABLE));
/* rmdir directories, unlink everything else */
Index: usr.sbin/pkg_install/add/extract.c
===================================================================
RCS file: /home/ncvs/src/usr.sbin/pkg_install/add/extract.c,v
retrieving revision 1.44
diff -u -t -r1.44 extract.c
--- usr.sbin/pkg_install/add/extract.c 7 Jan 2006 22:10:57 -0000 1.44
+++ usr.sbin/pkg_install/add/extract.c 5 Jun 2006 12:40:05 -0000
@@ -63,8 +63,7 @@
if (q->type == PLIST_FILE) {
snprintf(try, FILENAME_MAX, "%s/%s", dir, q->name);
if (make_preserve_name(bup, FILENAME_MAX, name, try) && fexists(bup)) {
- (void)chflags(try, 0);
- (void)unlink(try);
+ (void)lchflags(try, 0);
if (rename(bup, try))
warnx("rollback: unable to rename %s back to %s", bup, try);
}
@@ -160,7 +159,7 @@
/* first try to rename it into place */
snprintf(try, FILENAME_MAX, "%s/%s", Directory, p->name);
if (fexists(try)) {
- (void)chflags(try, 0); /* XXX hack - if truly immutable, rename fails */
+ (void)lchflags(try, 0); /* XXX hack - if truly immutable, rename fails */
if (preserve && PkgName) {
char pf[FILENAME_MAX];
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200606061642.k56Ggslg027123>