From owner-freebsd-current@FreeBSD.ORG Thu Oct 18 13:24:40 2007 Return-Path: Delivered-To: freebsd-current@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 0DD0616A41B for ; Thu, 18 Oct 2007 13:24:40 +0000 (UTC) (envelope-from askbill@conducive.net) Received: from conducive.net (lindfield.ch [203.194.153.81]) by mx1.freebsd.org (Postfix) with ESMTP id B9D3A13C458 for ; Thu, 18 Oct 2007 13:24:38 +0000 (UTC) (envelope-from askbill@conducive.net) Received: from cm218-253-81-177.hkcable.com.hk ([218.253.81.177]:60087 helo=pb.local) by conducive.net with esmtpsa (TLSv1:AES256-SHA:256) (Exim 4.63 (FreeBSD)) (envelope-from ) id 1IiVMb-0005yN-BG for freebsd-current@freebsd.org; Thu, 18 Oct 2007 13:24:37 +0000 Message-ID: <47175E94.6090309@conducive.net> Date: Thu, 18 Oct 2007 09:24:36 -0400 From: =?UTF-8?B?6Z+T5a625qiZIEJpbGwgSGFja2Vy?= User-Agent: Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.8.1.2) Gecko/20070221 SeaMonkey/1.1.1 MIME-Version: 1.0 To: freebsd-current@freebsd.org References: <00bd01c810ec$10371230$0c00a8c0@Artem> <8cb6106e0710171143m3dff7546o457192ede76e6598@mail.gmail.com> <012c01c810f3$aafeecf0$0c00a8c0@Artem> <20071017193615.GO9006@server.vk2pj.dyndns.org> <471667DB.1010601@conducive.net> <47170FF1.3050602@moneybookers.com> <471746C7.20306@conducive.net><47174BE4.6020300@moneybookers.com> <4717523E.1000403@conducive.net> <010f01c81184$cd375550$0c00a8c0@Artem> In-Reply-To: <010f01c81184$cd375550$0c00a8c0@Artem> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: Broken su in current - trying to fix myself, help needed! X-BeenThere: freebsd-current@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Discussions about the use of FreeBSD-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 18 Oct 2007 13:24:40 -0000 Artem Kuchin wrote: >> What Artem is seeing is not (yet) a 'bug' in su in my mind. >> > > You missed reply from David Xu in the list on this matter. > No, I saw it.. I just have a different 'take' on it. Two actually.. s.b. > To me there is CLEARLY a bug in the source code. It tried > to get group of already dead process. > > Here is quote from my and David's letters: > >> The weird thing is that if i just comment out those lines like this >> >> /* child_pgrp = getpgid(child_pid); >> if (tcgetpgrp(STDERR_FILENO) == child_pgrp) */ >> tcsetpgrp(STDERR_FILENO, getpgrp()); >> >> su starts working again just fine. >> >> Any idea why getpgid fails and why tcgetpgrp return 100000 (always the >> same >> number)? What will brak if i leave these lines commented? >> >> -- >> Regards, >> Artem > > file su.c, line 472 may be incorrect since line 456 is a while loop > which only > exits if child process is exited. just remove line 472 and 473 to see if > problem > is fixed. > > -- > Artem > Agree that *seems to fix* the immediate issue. But - it may be treating the symptom, not the underlying problem. Specifically - *why* was it coded that way to begin with? I'm sure 'su' has had lots of peer review and rewrite since V1 ATT UNIX. T'would be 'of interest', given how much re-write or auditing has been done to two such, to see how OpenBSD and DragonFlyBSD have altered it. Or if they have. Perhaps I'm overly conservative, but one has to ask - should there be more selective code *added* to handle the case of a missing child process pid and carry on, rather than removing that snippet of code? "Working again just fine' has yet to be proven to not break something else under some other set of circumstances. Or open an exploitable hole. JMNSHO, but 'su' is too important, in too many places to be trifled with lightly. So - your query 'What will break if..' is a good starting point. More review and testing is in order. Best, Bill