Broken su in current - trying to fix myself, help needed!

韓家標 Bill Hacker askbill at conducive.net
Thu Oct 18 06:59:10 PDT 2007


Artem Kuchin wrote:
> 韓家標 Bill Hacker wrote:
>> Artem Kuchin wrote:

*trimmed in the interest of brevity*

> Just MAUBE something was really badly broken in waitpid or lower, so
> it was coded this way. But as i see it is no  longer broken there and
> su is broken here.
> 
Correct. As you see it...

But asking for a pid on a process that has gone tits-up in the meanwhile is not 
a 'bug' per se.

The call cannot know that said process is gone until it makes the query.

The 'bug' is not having a way to handle an unexpected response.

That it does not matter that the process inquired after has gone walkabout in 
THIS CASE is not a guarantee that it is ALWAYS to be ignored.

Otherwise, why ask at all?

>  > 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?
> 
> Well, if you look at cvs history this  strange code was added  not so 
> long ago
> to fix something which came up back then. I think it is no longer needed.
> 

You may very well be correct.

But let's look at it that way.

A bit of *apparently* obsolete code that EITHER needs to be made able to return 
<something> and carry-on, OR determined 'for sure' to now be superfluous in 'all 
cases' and be removed.

>> JMNSHO, but 'su' is too important, in too many places to be trifled
>> with lightly.
> 
> Yes, and it su is broken now and must be fixed because it is
> important.
> 

Agreed. But among those who should look at that are (at least) the committer who 
made that change. I doubt it was done casually or without good reason.

>> So - your query 'What will break if..' is a good starting point. More 
>> review and testing is in order.
> 
> It is quite possible then commiter to su just overlooked this code
> error because they do no use su the  way some people do (like me) and
> they have never seen this bug. I have tested the cases uneder all shells 
> and
> su does not work correctly anywhere, so it is SU's fault.
> 
> So, i ask people to fix their su and play around a bit and then  submit 
> patch
> to cvs. I am not a submitter and have no clue about the procedure.
> 

We're only still 'talking' because you did a good and fast job of finding the 
lines involved. But more than 'play around a bit' will be needed.

BNL

Best,

Bill


> -- 
> Artem
> 
> _______________________________________________
> freebsd-current at freebsd.org mailing list
> http://lists.freebsd.org/mailman/listinfo/freebsd-current
> To unsubscribe, send any mail to "freebsd-current-unsubscribe at freebsd.org"
> 



More information about the freebsd-current mailing list