Skip site navigation (1)Skip section navigation (2)
Date:      Wed, 11 Apr 2012 10:03:30 +1000 (EST)
From:      Bruce Evans <brde@optusnet.com.au>
To:        Jaakko Heinonen <jh@freebsd.org>
Cc:        svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org
Subject:   Re: svn commit: r234103 - in head: lib/libc/sys sys/ufs/ufs
Message-ID:  <20120411085433.S1057@besplex.bde.org>
In-Reply-To: <201204101559.q3AFxbX2030563@svn.freebsd.org>
References:  <201204101559.q3AFxbX2030563@svn.freebsd.org>

next in thread | previous in thread | raw e-mail | index | archive | help
On Tue, 10 Apr 2012, Jaakko Heinonen wrote:

> Log:
>  - Return EPERM from ufs_setattr() when an user without PRIV_VFS_SYSFLAGS
>    privilege attempts to toggle SF_SETTABLE flags.
>  - Use the '^' operator in the SF_SNAPSHOT anti-toggling check.
>
>  Flags are now stored to ip->i_flags in one place after all checks.
>
>  Submitted by:	bde

Thanks.

The first point fixes longstanding brokenness of chflags(2).

> ...
> Modified: head/lib/libc/sys/chflags.2
> ==============================================================================
> --- head/lib/libc/sys/chflags.2	Tue Apr 10 15:58:22 2012	(r234102)
> +++ head/lib/libc/sys/chflags.2	Tue Apr 10 15:59:37 2012	(r234103)
> @@ -28,7 +28,7 @@
> .\"	@(#)chflags.2	8.3 (Berkeley) 5/2/95
> .\" $FreeBSD$
> .\"
> -.Dd Oct 29, 2010
> +.Dd Apr 10, 2012
> .Dt CHFLAGS 2
> .Os
> .Sh NAME
> @@ -114,8 +114,7 @@ The
> and
> .Dv SF_ARCHIVED
> flags may only be set or unset by the super-user.
> -Attempts to set these flags by non-super-users are rejected, attempts by
> -non-superusers to clear flags that are already unset are silently ignored.
> +Attempts to toggle these flags by non-super-users are rejected.
> These flags may be set at any time, but normally may only be unset when
> the system is in single-user mode.
> (See

The fixed semantics are documented here.  Previously, normal read-modify-
write didn't work for non-super-users when trying to change user flags
while preserving system flags that don't prevent user changes (SF_ARCHIVED
and SF_SNAPSHOT).  Naive programs like chflags(1) didn't understand the
complications and just did a normal read-modify-write.

A few more changes in the man page are needed.  Quoting
"man ./chflags.2 | col -bx":

%  ERRORS
%       The chflags() system call will fail if:
%  ...
%       [EPERM]            A non-super-user tries to set one of SF_ARCHIVED,
%                          SF_IMMUTABLE, SF_APPEND, or SF_NOUNLINK.

s/set/toggle/

s/tries to/attempted to/

This fixes this part of the man page not being as formal as the rest.
This is the only man page in all of libc/sys that uses the expression
"tries to".  3 lines use the correct tense "tried to".  14 lines use
"attempted to".  I got the changed wording mostly from getpriority.2.
The other (mostly worse) variations with "attempt" and "super" are:

% clock_gettime.2:A user other than the super-user attempted to set the time.
% ffclock.2:A user other than the super-user attempted to set the feed-forward clock
% getpriority.2:A non super-user attempted to lower a process priority.
% gettimeofday.2:A user other than the super-user attempted to set the time.
% jail.2:A user other than the super-user attempted to attach to or remove a jail.
% kenv.2:a user other than the superuser attempted to set or unset a kernel

Most of these presume that there is only 1 super-user.  "non-super-user"
avoids this and is shorter.  Apparently, "non-super-user" should be
hyhenated after "non", since 5 lines do it and only the above line in
getpriority.2 doesn't do it.  kenv.2 is also missing capitalization of
the sentence and hyphenation of super-user.  99 lines hyphenate
super-user, and only 16 don't.

% 
%       [EPERM]            User tries to set or remove the SF_SNAPSHOT flag.

s/User tries to set or remove/An attempt was made to toggle/

This sentence had many more bugs:
- missing article before "User"
- but "User" was just misleading -- the error applies to all callers
- "tries to" was informal
- "set or remove" was less clear than "toggle".

%  ...
%       The fchflags() system call will fail if:
%  ...
%       [EPERM]            A non-super-user tries to set one of SF_ARCHIVED,
%                          SF_IMMUTABLE, SF_APPEND, or SF_NOUNLINK.
% 
%       [EPERM]            User tries to set or remove the SF_SNAPSHOT flag.

As above.

Reviewing all other FreeBSD changes to chflags.2 showed a few more
problems:
- the description of SF_SNAPSHOT says "cannot be changed".  "cannot be
   toggled" would be clearer.  It also says "by any user", and as above,
   "user" is just misleading.  It is this syscall that cannot change
   SF_SNAPSHOT.
- the main description and the 2 EPERM error descriptions of SF_IMMUTABLE,
   etc., presume only 1 super-user
- the EOPNOTSUPP error description still says "the underlying file system
   does not support file flags", but this error can now happen even for
   ffs when the underlying file system doesn't support at least one of the
   flags that the syscall attempted to set
- lchflags() is missing in HISTORY.

Bruce



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20120411085433.S1057>