Date: Sun, 3 Jun 2018 14:33:57 -0700 (PDT) From: "Rodney W. Grimes" <freebsd@pdx.rh.CN85.dnsmgr.net> To: Warner Losh <imp@bsdimp.com> Cc: Eitan Adler <eadler@freebsd.org>, "Rodney W. Grimes" <rgrimes@freebsd.org>, src-committers <src-committers@freebsd.org>, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: Re: svn commit: r334543 - head/usr.bin/top Message-ID: <201806032133.w53LXvVY092879@pdx.rh.CN85.dnsmgr.net> In-Reply-To: <CANCZdfqEspS_h1Dy=nudZE55iHNK595g=X2CNdWPnE2grH1hsA@mail.gmail.com>
next in thread | previous in thread | raw e-mail | index | archive | help
> On Sat, Jun 2, 2018 at 11:08 PM, Eitan Adler <eadler@freebsd.org> wrote: > > > On 2 June 2018 at 16:56, Rodney W. Grimes > > <freebsd@pdx.rh.cn85.dnsmgr.net> 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. > > > > 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 rgrimes@freebsd.org
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201806032133.w53LXvVY092879>