Bug 18243

Summary: wrong description of -p option in sh(1) manpage
Product: Documentation Reporter: mellon <mellon>
Component: Books & ArticlesAssignee: alex <alex>
Status: Closed FIXED    
Severity: Affects Only Me    
Priority: Normal    
Version: Latest   
Hardware: Any   
OS: Any   
Attachments:
Description Flags
file.diff none

Description mellon 2000-04-26 23:50:01 UTC
A simple mistake in the manpage of sh(1). The patch below
is self-explanatory. The relevant code handling the option is in
bin/sh/options.s
Comment 1 Sheldon Hearn 2000-04-27 08:54:07 UTC
On Thu, 27 Apr 2000 01:40:57 GMT, mellon@pobox.com wrote:

>  .It Fl p Li privileged
>  Turn on privileged mode.  This mode is enabled on startup
>  if either the effective user or group id is not equal to the
> -real user or group id.  Turning this mode off sets the
> +real user or group id.  Turning this mode on sets the
>  effective user and group ids to the real user and group ids.

Weird, I don't get this at all.  My take on the code is that it should
say something like:

	Set the effective user and group ids
	to the real user and group ids respectively
	if this option is enabled on startup.

The existing text doesn't make any sense to me.  Am I right with my
description?

Ciao,
Sheldon.
Comment 2 mellon 2000-04-27 09:12:46 UTC
You, Sheldon Hearn, were spotted writing this on Thu, Apr 27, 2000 at 09:54:07AM +0200:
> 
> On Thu, 27 Apr 2000 01:40:57 GMT, mellon@pobox.com wrote:
> 
> >  .It Fl p Li privileged
> >  Turn on privileged mode.  This mode is enabled on startup
> >  if either the effective user or group id is not equal to the
> > -real user or group id.  Turning this mode off sets the
> > +real user or group id.  Turning this mode on sets the
> >  effective user and group ids to the real user and group ids.
> 
> Weird, I don't get this at all.  My take on the code is that it should
> say something like:
> 
> 	Set the effective user and group ids
> 	to the real user and group ids respectively
> 	if this option is enabled on startup.
> 
> The existing text doesn't make any sense to me.  Am I right with my
> description?

Yes. The option exists to guard against suid shell scripts (I presume)
by denying the shell any privileges it has from the suid bit. The
existing text, however, intends to describe both the effect of the
option and the general effect of the shell running suid. It calls
the former "turning the option on" and the latter "enabling the option"
referring to *different* things by these which is damn confusing. If the
shell is running suid, a different profile file is sourced and $ENV
is ignored -- both regardless of the -p option -- and the text tries to 
explain this. 

I think the description of different behavior under suid should be
put elsewhere earlier in the manpage, and referred to as running in
the privileged mode, while -p should be described as you did together
with stating that it only works in privileged mode. There's no point
in describing -p as "turning the privileged mode on" anyway because
it would be typically used to take extra privileges away rather than
enable them.

If you can hack this, more power to you! I tried to rewrite this
paragraph coherently, but gave up soon; thus the fix in my PR only
fixed the most obvious error, while secretly hoping people'll notice
the general stylistic conundrum. It worked ;)

-- 
Anatoly Vorobey,
mellon@pobox.com http://pobox.com/~mellon/
"Angels can fly because they take themselves lightly" - G.K.Chesterton
Comment 3 alex freebsd_committer freebsd_triage 2000-07-15 10:02:56 UTC
Responsible Changed
From-To: freebsd-doc->alex

I think the current behaviour w/ -p is a bug of sh, not of the documentation. 
However, I have my hands in it now, and will take a look at it after 
I have comments about the possible bug.
Comment 4 alex freebsd_committer freebsd_triage 2001-06-08 13:10:37 UTC
State Changed
From-To: open->closed

sh behaves exactly as stated in the manpage. 
Remember that +p disables an option, i.e. "turning it off". 
This is also done in the source: 
"+" -> val == 0 -> only setuid(getuid()),  if !val && priviledged.