Re: [PATCH] can: add protocol counter for AF_CAN sockets

[Davide Caratti <dcaratti@redhat.com>]

thanks for reviewing!

On Sun, Mar 16, 2025 at 01:36:40PM +0100, Oliver Hartkopp wrote:
> Hello Davide,
> 
> thanks for your patch!

ouch, I forgot j1939. I had a selftest for that, but I could only
test raw.

> But don't we need to take care on every place where sock_put() is called
> where sock_prot_inuse_add() has to decrease the counter?

only the last call to sock_put() needs sock_prot_inuse_add(..., -1):

[...]

> ~/linux/net/can$ git grep sock_put .
> af_can.c:               sock_put(sk);

167         if (sk->sk_prot->init)
168                 err = sk->sk_prot->init(sk);
169
170         if (err) {
171                 /* release sk on errors */
172                 sock_orphan(sk);
173                 sock_put(sk);
174                 sock->sk = NULL;
175         } else {
176                 sock_prot_inuse_add(net, sk->sk_prot, 1);
177         }

^^ this one does not need it because the 'in_use' counter is incremented
in the else branch; 

> af_can.c:               sock_put(sk);

491 static void can_rx_delete_receiver(struct rcu_head *rp)
492 {
493         struct receiver *rcv = container_of(rp, struct receiver, rcu);
494         struct sock *sk = rcv->sk;
495
496         kmem_cache_free(rcv_cache, rcv);
497         if (sk)
498                 sock_put(sk);
499 }

this one comes from can_rx_unregister(), and it's called in RCU callback - so
we can't tell if it happens before or after sock_put() in ->release().
So we probably need something smarter in case we are not sure that ->release()
is called at least once for each socket. 

> bcm.c:  sock_put(sk);
> isotp.c:        sock_put(sk);
> raw.c:  sock_put(sk);

this is  '->release()' of each protocol, that I aimed to cover in the
patch...

> j1939/socket.c: sock_put(sk);
> j1939/transport.c:      sock_put(session->sk);

... except this one, that I forgot :) 

I will send a follow-up patch soon.
Thanks!
-- 
davide