Re: sshd failing in jail

My apologies for the delay.  Without warning, my ISP decided to
disable the domain I was using for email and I have been busy
repairing the damage.
 
On 2009-Aug-30 17:44:52 +0300, Kostik Belousov <kostikbel_at_gmail.com> wrote:
>On Sun, Aug 30, 2009 at 09:34:54AM +1000, Peter Jeremy wrote:
>> Turns out this is a bug in the 32-bit select(2) wrapper on 64-bit
>> kernels.  The userland fd_set arguments are not wrapped but passed
>> directly to kern_select().  Unfortunately, fd_set is (effectively) an
>> array of longs which means kern_select() assumes fd_set is a multiple
>> of 8-bytes whilst userland assumes it is a multiple of 4 bytes.  As a
>> result, the kernel can over-write an extra 4 bytes of user memory.  In
>> the case of sshd, this causes part of the RSA host key to be trashed
>> when privilege separation mode is enabled.
>> 
>> This bug also affects linux emulation on amd64 and potentially affects
>> any other 64-bit kernels with 32-bit emulation modes.  I have raised
>> amd64/138318 to cover it.
>
>I do not think that we can go the proposed route, since changing the
>type of __fd_mask changes the type of fd_set. The later would not
>affect the kernel ABI, but definitely changes the ABI of any code that
>passes fd_sets.

I agree it was something of a hack.

>Also, looking closely at the issue you found, I think that copyin
>is the same problematic as copyout, since we can end up reading
>one more word then userspace supplied. This is not a problem only
>because most user code keeps fd_sets on stack.

Agreed.  I was aware that copyin() could read an extra 4 bytes but
did not work through the full implications of this - it is possible
that select(2) could unexpectedly return EFAULT.

>Could you test that the patch below fixes real sshd issue. At least,
>it passes your select test from the PR.

It works OK for me.  Please feel free to commit it.

-- 
Peter Jeremy