Re: svn commit: r334543 - head/usr.bin/top

2018-06-04 Thread Warner Losh
On Mon, Jun 4, 2018 at 9:13 AM, Rodney W. Grimes <
free...@pdx.rh.cn85.dnsmgr.net> wrote:

> > On  2 Jun, Rodney W. Grimes wrote:
> > >> Author: eadler
> > >> Date: Sat Jun  2 22:06:27 2018
> > >> New Revision: 334543
> > >> URL: https://svnweb.freebsd.org/changeset/base/334543
> > >>
> > >> Log:
> > >>   top(1): chdir to / as init; remove unneeded comment
> > >>
> > >>   - chdir to / to allow unmounting of wd
> > >>   - remove warning about running top(1) as setuid. If this is a
> concern we
> > >>   should just drop privs instead.
> > >>
> > >> Modified:
> > >>   head/usr.bin/top/machine.c
> > >>   head/usr.bin/top/top.c
> > >>
> > >> Modified: head/usr.bin/top/machine.c
> > >> 
> ==
> > >> --- head/usr.bin/top/machine.c Sat Jun  2 21:50:00 2018
> (r334542)
> > >> +++ head/usr.bin/top/machine.c Sat Jun  2 22:06:27 2018
> (r334543)
> > >> @@ -1613,11 +1613,6 @@ compare_ivcsw(const void *arg1, const void
> *arg2)
> > >>  /*
> > >>   * proc_owner(pid) - returns the uid that owns process "pid", or -1
> if
> > >>   *the process does not exist.
> > >> - *It is EXTREMELY IMPORTANT that this function work
> correctly.
> > >> - *If top runs setuid root (as in SVR4), then this
> function
> > >> - *is the only thing that stands in the way of a
> serious
> > >> - *security problem.  It validates requests for the
> "kill"
> > >> - *and "renice" commands.
> > >>   */
> > >>
> > >>  int
> > >>
> > >> Modified: head/usr.bin/top/top.c
> > >> 
> ==
> > >> --- head/usr.bin/top/top.c Sat Jun  2 21:50:00 2018(r334542)
> > >> +++ head/usr.bin/top/top.c Sat Jun  2 22:06:27 2018(r334543)
> > >> @@ -260,6 +260,15 @@ main(int argc, char *argv[])
> > >>  #define CMD_order 26
> > >>  #define CMD_pid   27
> > >>
> > >> +/*
> > >> + * Since top(1) is often long running and
> > >> + * doesn't typically care about where its running from
> > >> + * chdir to the root to allow unmounting of its
> > >> + * originall wd. Failure is alright as this is
> > >> + * just a courtesy for users.
> > >> + */
> > >> +chdir("/");
> > >> +
> > >
> > > Bad side effect of doing that is it is not hard to get a "core"
> > > from top when run as a user, as it is going to try to write
> > > to /, and it probably does not have permission for that.
> > >
> > > Better might be a cd to /tmp, or /var/tmp, which are usually
> > > hard to unmount for these reasons anyway.
> >
> > Unless you start top using the exec shell builtin, the shell that you
> > use to launch top will also be long running and will also prevent its
> > $cwd from being unmounted.
>
> Thats a good point, so that makes the chdir worthless.


Turns out it wasn't completely useless, but the usefulness ended before
FreeBSD 1.0 was released.


> > If you do use exec, then you will get logged out when you kill top ...
>
> :-(.
>
> The long standing (30 years) solution is to use lsof and find
> the processes that have cwd's in what ever it is you want to
> unmount.
>

30 years is a bit too long. lsof didn't exist until 1991. :) The issues
that prompted top to cd to / didn't get fixed until SysVr4 / early BSD
kernels in the early 90s, and didn't make it into some vendor code until
the mid 90's.


> Special casing top(1) is just a none solution to the
> can not unmount foo problem.
>

True. It used to be critically important to do.   Now, it's irrelevant. I
posted a longer version why after doing some research. Basically, through
the early System V releases, rebooting was weird and long running processes
had to take actions to ensure they didn't accidentally hold references to
non / filesystems. That did get fixed by the late 80's / early 90's, so
it's pointless these days. I'd misremembered the details over the weekend,
so forget I said it was a good change :)

Warner
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"


Re: svn commit: r334543 - head/usr.bin/top

2018-06-04 Thread Ian Lepore
On Sun, 2018-06-03 at 14:33 -0700, Rodney W. Grimes wrote:
> > 
> > On Sat, Jun 2, 2018 at 11:08 PM, Eitan Adler  wrote:
> > 
> > > 
> > > On 2 June 2018 at 16:56, Rodney W. Grimes
> > >  wrote:
> > > > 
> > > > > 
> > > > > Author: eadler
> > > > > Date: Sat Jun  2 22:06:27 2018
> > > > > New Revision: 334543
> > > > > URL: https://svnweb.freebsd.org/changeset/base/334543
> > > > > 
> > > > > Log:
> > > > >   top(1): chdir to / as init; remove unneeded comment
> > > > > 
> > > > >   - chdir to / to allow unmounting of wd
> > > > >   - remove warning about running top(1) as setuid. If this is a 
> > > > > concern
> > > we
> > > > 
> > > > > 
> > > > >   should just drop privs instead.
> > > > > 
> > > > > Modified:
> > > > >   head/usr.bin/top/machine.c
> > > > >   head/usr.bin/top/top.c
> > > > > 
> > > > > Modified: head/usr.bin/top/machine.c
> > > > > 
> > > ==
> > > > 
> > > > > 
> > > > > --- head/usr.bin/top/machine.cSat Jun  2 21:50:00 2018
> > > (r334542)
> > > > 
> > > > > 
> > > > > +++ head/usr.bin/top/machine.cSat Jun  2 22:06:27 2018
> > > (r334543)
> > > > 
> > > > > 
> > > > > @@ -1613,11 +1613,6 @@ compare_ivcsw(const void *arg1, const void 
> > > > > *arg2)
> > > > >  /*
> > > > >   * proc_owner(pid) - returns the uid that owns process "pid", or -1 
> > > > > if
> > > > >   *   the process does not exist.
> > > > > - *   It is EXTREMELY IMPORTANT that this function work
> > > correctly.
> > > > 
> > > > > 
> > > > > - *   If top runs setuid root (as in SVR4), then this function
> > > > > - *   is the only thing that stands in the way of a serious
> > > > > - *   security problem.  It validates requests for the "kill"
> > > > > - *   and "renice" commands.
> > > > >   */
> > > > > 
> > > > >  int
> > > > > 
> > > > > Modified: head/usr.bin/top/top.c
> > > > > 
> > > ==
> > > > 
> > > > > 
> > > > > --- head/usr.bin/top/top.cSat Jun  2 21:50:00 2018
> > > > > (r334542)
> > > > > +++ head/usr.bin/top/top.cSat Jun  2 22:06:27 2018
> > > > > (r334543)
> > > > > @@ -260,6 +260,15 @@ main(int argc, char *argv[])
> > > > >  #define CMD_order26
> > > > >  #define CMD_pid  27
> > > > > 
> > > > > +/*
> > > > > + * Since top(1) is often long running and
> > > > > + * doesn't typically care about where its running from
> > > > > + * chdir to the root to allow unmounting of its
> > > > > + * originall wd. Failure is alright as this is
> > > > > + * just a courtesy for users.
> > > > > + */
> > > > > +chdir("/");
> > > > > +
> > > > Bad side effect of doing that is it is not hard to get a "core"
> > > > from top when run as a user, as it is going to try to write
> > > > to /, and it probably does not have permission for that.
> > > Another person made the point that other similar applications don't do
> > > this, so I just reverted it.
> > > 
> > Actually,  it was a good change.
> > 
> > I've had issues on other systems where I couldn't unmount a filesystem for
> > reasons unknown.
> lsof is your friend here.  That is the tool of choice for finding
> cwd of processes that are in directories you can not unmount.
> 

Actually, rather than lsof (which I never even bother to install
anymore), I think the newer version of this advice is to use
procstat(1) from base. For example, to see why you can't umount /foo:

  procstat -af | grep /foo

-- Ian

___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"


Re: svn commit: r334543 - head/usr.bin/top

2018-06-04 Thread Rodney W. Grimes
[ Charset UTF-8 unsupported, converting... ]
> On 04/06/2018 00:33, Rodney W. Grimes wrote:
> > lsof is your friend here.  That is the tool of choice for finding
> > cwd of processes that are in directories you can not unmount.
> 
> s/lsof/fstat -f [-m]/
> :-)

I forget we have that tool!!   Old finger memory spells it lsof no
matter what my brain says.  I even forget when I dont have lsof
installed that I can get what I want from fstat!

-- 
Rod Grimes rgri...@freebsd.org
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"


Re: svn commit: r334543 - head/usr.bin/top

2018-06-04 Thread Rodney W. Grimes
> On  2 Jun, Rodney W. Grimes wrote:
> >> Author: eadler
> >> Date: Sat Jun  2 22:06:27 2018
> >> New Revision: 334543
> >> URL: https://svnweb.freebsd.org/changeset/base/334543
> >> 
> >> Log:
> >>   top(1): chdir to / as init; remove unneeded comment
> >>   
> >>   - chdir to / to allow unmounting of wd
> >>   - remove warning about running top(1) as setuid. If this is a concern we
> >>   should just drop privs instead.
> >> 
> >> Modified:
> >>   head/usr.bin/top/machine.c
> >>   head/usr.bin/top/top.c
> >> 
> >> Modified: head/usr.bin/top/machine.c
> >> ==
> >> --- head/usr.bin/top/machine.c Sat Jun  2 21:50:00 2018
> >> (r334542)
> >> +++ head/usr.bin/top/machine.c Sat Jun  2 22:06:27 2018
> >> (r334543)
> >> @@ -1613,11 +1613,6 @@ compare_ivcsw(const void *arg1, const void *arg2)
> >>  /*
> >>   * proc_owner(pid) - returns the uid that owns process "pid", or -1 if
> >>   *the process does not exist.
> >> - *It is EXTREMELY IMPORTANT that this function work 
> >> correctly.
> >> - *If top runs setuid root (as in SVR4), then this function
> >> - *is the only thing that stands in the way of a serious
> >> - *security problem.  It validates requests for the "kill"
> >> - *and "renice" commands.
> >>   */
> >>  
> >>  int
> >> 
> >> Modified: head/usr.bin/top/top.c
> >> ==
> >> --- head/usr.bin/top/top.c Sat Jun  2 21:50:00 2018(r334542)
> >> +++ head/usr.bin/top/top.c Sat Jun  2 22:06:27 2018(r334543)
> >> @@ -260,6 +260,15 @@ main(int argc, char *argv[])
> >>  #define CMD_order 26
> >>  #define CMD_pid   27
> >>  
> >> +/*
> >> + * Since top(1) is often long running and
> >> + * doesn't typically care about where its running from
> >> + * chdir to the root to allow unmounting of its
> >> + * originall wd. Failure is alright as this is
> >> + * just a courtesy for users.
> >> + */
> >> +chdir("/");
> >> +
> > 
> > Bad side effect of doing that is it is not hard to get a "core"
> > from top when run as a user, as it is going to try to write
> > to /, and it probably does not have permission for that.
> > 
> > Better might be a cd to /tmp, or /var/tmp, which are usually
> > hard to unmount for these reasons anyway.
> 
> Unless you start top using the exec shell builtin, the shell that you
> use to launch top will also be long running and will also prevent its
> $cwd from being unmounted.

Thats a good point, so that makes the chdir worthless.

> If you do use exec, then you will get logged out when you kill top ...

:-(.

The long standing (30 years) solution is to use lsof and find 
the processes that have cwd's in what ever it is you want to
unmount.

Special casing top(1) is just a none solution to the
can not unmount foo problem.


-- 
Rod Grimes rgri...@freebsd.org
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"


Re: svn commit: r334543 - head/usr.bin/top

2018-06-04 Thread Andriy Gapon
On 04/06/2018 00:33, Rodney W. Grimes wrote:
> lsof is your friend here.  That is the tool of choice for finding
> cwd of processes that are in directories you can not unmount.

s/lsof/fstat -f [-m]/
:-)

-- 
Andriy Gapon
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"


Re: svn commit: r334543 - head/usr.bin/top

2018-06-03 Thread Eugene Grosbein
04.06.2018 12:07, Cy Schubert wrote:

> In message <5b14c64b.2070...@grosbein.net>, Eugene Grosbein writes:

>> Bad side effect of doing that is it is not hard to get a "core"
>> from top when run as a user, as it is going to try to write
>> to /, and it probably does not have permission for that.
>>
>> We already have global sysctl kern.corefile that can be changed to /var/tmp/%
>> N.core
>>
>> Perhaps, a kernel could take a look to process environment to something like
>> KERN_COREFILE variable for an override of that sysctl?
> 
> Only if the file doesn't exist and the lowest level directory is 
> writable by UID. Even then if any directory within the path is not 
> searchable by UID it should be disallowed. Otherwise it would be a CVE.

AFAIK all security checks are in place already for sysctl kern.corefile having 
default value
relative to current working directory of the process (user).



___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"


Re: svn commit: r334543 - head/usr.bin/top

2018-06-03 Thread Cy Schubert
In message <5b14c64b.2070...@grosbein.net>, Eugene Grosbein writes:
> 04.06.2018 4:33, Rodney W. Grimes wrote:
>
>  Bad side effect of doing that is it is not hard to get a "core"
>  from top when run as a user, as it is going to try to write
>  to /, and it probably does not have permission for that.
>
> We already have global sysctl kern.corefile that can be changed to /var/tmp/%
> N.core
>
> Perhaps, a kernel could take a look to process environment to something like
> KERN_COREFILE variable for an override of that sysctl?
>

Only if the file doesn't exist and the lowest level directory is 
writable by UID. Even then if any directory within the path is not 
searchable by UID it should be disallowed. Otherwise it would be a CVE.


-- 
Cheers,
Cy Schubert 
FreeBSD UNIX: Web:  http://www.FreeBSD.org

The need of the many outweighs the greed of the few.


___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"


Re: svn commit: r334543 - head/usr.bin/top

2018-06-03 Thread Don Lewis
On  2 Jun, Rodney W. Grimes wrote:
>> Author: eadler
>> Date: Sat Jun  2 22:06:27 2018
>> New Revision: 334543
>> URL: https://svnweb.freebsd.org/changeset/base/334543
>> 
>> Log:
>>   top(1): chdir to / as init; remove unneeded comment
>>   
>>   - chdir to / to allow unmounting of wd
>>   - remove warning about running top(1) as setuid. If this is a concern we
>>   should just drop privs instead.
>> 
>> Modified:
>>   head/usr.bin/top/machine.c
>>   head/usr.bin/top/top.c
>> 
>> Modified: head/usr.bin/top/machine.c
>> ==
>> --- head/usr.bin/top/machine.c   Sat Jun  2 21:50:00 2018
>> (r334542)
>> +++ head/usr.bin/top/machine.c   Sat Jun  2 22:06:27 2018
>> (r334543)
>> @@ -1613,11 +1613,6 @@ compare_ivcsw(const void *arg1, const void *arg2)
>>  /*
>>   * proc_owner(pid) - returns the uid that owns process "pid", or -1 if
>>   *  the process does not exist.
>> - *  It is EXTREMELY IMPORTANT that this function work correctly.
>> - *  If top runs setuid root (as in SVR4), then this function
>> - *  is the only thing that stands in the way of a serious
>> - *  security problem.  It validates requests for the "kill"
>> - *  and "renice" commands.
>>   */
>>  
>>  int
>> 
>> Modified: head/usr.bin/top/top.c
>> ==
>> --- head/usr.bin/top/top.c   Sat Jun  2 21:50:00 2018(r334542)
>> +++ head/usr.bin/top/top.c   Sat Jun  2 22:06:27 2018(r334543)
>> @@ -260,6 +260,15 @@ main(int argc, char *argv[])
>>  #define CMD_order   26
>>  #define CMD_pid 27
>>  
>> +/*
>> + * Since top(1) is often long running and
>> + * doesn't typically care about where its running from
>> + * chdir to the root to allow unmounting of its
>> + * originall wd. Failure is alright as this is
>> + * just a courtesy for users.
>> + */
>> +chdir("/");
>> +
> 
> Bad side effect of doing that is it is not hard to get a "core"
> from top when run as a user, as it is going to try to write
> to /, and it probably does not have permission for that.
> 
> Better might be a cd to /tmp, or /var/tmp, which are usually
> hard to unmount for these reasons anyway.

Unless you start top using the exec shell builtin, the shell that you
use to launch top will also be long running and will also prevent its
$cwd from being unmounted.

If you do use exec, then you will get logged out when you kill top ...



___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"


Re: svn commit: r334543 - head/usr.bin/top

2018-06-03 Thread Eugene Grosbein
04.06.2018 4:33, Rodney W. Grimes wrote:

 Bad side effect of doing that is it is not hard to get a "core"
 from top when run as a user, as it is going to try to write
 to /, and it probably does not have permission for that.

We already have global sysctl kern.corefile that can be changed to 
/var/tmp/%N.core

Perhaps, a kernel could take a look to process environment to something like
KERN_COREFILE variable for an override of that sysctl?
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"


Re: svn commit: r334543 - head/usr.bin/top

2018-06-03 Thread Rodney W. Grimes
> On Sat, Jun 2, 2018 at 11:08 PM, Eitan Adler  wrote:
> 
> > On 2 June 2018 at 16:56, Rodney W. Grimes
> >  wrote:
> > >> Author: eadler
> > >> Date: Sat Jun  2 22:06:27 2018
> > >> New Revision: 334543
> > >> URL: https://svnweb.freebsd.org/changeset/base/334543
> > >>
> > >> Log:
> > >>   top(1): chdir to / as init; remove unneeded comment
> > >>
> > >>   - chdir to / to allow unmounting of wd
> > >>   - remove warning about running top(1) as setuid. If this is a concern
> > we
> > >>   should just drop privs instead.
> > >>
> > >> Modified:
> > >>   head/usr.bin/top/machine.c
> > >>   head/usr.bin/top/top.c
> > >>
> > >> Modified: head/usr.bin/top/machine.c
> > >> 
> > ==
> > >> --- head/usr.bin/top/machine.cSat Jun  2 21:50:00 2018
> > (r334542)
> > >> +++ head/usr.bin/top/machine.cSat Jun  2 22:06:27 2018
> > (r334543)
> > >> @@ -1613,11 +1613,6 @@ compare_ivcsw(const void *arg1, const void *arg2)
> > >>  /*
> > >>   * proc_owner(pid) - returns the uid that owns process "pid", or -1 if
> > >>   *   the process does not exist.
> > >> - *   It is EXTREMELY IMPORTANT that this function work
> > correctly.
> > >> - *   If top runs setuid root (as in SVR4), then this function
> > >> - *   is the only thing that stands in the way of a serious
> > >> - *   security problem.  It validates requests for the "kill"
> > >> - *   and "renice" commands.
> > >>   */
> > >>
> > >>  int
> > >>
> > >> Modified: head/usr.bin/top/top.c
> > >> 
> > ==
> > >> --- head/usr.bin/top/top.cSat Jun  2 21:50:00 2018(r334542)
> > >> +++ head/usr.bin/top/top.cSat Jun  2 22:06:27 2018(r334543)
> > >> @@ -260,6 +260,15 @@ main(int argc, char *argv[])
> > >>  #define CMD_order26
> > >>  #define CMD_pid  27
> > >>
> > >> +/*
> > >> + * Since top(1) is often long running and
> > >> + * doesn't typically care about where its running from
> > >> + * chdir to the root to allow unmounting of its
> > >> + * originall wd. Failure is alright as this is
> > >> + * just a courtesy for users.
> > >> + */
> > >> +chdir("/");
> > >> +
> > >
> > > Bad side effect of doing that is it is not hard to get a "core"
> > > from top when run as a user, as it is going to try to write
> > > to /, and it probably does not have permission for that.
> >
> > Another person made the point that other similar applications don't do
> > this, so I just reverted it.
> >
> 
> Actually,  it was a good change.
> 
> I've had issues on other systems where I couldn't unmount a filesystem for
> reasons unknown.

lsof is your friend here.  That is the tool of choice for finding
cwd of processes that are in directories you can not unmount.

> Not being able to take a core dump for top is a secondary concern: that can
> easily be worked around by rebuilding top. And chdiring to a different
> location defeats the point of chdir to "/".

Rebuilding top is *not* an "easy" solution, that assumes srcs are installed,
and the problem can easily be retriggered.

One could argue we should do this to lots of binaries based on the
long running nature, like tail (-f).

You would also have to know that top didnt core for you cause it could
not write to /, and that is not going to be a well known fact.

True long running things like daemons often do cd to well known places.
 
> While we do have forced unmounts, I'm hesitant to use them unless I know
> for sure why I need to force it.

That is an administrative issue easily solved with lsof.


-- 
Rod Grimes rgri...@freebsd.org
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"


Re: svn commit: r334543 - head/usr.bin/top

2018-06-03 Thread Warner Losh
On Sat, Jun 2, 2018 at 11:08 PM, Eitan Adler  wrote:

> On 2 June 2018 at 16:56, Rodney W. Grimes
>  wrote:
> >> Author: eadler
> >> Date: Sat Jun  2 22:06:27 2018
> >> New Revision: 334543
> >> URL: https://svnweb.freebsd.org/changeset/base/334543
> >>
> >> Log:
> >>   top(1): chdir to / as init; remove unneeded comment
> >>
> >>   - chdir to / to allow unmounting of wd
> >>   - remove warning about running top(1) as setuid. If this is a concern
> we
> >>   should just drop privs instead.
> >>
> >> Modified:
> >>   head/usr.bin/top/machine.c
> >>   head/usr.bin/top/top.c
> >>
> >> Modified: head/usr.bin/top/machine.c
> >> 
> ==
> >> --- head/usr.bin/top/machine.cSat Jun  2 21:50:00 2018
> (r334542)
> >> +++ head/usr.bin/top/machine.cSat Jun  2 22:06:27 2018
> (r334543)
> >> @@ -1613,11 +1613,6 @@ compare_ivcsw(const void *arg1, const void *arg2)
> >>  /*
> >>   * proc_owner(pid) - returns the uid that owns process "pid", or -1 if
> >>   *   the process does not exist.
> >> - *   It is EXTREMELY IMPORTANT that this function work
> correctly.
> >> - *   If top runs setuid root (as in SVR4), then this function
> >> - *   is the only thing that stands in the way of a serious
> >> - *   security problem.  It validates requests for the "kill"
> >> - *   and "renice" commands.
> >>   */
> >>
> >>  int
> >>
> >> Modified: head/usr.bin/top/top.c
> >> 
> ==
> >> --- head/usr.bin/top/top.cSat Jun  2 21:50:00 2018(r334542)
> >> +++ head/usr.bin/top/top.cSat Jun  2 22:06:27 2018(r334543)
> >> @@ -260,6 +260,15 @@ main(int argc, char *argv[])
> >>  #define CMD_order26
> >>  #define CMD_pid  27
> >>
> >> +/*
> >> + * Since top(1) is often long running and
> >> + * doesn't typically care about where its running from
> >> + * chdir to the root to allow unmounting of its
> >> + * originall wd. Failure is alright as this is
> >> + * just a courtesy for users.
> >> + */
> >> +chdir("/");
> >> +
> >
> > Bad side effect of doing that is it is not hard to get a "core"
> > from top when run as a user, as it is going to try to write
> > to /, and it probably does not have permission for that.
>
> Another person made the point that other similar applications don't do
> this, so I just reverted it.
>

Actually,  it was a good change.

I've had issues on other systems where I couldn't unmount a filesystem for
reasons unknown.

Not being able to take a core dump for top is a secondary concern: that can
easily be worked around by rebuilding top. And chdiring to a different
location defeats the point of chdir to "/".

While we do have forced unmounts, I'm hesitant to use them unless I know
for sure why I need to force it.

Warner
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"


Re: svn commit: r334543 - head/usr.bin/top

2018-06-02 Thread Eitan Adler
On 2 June 2018 at 16:56, Rodney W. Grimes
 wrote:
>> Author: eadler
>> Date: Sat Jun  2 22:06:27 2018
>> New Revision: 334543
>> URL: https://svnweb.freebsd.org/changeset/base/334543
>>
>> Log:
>>   top(1): chdir to / as init; remove unneeded comment
>>
>>   - chdir to / to allow unmounting of wd
>>   - remove warning about running top(1) as setuid. If this is a concern we
>>   should just drop privs instead.
>>
>> Modified:
>>   head/usr.bin/top/machine.c
>>   head/usr.bin/top/top.c
>>
>> Modified: head/usr.bin/top/machine.c
>> ==
>> --- head/usr.bin/top/machine.cSat Jun  2 21:50:00 2018
>> (r334542)
>> +++ head/usr.bin/top/machine.cSat Jun  2 22:06:27 2018
>> (r334543)
>> @@ -1613,11 +1613,6 @@ compare_ivcsw(const void *arg1, const void *arg2)
>>  /*
>>   * proc_owner(pid) - returns the uid that owns process "pid", or -1 if
>>   *   the process does not exist.
>> - *   It is EXTREMELY IMPORTANT that this function work correctly.
>> - *   If top runs setuid root (as in SVR4), then this function
>> - *   is the only thing that stands in the way of a serious
>> - *   security problem.  It validates requests for the "kill"
>> - *   and "renice" commands.
>>   */
>>
>>  int
>>
>> Modified: head/usr.bin/top/top.c
>> ==
>> --- head/usr.bin/top/top.cSat Jun  2 21:50:00 2018(r334542)
>> +++ head/usr.bin/top/top.cSat Jun  2 22:06:27 2018(r334543)
>> @@ -260,6 +260,15 @@ main(int argc, char *argv[])
>>  #define CMD_order26
>>  #define CMD_pid  27
>>
>> +/*
>> + * Since top(1) is often long running and
>> + * doesn't typically care about where its running from
>> + * chdir to the root to allow unmounting of its
>> + * originall wd. Failure is alright as this is
>> + * just a courtesy for users.
>> + */
>> +chdir("/");
>> +
>
> Bad side effect of doing that is it is not hard to get a "core"
> from top when run as a user, as it is going to try to write
> to /, and it probably does not have permission for that.

Another person made the point that other similar applications don't do
this, so I just reverted it.

thanks!


-- 
Eitan Adler
Source, Ports, Doc committer
Bugmeister, Ports Security teams
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"


Re: svn commit: r334543 - head/usr.bin/top

2018-06-02 Thread Rodney W. Grimes
> > Author: eadler
> > Date: Sat Jun  2 22:06:27 2018
> > New Revision: 334543
> > URL: https://svnweb.freebsd.org/changeset/base/334543
> > 
> > Log:
> >   top(1): chdir to / as init; remove unneeded comment
> >   
> >   - chdir to / to allow unmounting of wd
> >   - remove warning about running top(1) as setuid. If this is a concern we
> >   should just drop privs instead.
> > 
> > Modified:
> >   head/usr.bin/top/machine.c
> >   head/usr.bin/top/top.c
> > 
> > Modified: head/usr.bin/top/machine.c
> > ==
> > --- head/usr.bin/top/machine.c  Sat Jun  2 21:50:00 2018
> > (r334542)
> > +++ head/usr.bin/top/machine.c  Sat Jun  2 22:06:27 2018
> > (r334543)
> > @@ -1613,11 +1613,6 @@ compare_ivcsw(const void *arg1, const void *arg2)
> >  /*
> >   * proc_owner(pid) - returns the uid that owns process "pid", or -1 if
> >   * the process does not exist.
> > - * It is EXTREMELY IMPORTANT that this function work correctly.
> > - * If top runs setuid root (as in SVR4), then this function
> > - * is the only thing that stands in the way of a serious
> > - * security problem.  It validates requests for the "kill"
> > - * and "renice" commands.
> >   */
> >  
> >  int
> > 
> > Modified: head/usr.bin/top/top.c
> > ==
> > --- head/usr.bin/top/top.c  Sat Jun  2 21:50:00 2018(r334542)
> > +++ head/usr.bin/top/top.c  Sat Jun  2 22:06:27 2018(r334543)
> > @@ -260,6 +260,15 @@ main(int argc, char *argv[])
> >  #define CMD_order  26
> >  #define CMD_pid27
> >  
> > +/*
> > + * Since top(1) is often long running and
> > + * doesn't typically care about where its running from
> > + * chdir to the root to allow unmounting of its
> > + * originall wd. Failure is alright as this is
> > + * just a courtesy for users.
> > + */
> > +chdir("/");
> > +
> 
> Bad side effect of doing that is it is not hard to get a "core"
 ^^^ now
> from top when run as a user, as it is going to try to write
> to /, and it probably does not have permission for that.
> 
> Better might be a cd to /tmp, or /var/tmp, which are usually
> hard to unmount for these reasons anyway.
> 
> >  /* set the buffer for stdout */
> >  #ifdef DEBUG
> >  extern FILE *debug;
> > 
> > 
> 
> -- 
> Rod Grimes rgri...@freebsd.org
> 
> 

-- 
Rod Grimes rgri...@freebsd.org
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"


Re: svn commit: r334543 - head/usr.bin/top

2018-06-02 Thread Rodney W. Grimes
> Author: eadler
> Date: Sat Jun  2 22:06:27 2018
> New Revision: 334543
> URL: https://svnweb.freebsd.org/changeset/base/334543
> 
> Log:
>   top(1): chdir to / as init; remove unneeded comment
>   
>   - chdir to / to allow unmounting of wd
>   - remove warning about running top(1) as setuid. If this is a concern we
>   should just drop privs instead.
> 
> Modified:
>   head/usr.bin/top/machine.c
>   head/usr.bin/top/top.c
> 
> Modified: head/usr.bin/top/machine.c
> ==
> --- head/usr.bin/top/machine.cSat Jun  2 21:50:00 2018
> (r334542)
> +++ head/usr.bin/top/machine.cSat Jun  2 22:06:27 2018
> (r334543)
> @@ -1613,11 +1613,6 @@ compare_ivcsw(const void *arg1, const void *arg2)
>  /*
>   * proc_owner(pid) - returns the uid that owns process "pid", or -1 if
>   *   the process does not exist.
> - *   It is EXTREMELY IMPORTANT that this function work correctly.
> - *   If top runs setuid root (as in SVR4), then this function
> - *   is the only thing that stands in the way of a serious
> - *   security problem.  It validates requests for the "kill"
> - *   and "renice" commands.
>   */
>  
>  int
> 
> Modified: head/usr.bin/top/top.c
> ==
> --- head/usr.bin/top/top.cSat Jun  2 21:50:00 2018(r334542)
> +++ head/usr.bin/top/top.cSat Jun  2 22:06:27 2018(r334543)
> @@ -260,6 +260,15 @@ main(int argc, char *argv[])
>  #define CMD_order26
>  #define CMD_pid  27
>  
> +/*
> + * Since top(1) is often long running and
> + * doesn't typically care about where its running from
> + * chdir to the root to allow unmounting of its
> + * originall wd. Failure is alright as this is
> + * just a courtesy for users.
> + */
> +chdir("/");
> +

Bad side effect of doing that is it is not hard to get a "core"
from top when run as a user, as it is going to try to write
to /, and it probably does not have permission for that.

Better might be a cd to /tmp, or /var/tmp, which are usually
hard to unmount for these reasons anyway.

>  /* set the buffer for stdout */
>  #ifdef DEBUG
>  extern FILE *debug;
> 
> 

-- 
Rod Grimes rgri...@freebsd.org
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"


svn commit: r334543 - head/usr.bin/top

2018-06-02 Thread Eitan Adler
Author: eadler
Date: Sat Jun  2 22:06:27 2018
New Revision: 334543
URL: https://svnweb.freebsd.org/changeset/base/334543

Log:
  top(1): chdir to / as init; remove unneeded comment
  
  - chdir to / to allow unmounting of wd
  - remove warning about running top(1) as setuid. If this is a concern we
  should just drop privs instead.

Modified:
  head/usr.bin/top/machine.c
  head/usr.bin/top/top.c

Modified: head/usr.bin/top/machine.c
==
--- head/usr.bin/top/machine.c  Sat Jun  2 21:50:00 2018(r334542)
+++ head/usr.bin/top/machine.c  Sat Jun  2 22:06:27 2018(r334543)
@@ -1613,11 +1613,6 @@ compare_ivcsw(const void *arg1, const void *arg2)
 /*
  * proc_owner(pid) - returns the uid that owns process "pid", or -1 if
  * the process does not exist.
- * It is EXTREMELY IMPORTANT that this function work correctly.
- * If top runs setuid root (as in SVR4), then this function
- * is the only thing that stands in the way of a serious
- * security problem.  It validates requests for the "kill"
- * and "renice" commands.
  */
 
 int

Modified: head/usr.bin/top/top.c
==
--- head/usr.bin/top/top.c  Sat Jun  2 21:50:00 2018(r334542)
+++ head/usr.bin/top/top.c  Sat Jun  2 22:06:27 2018(r334543)
@@ -260,6 +260,15 @@ main(int argc, char *argv[])
 #define CMD_order  26
 #define CMD_pid27
 
+/*
+ * Since top(1) is often long running and
+ * doesn't typically care about where its running from
+ * chdir to the root to allow unmounting of its
+ * originall wd. Failure is alright as this is
+ * just a courtesy for users.
+ */
+chdir("/");
+
 /* set the buffer for stdout */
 #ifdef DEBUG
 extern FILE *debug;
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"