fibs: restrict jail_attach(2) if process fibnum >= numfibs in the jail.

[melifaro]

Summary

Currently, process fibnum is not changed in jail_attach(2) and is inherited by the other processes afterwards.
If jail has its own VNET and number of fibs in this VNET is less than in the host system, it can lead to a panic.

This diff checks if the process fibnum is still valid in the new VNET and returns error if that's not true.

Test Plan

Before:

jail -c name=bug persist vnet
sysctl net.fibs=3
setfib 2 jexec bug ping -c 1 1.1.1

PING 1.1.1 (1.1.0.1): 56 data bytes
client_loop: send disconnect: Broken pipe

...
Fatal trap 12: page fault while in kernel mode

After:

jail -c name=bug persist vnet
sysctl net.fibs=3
setfib 2 jexec bug ping -c 1 1.1.1
jexec: jail_attach(1): Operation not permitted

[glebius]

How is that possible that in your example you got EPERM, but the patch provides EINVAL?

[melifaro]

fix errcode.

[melifaro]

In D38505#876205, @glebius wrote:

How is that possible that in your example you got EPERM, but the patch provides EINVAL?

I forgot to change variable type from bool to int.

[kp]

sys/net/route/route_tables.c

176

I worried about races here, but if I'm reading sysctl_fibs() right we can only ever grow V_rt_numfibs, so that should be fine.

I might spell this

if (td->td_proc->p_fibnum < V_rt_numfibs)
    error = EINVAL;

but that's more a question of preference than a real issue to discuss.

[olivier]

Fixed the crash here:

setfib 2 jexec bug ping -c 1 1.1.1.1
jexec: jail_attach(1): Invalid argument

[melifaro]

sys/net/route/route_tables.c

176

Thank you for the review!

I worried about races here, but if I'm reading sysctl_fibs() right we can only ever grow V_rt_numfibs, so that should be fine.

Yes, it's indeed grow-only. There are quite a lot of things that need to be handled correctly to permit decreasing V_rt_numfibs.

I might spell this

if (td->td_proc->p_fibnum < V_rt_numfibs)
    error = EINVAL;

Yep, that indeed looks more readable.