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.