git: 04e967d7270e - stable/13 - Add if_try_ref() to simplify refcount handling inside epoch.

[Alexander V. Chernikov]

The branch stable/13 has been updated by melifaro:

URL: https://cgit.FreeBSD.org/src/commit/?id=04e967d7270eacada2075906cca9a03043a9609a

commit 04e967d7270eacada2075906cca9a03043a9609a
Author:     Alexander V. Chernikov <melifaro at FreeBSD.org>
AuthorDate: 2021-02-22 21:37:55 +0000
Commit:     Alexander V. Chernikov <melifaro at FreeBSD.org>
CommitDate: 2021-09-07 20:55:51 +0000

    Add if_try_ref() to simplify refcount handling inside epoch.
    
    When we have an ifp pointer and the code is running inside epoch,
     epoch guarantees the pointer will not be freed.
    However, the following case can still happen:
    
    * in thread 1 we drop to refcount=0 for ifp and schedule its deletion.
    * in thread 2 we use this ifp and reference it
    * destroy callout kicks in
    * unhappy user reports a bug
    
    This can happen with the current implementation of ifnet_byindex_ref(),
     as we're not holding any locks preventing ifnet deletion by a parallel thread.
    
    To address it, add if_try_ref(), allowing to return failure when
     referencing ifp with refcount=0.
    Additionally, enforce existing if_ref() is with KASSERT to provide a
     cleaner error in such scenarios.
    
    Finally, fix ifnet_byindex_ref() by using if_try_ref() and returning NULL
     if the latter fails.
    
    MFC after:      2 weeks
    Differential Revision:  https://reviews.freebsd.org/D28836
    
    (cherry picked from commit 7563019bc69301a382abefbac3b0fea1d876410e)
---
 sys/net/if.c     | 14 ++++++++++++--
 sys/net/if_var.h |  1 +
 2 files changed, 13 insertions(+), 2 deletions(-)

diff --git a/sys/net/if.c b/sys/net/if.c
index f6926c43ef96..aeb7230fcc9a 100644
--- a/sys/net/if.c
+++ b/sys/net/if.c
@@ -359,7 +359,8 @@ ifnet_byindex_ref(u_short idx)
 	ifp = ifnet_byindex(idx);
 	if (ifp == NULL || (ifp->if_flags & IFF_DYING))
 		return (NULL);
-	if_ref(ifp);
+	if (!if_try_ref(ifp))
+		return (NULL);
 	return (ifp);
 }
 
@@ -738,9 +739,18 @@ if_free(struct ifnet *ifp)
 void
 if_ref(struct ifnet *ifp)
 {
+	u_int old;
 
 	/* We don't assert the ifnet list lock here, but arguably should. */
-	refcount_acquire(&ifp->if_refcount);
+	old = refcount_acquire(&ifp->if_refcount);
+	KASSERT(old > 0, ("%s: ifp %p has 0 refs", __func__, ifp));
+}
+
+bool
+if_try_ref(struct ifnet *ifp)
+{
+	NET_EPOCH_ASSERT();
+	return (refcount_acquire_if_not_zero(&ifp->if_refcount));
 }
 
 void
diff --git a/sys/net/if_var.h b/sys/net/if_var.h
index 291a7781d73c..33a737880a8d 100644
--- a/sys/net/if_var.h
+++ b/sys/net/if_var.h
@@ -661,6 +661,7 @@ void	if_link_state_change(struct ifnet *, int);
 int	if_printf(struct ifnet *, const char *, ...) __printflike(2, 3);
 void	if_ref(struct ifnet *);
 void	if_rele(struct ifnet *);
+bool	if_try_ref(struct ifnet *);
 int	if_setlladdr(struct ifnet *, const u_char *, int);
 int	if_tunnel_check_nesting(struct ifnet *, struct mbuf *, uint32_t, int);
 void	if_up(struct ifnet *);