From owner-svn-src-head@freebsd.org Sun Jun 3 21:34:04 2018 Return-Path: Delivered-To: svn-src-head@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 4A3FAFDF225; Sun, 3 Jun 2018 21:34:04 +0000 (UTC) (envelope-from freebsd@pdx.rh.CN85.dnsmgr.net) Received: from pdx.rh.CN85.dnsmgr.net (br1.CN84in.dnsmgr.net [69.59.192.140]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id 522CC7E5D9; Sun, 3 Jun 2018 21:34:03 +0000 (UTC) (envelope-from freebsd@pdx.rh.CN85.dnsmgr.net) Received: from pdx.rh.CN85.dnsmgr.net (localhost [127.0.0.1]) by pdx.rh.CN85.dnsmgr.net (8.13.3/8.13.3) with ESMTP id w53LXvGJ092880; Sun, 3 Jun 2018 14:33:57 -0700 (PDT) (envelope-from freebsd@pdx.rh.CN85.dnsmgr.net) Received: (from freebsd@localhost) by pdx.rh.CN85.dnsmgr.net (8.13.3/8.13.3/Submit) id w53LXvVY092879; Sun, 3 Jun 2018 14:33:57 -0700 (PDT) (envelope-from freebsd) From: "Rodney W. Grimes" Message-Id: <201806032133.w53LXvVY092879@pdx.rh.CN85.dnsmgr.net> Subject: Re: svn commit: r334543 - head/usr.bin/top In-Reply-To: To: Warner Losh Date: Sun, 3 Jun 2018 14:33:57 -0700 (PDT) CC: Eitan Adler , "Rodney W. Grimes" , src-committers , svn-src-all@freebsd.org, svn-src-head@freebsd.org Reply-To: rgrimes@freebsd.org X-Mailer: ELM [version 2.4ME+ PL121h (25)] MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Type: text/plain; charset=US-ASCII X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.26 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 03 Jun 2018 21:34:04 -0000 > 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.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