From owner-freebsd-hackers@FreeBSD.ORG Wed Oct 6 23:03:01 2010 Return-Path: Delivered-To: freebsd-hackers@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id EDB42106566C; Wed, 6 Oct 2010 23:03:01 +0000 (UTC) (envelope-from yanegomi@gmail.com) Received: from mail-iw0-f182.google.com (mail-iw0-f182.google.com [209.85.214.182]) by mx1.freebsd.org (Postfix) with ESMTP id 9D6D38FC13; Wed, 6 Oct 2010 23:03:01 +0000 (UTC) Received: by iwn8 with SMTP id 8so147495iwn.13 for ; Wed, 06 Oct 2010 16:03:01 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=domainkey-signature:mime-version:received:sender:received :in-reply-to:references:date:x-google-sender-auth:message-id:subject :from:to:cc:content-type; bh=OT9MocMOFKh746L3QIUTO3o+CEln2CH9q0EDmsarrTY=; b=W93G5NEBatX6mZw28sOJstGk/Xz/si1osh2p6dwP9tzErakF0hQrfvn8ROeZ3f27qF /aGkBLUCEMAGJOGOQPBc4d9a6zZ34IEplygmATqa2AeO1f+aQEAv7gj33O8/Yn3bCy4V 20rVouGqVpiebqhPTHduwY018gb66e+d4c/QE= DomainKey-Signature: a=rsa-sha1; c=nofws; d=gmail.com; s=gamma; h=mime-version:sender:in-reply-to:references:date :x-google-sender-auth:message-id:subject:from:to:cc:content-type; b=EaJYS8Ij2MEWhcK1H4kmU8DPpRb8pkZWBEZNAk4BaNCkLXOv7Pa1YpDoLQnWeq8NEU 4QBtfZsnyX6rhVAkryAtCdDFVnqkTpB46eJPz4BxPfQIb5X3cG2ZYHNjaIdx7rtmpSkF Pg0TWwznZmanu4ayc6euAf9AgcMRCtBvgzQ7Q= MIME-Version: 1.0 Received: by 10.231.149.198 with SMTP id u6mr14770364ibv.7.1286406180844; Wed, 06 Oct 2010 16:03:00 -0700 (PDT) Sender: yanegomi@gmail.com Received: by 10.231.184.3 with HTTP; Wed, 6 Oct 2010 16:03:00 -0700 (PDT) In-Reply-To: References: <20101005235054.GA45827@freebsd.org> <20101006173522.GA92402@freebsd.org> <20101006193827.GA13528@freebsd.org> Date: Wed, 6 Oct 2010 16:03:00 -0700 X-Google-Sender-Auth: ZnnEC0SXHaRIg4P3YR2DnT0wyqo Message-ID: From: Garrett Cooper To: Sergey Kandaurov Content-Type: multipart/mixed; boundary=0050450157037235620491fac75b Cc: Alexander Best , freebsd-hackers@freebsd.org Subject: Re: issue with unsetting 'arch' flag X-BeenThere: freebsd-hackers@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Technical Discussions relating to FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 06 Oct 2010 23:03:02 -0000 --0050450157037235620491fac75b Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable On Wed, Oct 6, 2010 at 3:01 PM, Sergey Kandaurov wrote: > On 6 October 2010 23:38, Alexander Best wrote: >> On Wed Oct =A06 10, Garrett Cooper wrote: >>> On Wed, Oct 6, 2010 at 10:35 AM, Alexander Best w= rote: >>> > On Wed Oct =A06 10, Garrett Cooper wrote: >>> >> On Tue, Oct 5, 2010 at 4:50 PM, Alexander Best = wrote: >>> >> > hi there, >>> >> > >>> >> > i think the following example shows the problem better than a long= explanation: >>> >> > >>> >> > `touch ftest && chflags arch ftest && chflags -vv 0 ftest`. >>> >> > =A0^^non-root =A0 =A0 ^^root =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0^^non-= root >>> >> > >>> >> > chflags claims to have cleared the 'arch' flag (which should be im= possible as >>> >> > non-root user), but indeed has done nothing. >>> >> > >>> >> > i've tried the same with 'sappnd' and that works as can be expecte= d. >>> >> > >>> >> > The issue was confirmed to exist in HEAD (me), stable/8 (pgollucc1= , jpaetzel) >>> >> > and stable/7 (nox). >>> >> > On stable/6 it does NOT exist (jpaetzel). chflags properly fails w= ith EPERM. >>> >> >>> >> =A0 =A0 Fails for me when I call the syscall directly, as I would ex= pect, >>> >> and passes when I'm superuser: >>> >> >>> >> $ ./test_chflags >>> >> (uid, euid) =3D (1000, 1000) >>> >> test_chflags: chflags: Operation not permitted >>> >> test_chflags: lchflags: Operation not permitted >>> >> $ sudo ./test_chflags >>> >> (uid, euid) =3D (0, 0) >>> >> >>> >> =A0 =A0 According to my basic inspection in strtofflags >>> >> (.../lib/libc/gen/strtofflags.c), it works as well. >>> >> =A0 =A0 And last but not least, executing the commands directly on t= he CLI work: >>> >> >>> >> $ tmpfile=3D`mktemp /tmp/chflags.XXXXXX` >>> >> $ chflags arch $tmpfile >>> >> chflags: /tmp/chflags.nQm1IL: Operation not permitted >>> >> $ rm $tmpfile >>> >> $ tmpfile=3D`mktemp /tmp/chflags.XXXXXX` >>> >> $ sudo chflags arch $tmpfile >>> >> $ sudo chflags noarch $tmpfile >>> >> $ rm $tmpfile >>> > >>> > thanks for your test app and helping out with this problem. i'm not s= ure >>> > however you understood the problem. probably i didn't explain it righ= t: >>> > >>> > $ sudo rm -d /tmp/chflags.XXXXXX >>> > $ tmpfile=3D`mktemp /tmp/chflags.XXXXXX` >>> > $ sudo chflags arch $tmpfile >>> > $ chflags noarch $tmpfile >>> > >>> > is what's causing the problem. the last chflags call should fail, but= it >>> > doesn't. >>> >>> Sorry... my CLI based example was stupid. I meant: >>> >>> $ tmpfile=3D`mktemp /tmp/chflags.XXXXXX` >>> $ chflags arch $tmpfile >>> chflags: /tmp/chflags.V2NpXR: Operation not permitted >>> $ chflags noarch $tmpfile >>> $ rm $tmpfile >>> >>> Currently chflags(2) states: >>> >>> =A0 =A0 =A0The SF_IMMUTABLE, SF_APPEND, SF_NOUNLINK, and SF_ARCHIVED fl= ags may only >>> =A0 =A0 =A0be set or unset by the super-user. =A0Attempts to set these = flags by non- >>> =A0 =A0 =A0super-users are rejected, >>> attempts by non-superusers to = clear >>> flags that >>> =A0 =A0 =A0are already unset are silently ignored. <<< =A0These flags m= ay be set at any >>> =A0 =A0 =A0time, but normally may only be unset when the system is in s= ingle-user >>> =A0 =A0 =A0mode. =A0(See init(8) for details.) >>> >>> So this behavior is already well documented :). The EPERM section >>> should really note SF_ARCHIVED though (whoever added the flag forgot >>> to add that particular item to the ERRORS section). >> >> that's perfectly alright. clearing an unset flag shouldn't cause any err= or to >> be returned. however in my example arch *does* get set and still trying = to >> unset it as normal user doesn't return an error. >> > > It's even more interesting. > > As far as I could parse the code: > - UFS has no special handling for SF_ARCHIVED (I found only it for msdosf= s) _very_ interesting: [/sys]$ grep -r SF_ARCHIVED kern/ fs/ ufs/ | grep -v svn fs/msdosfs/msdosfs_vnops.c: vap->va_flags |=3D SF_ARCHIVED; fs/msdosfs/msdosfs_vnops.c: if (vap->va_flags & ~SF_ARCHIVED) fs/msdosfs/msdosfs_vnops.c: if (vap->va_flags & SF_ARCHIVED) The commit that introduced this change probably wasn't doing the right thing: http://svn.freebsd.org/viewvc/base/head/sys/fs/msdosfs/msdosfs= _vnops.c?revision=3D5241&view=3Dmarkup ; cp(1) probably should have been fixed in lieu of `fixing' msdosfs. > - ufs_setattr() does not handle unsetting SF_ARCHIVED, > =A0so all what it does is simply return zero. [EOPNOTSUPP] The underlying file system does not support file flags. So I would expect for invalid flags to return EOPNOTSUPP. ... $ ~/test_chflags_negative test_chflags_negative: should not get here $ sudo ~/test_chflags_negative test_chflags_negative: should not get here *facepalm* I think the problem in part is here (sys/stat.h): * * Super-user and owner changeable flags. */ #define UF_SETTABLE 0x0000ffff /* mask of owner changeable flags *= / #define UF_NODUMP 0x00000001 /* do not dump file */ #define UF_IMMUTABLE 0x00000002 /* file may not be changed */ #define UF_APPEND 0x00000004 /* writes to file may only append *= / #define UF_OPAQUE 0x00000008 /* directory is opaque wrt. union *= / #define UF_NOUNLINK 0x00000010 /* file may not be removed or renam= ed */ /* * Super-user changeable flags. */ #define SF_SETTABLE 0xffff0000 /* mask of superuser changeable fla= gs */ #define SF_ARCHIVED 0x00010000 /* file is archived */ #define SF_IMMUTABLE 0x00020000 /* file may not be changed */ #define SF_APPEND 0x00040000 /* writes to file may only append *= / #define SF_NOUNLINK 0x00100000 /* file may not be removed or renam= ed */ #define SF_SNAPSHOT 0x00200000 /* snapshot inode */ Note the *_SETTABLE macros, and the fact that they allow for more functionality than what's currently slotted with the one-hot encoded flags currently available. SF_ARCHIVED is not present in the other BSDs or Mac OSX either (I did some hunting for a python bug related to chflags a few weeks ago)... and I'm not even sure what this functionality really buys us because it's not well described (but I'd be happy to get an explanation/history lesson). > - /bin/chflags doesn't check the actual flags value from inode after > calling chflags() syscall, and blindly assumes all is well, if chflags() > returns with zero, Yeah... but ideally tests should be written for this stuff and exercised on all filesystems and exercised whenever code in this particular path is changed, because that would potentially turn into a noticeable performance hit [depending on how it's implemented in chflags(1)]. And lo and behold it already does exist under .../tools/regression/fstest/tests/chflags . I'll audit this once I get back home... Thanks, -Garrett --0050450157037235620491fac75b Content-Type: application/octet-stream; name="test_chflags_negative.c" Content-Disposition: attachment; filename="test_chflags_negative.c" Content-Transfer-Encoding: base64 X-Attachment-Id: f_geyss8wh0 I2luY2x1ZGUgPHN5cy9zdGF0Lmg+CiNpbmNsdWRlIDxlcnIuaD4KI2luY2x1ZGUgPHVuaXN0ZC5o PgoKaW50Cm1haW4odm9pZCkKewoJaWYgKGNoZmxhZ3MoIi90bXAiLCAtMSkpIHsKCQl3YXJuKCJP ayIpOwoJCXJldHVybiAoMCk7Cgl9Cgl3YXJueCgic2hvdWxkIG5vdCBnZXQgaGVyZSIpOwoJcmV0 dXJuICgxKTsKfQo= --0050450157037235620491fac75b--