On Mon, 17 Jun 2019 15:06:33 +0200, Davide Caratti wrote:
> > > + if (icsk->icsk_ulp_ops->get_info_size)
> > > + size += icsk->icsk_ulp_ops->get_info_size(sk);
> >
> > I don't know the diag code, is the socket locked at this point?
>
> as far as I can see, it's not. Thanks a lot for noticing this!
>
> anyway, I see a similar pattern for icsk_ca_ops: when we set the congestion
> control with do_tcp_setsockopt(), the socket is locked - but then, when 'ss'
> reads a diag request with INET_DIAG_CONG bit set, the value of
> icsk->icsk_ca_ops
> is accessed with READ_ONCE(), surrounded by rcu_read_{,un}lock().
>
> Maybe it's sufficient to do something similar, and then the socket lock can be
> optionally taken within icsk_ulp_ops->get_info(), only in case we need to
> access
> members of sk that are protected with the socket lock?
Sounds reasonable, we just need to keep that in mind as we extend TLS
code do dump more information.