Re: [PATCH net-next 2/3] tcp: convert do_tcp_getsockopt() to sockopt_t

[Stanislav Fomichev]

On 11/12, Breno Leitao wrote:
> Convert the SOL_TCP switch to sockopt_t. The TCP-AO helpers already
> take one, so they get it directly and the tcp_ao_get_sockptr() bridge
> goes away.
> 
> The options that re-read optlen to get past the sizeof(int) clamp at
> the top take it from sopt->optlen instead, so optlen is read once.
> 
> Only a user buffer reaches TCP_ZEROCOPY_RECEIVE: BPF's sol_tcp_sockopt()
> does not take it, and it maps pages into the caller's mm anyway. It
> keeps checking the bytes past its struct with check_zeroed_user(), now
> on iter_in.ubuf, and a kernel buffer gets -EINVAL.
> 
> tcp_getsockopt() builds the sockopt_t with sockopt_init_user() once the
> level is known to be SOL_TCP, and writes optlen back unconditionally,
> because TCP_SAVED_SYN reports the length it needs and then fails with
> -EINVAL. This mirrors ip_getsockopt().
> 
> Every SOL_TCP getsockopt() now writes optlen, including the ones that
> did not before, such as TCP_REPAIR_WINDOW, TCP_ZEROCOPY_RECEIVE and the
> error returns. An unwritable optlen word therefore turns them into
> -EFAULT.
> 
> The others stored optlen before optval, so an unwritable optlen word
> stopped optval from being written. Now optval is written first.
> 
> sol_tcp_sockopt() and sol_tcp_sockopt_congestion() reach
> do_tcp_getsockopt() through sol_tcp_getsockopt(), which builds the
> sockopt_t with sockptr_to_sockopt() and stores optlen back, as
> sol_ip_sockopt() does.
> 
> copy_to_iter() runs the hardened usercopy check on its source, which
> copy_to_sockptr() skips for a kernel buffer. BPF programs call
> bpf_getsockopt() from the receive softirq too. On the vmapped IRQ
> stack, the check then takes the lock in find_vmap_area(), which is not
> softirq safe, and lockdep reports an inconsistent lock state.
> 
> sol_tcp_getsockopt() therefore calls do_tcp_getsockopt() with page
> faults disabled, which makes check_heap_object() skip that lookup.
> Commit d319f344561d ("mm: Fix copy_from_user_nofault().") added that
> guard for BPF and kprobes.
> 
> Signed-off-by: Breno Leitao <leitao@xxxxxxxxxx>

Acked-by: Stanislav Fomichev <sdf@xxxxxxxxxxx>