Re: svn commit: r334543 - head/usr.bin/top
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
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
[ 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
> 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
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
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
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
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
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
> 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
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
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
> > 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
> 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
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"