On Mon, Apr 17, 2023 at 04:32:13PM +0200, Alexander Bluhm wrote:
> On Mon, Apr 17, 2023 at 02:36:57AM +0300, Vitaliy Makkoveev wrote:
> > It seems rt_setsource() needs some attention, but sysctl_source() could
> > be called with shared netlock just now.
> 
> I think rtable_setsource() is not MP safe.  It is documented as [K]
> kernel lock.  But that is not true and makes no sense.  It should
> be exclusive netlock.  in_pcbselsrc() calls rtable_getsource() with
> netlock.  We should rename source to ar_source so we can grep for
> its users.

Perhaps something like this.  I will run it through regress.

Index: net/art.h
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/net/art.h,v
retrieving revision 1.21
diff -u -p -r1.21 art.h
--- net/art.h   2 Mar 2021 17:50:41 -0000       1.21
+++ net/art.h   17 Apr 2023 14:48:46 -0000
@@ -41,7 +41,7 @@ struct art_root {
        uint8_t                  ar_nlvl;       /* [I] Number of levels */
        uint8_t                  ar_alen;       /* [I] Address length in bits */
        uint8_t                  ar_off;        /* [I] Offset of key in bytes */
-       struct sockaddr         *source;        /* [K] optional src addr to use 
*/
+       struct sockaddr         *ar_source;     /* [N] use optional src addr */
 };
 
 #define ISLEAF(e)      (((unsigned long)(e) & 1) == 0)
Index: net/rtable.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/net/rtable.c,v
retrieving revision 1.80
diff -u -p -r1.80 rtable.c
--- net/rtable.c        29 Jun 2022 22:20:47 -0000      1.80
+++ net/rtable.c        17 Apr 2023 14:54:52 -0000
@@ -376,10 +376,12 @@ rtable_setsource(unsigned int rtableid, 
 {
        struct art_root         *ar;
 
+       NET_ASSERT_LOCKED_EXCLUSIVE();
+
        if ((ar = rtable_get(rtableid, af)) == NULL)
                return (EAFNOSUPPORT);
 
-       ar->source = src;
+       ar->ar_source = src;
 
        return (0);
 }
@@ -389,11 +391,13 @@ rtable_getsource(unsigned int rtableid, 
 {
        struct art_root         *ar;
 
+       NET_ASSERT_LOCKED();
+
        ar = rtable_get(rtableid, af);
        if (ar == NULL)
                return (NULL);
 
-       return (ar->source);
+       return (ar->ar_source);
 }
 
 void
Index: net/rtsock.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/net/rtsock.c,v
retrieving revision 1.359
diff -u -p -r1.359 rtsock.c
--- net/rtsock.c        22 Jan 2023 12:05:44 -0000      1.359
+++ net/rtsock.c        17 Apr 2023 15:16:48 -0000
@@ -853,8 +853,10 @@ route_output(struct mbuf *m, struct sock
                        error = EINVAL;
                        goto fail;
                }
-               if ((error =
-                   rt_setsource(tableid, info.rti_info[RTAX_IFA])) != 0)
+               NET_LOCK();
+               error = rt_setsource(tableid, info.rti_info[RTAX_IFA]);
+               NET_UNLOCK();
+               if (error)
                        goto fail;
        } else {
                error = rtm_output(rtm, &rt, &info, prio, tableid);
@@ -862,7 +864,9 @@ route_output(struct mbuf *m, struct sock
                        type = rtm->rtm_type;
                        seq = rtm->rtm_seq;
                        free(rtm, M_RTABLE, len);
+                       NET_LOCK_SHARED();
                        rtm = rtm_report(rt, type, seq, tableid);
+                       NET_UNLOCK_SHARED();
                        len = rtm->rtm_msglen;
                }
        }

Reply via email to