Hello Bastien,

Thank you for taking the time to prepare this patch.

As already noted, the subject line of a patch should read

    [PATCH ...] page.n: .....

I've applied the patch, but made many edits.

On 1/30/21 12:29 AM, roucaries.bast...@gmail.com wrote:
> From: Bastien Roucariès <ro...@debian.org>
> 
> Clearly document that HOME, LOGNAME, SHELL and USER are set at login time
> by program like login (1).
> 
> Document also that using su could result in a mixed environment, and point to
> su manual.
> 
> Signed-off-by: Bastien Roucariès <ro...@debian.org>
> ---
>  man7/environ.7 | 34 ++++++++++++++++++++++++++++++----
>  1 file changed, 30 insertions(+), 4 deletions(-)
> 
> diff --git a/man7/environ.7 b/man7/environ.7
> index 8f89ba86b..e33dcc754 100644
> --- a/man7/environ.7
> +++ b/man7/environ.7
> @@ -65,15 +65,15 @@ Common examples are:
>  .TP
>  .B USER
>  The name of the logged-in user (used by some BSD-derived programs).
> +Set at login time, see section NOTES below.
>  .TP
>  .B LOGNAME
>  The name of the logged-in user (used by some System-V derived programs).
> +Set at login time, see section NOTES below.
>  .TP
>  .B HOME
> -A user's login directory, set by
> -.BR login (1)
> -from the password file
> -.BR passwd (5).
> +A user's login directory, set a login time.
> +Set at login time, see section NOTES below.
>  .TP
>  .B LANG
>  The name of a locale to use for locale categories when not overridden
> @@ -114,6 +114,7 @@ Set by some shells.
>  .TP
>  .B SHELL
>  The absolute pathname of the user's login shell.
> +Set at login time, see section NOTES below.
>  .TP
>  .B TERM
>  The terminal type for which output is to be prepared.
> @@ -260,6 +261,30 @@ The
>  and
>  .B PR_SET_MM_ENV_END
>  operations can be used to control the location of the process's environment.
> +.PP
> +The
> +.B HOME,
> +.B LOGNAME,
> +.B SHELL
> +and
> +.B USER

Please take a careful look at man-pages(7) and also groff_man(7).
The above should be written as:

The
.BR HOME ,
.BR LOGNAME ,
.BR SHELL ,
and
.B USER

> +variables are only set when an user is changing using

s/only//

> +session management interface, typically by program
> +.B login(1)

.BR login (1)

> +from user database (for instance, but not limited, by using
> +.B password (5)

s/password/passwd/

> +database).
> +Particularly,
> +.BR setuid (2)
> +family of function
> +does not set theses variables. Notes that as documented,

I'm not sure that there's value in mentioning that setuid(2)
doesn't change these.

Also, (again) as previously stated, new sentences ("Notes...")
should start on new lines.

> +going to root by
> +.BR su (8)
> +may result in a mixed environment where
> +.B LOGNAME
> +and
> +.B USER
> +are retained from old user.
>  .SH BUGS
>  Clearly there is a security risk here.
>  Many a system command has been
> @@ -305,6 +330,7 @@ should consider renaming their option to
>  .BR login (1),
>  .BR printenv (1),
>  .BR sh (1),
> +.BR su (1),
>  .BR tcsh (1),
>  .BR execve (2),
>  .BR clearenv (3),

Thanks, 

Michael


-- 
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Linux/UNIX System Programming Training: http://man7.org/training/

Reply via email to