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.

Reply via email to