From 86d02c5c63bb5947eb352a0fcdda2b3e3ecebb0a Mon Sep 17 00:00:00 2001 From: "Bjoern A. Zeeb" Date: Sat, 4 Oct 2008 15:06:34 +0000 Subject: [PATCH] Cache so_cred as inp_cred in the inpcb. This means that inp_cred is always there, even after the socket has gone away. It also means that it is constant for the lifetime of the inp. Both facts lead to simpler code and possibly less locking. Suggested by: rwatson Reviewed by: rwatson MFC after: 6 weeks X-MFC Note: use a inp_pspare for inp_cred --- sys/contrib/pf/net/pf.c | 20 +++++--------------- sys/netinet/in_pcb.c | 12 ++++++++---- sys/netinet/in_pcb.h | 1 + sys/netinet/ip_fw2.c | 22 +++++++--------------- sys/netinet/raw_ip.c | 23 ++++++++++++----------- sys/netinet/tcp_subr.c | 4 ++-- sys/netinet/udp_usrreq.c | 2 +- sys/netinet6/in6_pcb.c | 12 ++++++------ sys/netinet6/udp6_usrreq.c | 2 +- 9 files changed, 43 insertions(+), 55 deletions(-) diff --git a/sys/contrib/pf/net/pf.c b/sys/contrib/pf/net/pf.c index c1011be9183..7ac43dadb86 100644 --- a/sys/contrib/pf/net/pf.c +++ b/sys/contrib/pf/net/pf.c @@ -2941,13 +2941,9 @@ pf_socket_lookup(int direction, struct pf_pdesc *pd) #ifdef __FreeBSD__ if (inp_arg != NULL) { INP_LOCK_ASSERT(inp_arg); - if (inp_arg->inp_socket) { - pd->lookup.uid = inp_arg->inp_socket->so_cred->cr_uid; - pd->lookup.gid = - inp_arg->inp_socket->so_cred->cr_groups[0]; - return (1); - } else - return (-1); + pd->lookup.uid = inp_arg->inp_cred->cr_uid; + pd->lookup.gid = inp_arg->inp_cred->cr_groups[0]; + return (1); } #endif switch (pd->proto) { @@ -3043,15 +3039,9 @@ pf_socket_lookup(int direction, struct pf_pdesc *pd) return (-1); } #ifdef __FreeBSD__ - INP_RLOCK(inp); + pd->lookup.uid = inp->inp_cred->cr_uid; + pd->lookup.gid = inp->inp_cred->cr_groups[0]; INP_INFO_RUNLOCK(pi); - if ((inp->inp_socket == NULL) || (inp->inp_socket->so_cred == NULL)) { - INP_RUNLOCK(inp); - return (-1); - } - pd->lookup.uid = inp->inp_socket->so_cred->cr_uid; - pd->lookup.gid = inp->inp_socket->so_cred->cr_groups[0]; - INP_RUNLOCK(inp); #else pd->lookup.uid = inp->inp_socket->so_euid; pd->lookup.gid = inp->inp_socket->so_egid; diff --git a/sys/netinet/in_pcb.c b/sys/netinet/in_pcb.c index ea98a6fba89..d76ed16c49a 100644 --- a/sys/netinet/in_pcb.c +++ b/sys/netinet/in_pcb.c @@ -197,6 +197,7 @@ in_pcballoc(struct socket *so, struct inpcbinfo *pcbinfo) bzero(inp, inp_zero_size); inp->inp_pcbinfo = pcbinfo; inp->inp_socket = so; + inp->inp_cred = crhold(so->so_cred); inp->inp_inc.inc_fibnum = so->so_fibnum; #ifdef MAC error = mac_inpcb_init(inp, M_NOWAIT); @@ -235,8 +236,10 @@ in_pcballoc(struct socket *so, struct inpcbinfo *pcbinfo) #if defined(IPSEC) || defined(MAC) out: - if (error != 0) + if (error != 0) { + crfree(inp->inp_cred); uma_zfree(pcbinfo->ipi_zone, inp); + } #endif return (error); } @@ -357,7 +360,7 @@ in_pcbbind_setup(struct inpcb *inp, struct sockaddr *nam, in_addr_t *laddrp, if (jailed(cred)) prison = 1; if (!IN_MULTICAST(ntohl(sin->sin_addr.s_addr)) && - priv_check_cred(so->so_cred, + priv_check_cred(inp->inp_cred, PRIV_NETINET_REUSEPORT, 0) != 0) { t = in_pcblookup_local(pcbinfo, sin->sin_addr, lport, prison ? 0 : INPLOOKUP_WILDCARD, @@ -374,8 +377,8 @@ in_pcbbind_setup(struct inpcb *inp, struct sockaddr *nam, in_addr_t *laddrp, ntohl(t->inp_laddr.s_addr) != INADDR_ANY || (t->inp_socket->so_options & SO_REUSEPORT) == 0) && - (so->so_cred->cr_uid != - t->inp_socket->so_cred->cr_uid)) + (inp->inp_cred->cr_uid != + t->inp_cred->cr_uid)) return (EADDRINUSE); } if (prison && prison_ip(cred, 0, &sin->sin_addr.s_addr)) @@ -901,6 +904,7 @@ in_pcbfree(struct inpcb *inp) if (inp->inp_moptions != NULL) inp_freemoptions(inp->inp_moptions); inp->inp_vflag = 0; + crfree(inp->inp_cred); #ifdef MAC mac_inpcb_destroy(inp); diff --git a/sys/netinet/in_pcb.h b/sys/netinet/in_pcb.h index 857d00751a1..b393c189b33 100644 --- a/sys/netinet/in_pcb.h +++ b/sys/netinet/in_pcb.h @@ -153,6 +153,7 @@ struct inpcb { void *inp_ppcb; /* (i) pointer to per-protocol pcb */ struct inpcbinfo *inp_pcbinfo; /* (c) PCB list info */ struct socket *inp_socket; /* (i) back pointer to socket */ + struct ucred *inp_cred; /* (c) cache of socket cred */ u_int32_t inp_flow; /* (i) IPv6 flow information */ int inp_flags; /* (i) generic IP/datagram flags */ diff --git a/sys/netinet/ip_fw2.c b/sys/netinet/ip_fw2.c index 861f9e9c9fc..ab4fbb6ca46 100644 --- a/sys/netinet/ip_fw2.c +++ b/sys/netinet/ip_fw2.c @@ -1977,15 +1977,11 @@ fill_ugid_cache(struct inpcb *inp, struct ip_fw_ugid *ugp) { struct ucred *cr; - if (inp->inp_socket != NULL) { - cr = inp->inp_socket->so_cred; - ugp->fw_prid = jailed(cr) ? - cr->cr_prison->pr_id : -1; - ugp->fw_uid = cr->cr_uid; - ugp->fw_ngroups = cr->cr_ngroups; - bcopy(cr->cr_groups, ugp->fw_groups, - sizeof(ugp->fw_groups)); - } + cr = inp->inp_cred; + ugp->fw_prid = jailed(cr) ? cr->cr_prison->pr_id : -1; + ugp->fw_uid = cr->cr_uid; + ugp->fw_ngroups = cr->cr_ngroups; + bcopy(cr->cr_groups, ugp->fw_groups, sizeof(ugp->fw_groups)); } static int @@ -2042,12 +2038,8 @@ check_uidgid(ipfw_insn_u32 *insn, int proto, struct ifnet *oif, dst_ip, htons(dst_port), wildcard, NULL); if (pcb != NULL) { - INP_RLOCK(pcb); - if (pcb->inp_socket != NULL) { - fill_ugid_cache(pcb, ugp); - *ugid_lookupp = 1; - } - INP_RUNLOCK(pcb); + fill_ugid_cache(pcb, ugp); + *ugid_lookupp = 1; } INP_INFO_RUNLOCK(pi); if (*ugid_lookupp == 0) { diff --git a/sys/netinet/raw_ip.c b/sys/netinet/raw_ip.c index c6bca326d8f..0300492110b 100644 --- a/sys/netinet/raw_ip.c +++ b/sys/netinet/raw_ip.c @@ -261,6 +261,7 @@ rip_input(struct mbuf *m, int off) if (inp->inp_ip_p != proto) continue; #ifdef INET6 + /* XXX inp locking */ if ((inp->inp_vflag & INP_IPV4) == 0) continue; #endif @@ -268,11 +269,9 @@ rip_input(struct mbuf *m, int off) continue; if (inp->inp_faddr.s_addr != ip->ip_src.s_addr) continue; - INP_RLOCK(inp); - if (jailed(inp->inp_socket->so_cred) && - (htonl(prison_getip(inp->inp_socket->so_cred)) != + if (jailed(inp->inp_cred) && + (htonl(prison_getip(inp->inp_cred)) != ip->ip_dst.s_addr)) { - INP_RUNLOCK(inp); continue; } if (last) { @@ -284,12 +283,14 @@ rip_input(struct mbuf *m, int off) /* XXX count dropped packet */ INP_RUNLOCK(last); } + INP_RLOCK(inp); last = inp; } LIST_FOREACH(inp, &V_ripcbinfo.ipi_hashbase[0], inp_hash) { if (inp->inp_ip_p && inp->inp_ip_p != proto) continue; #ifdef INET6 + /* XXX inp locking */ if ((inp->inp_vflag & INP_IPV4) == 0) continue; #endif @@ -299,9 +300,8 @@ rip_input(struct mbuf *m, int off) if (inp->inp_faddr.s_addr && inp->inp_faddr.s_addr != ip->ip_src.s_addr) continue; - INP_RLOCK(inp); - if (jailed(inp->inp_socket->so_cred) && - (htonl(prison_getip(inp->inp_socket->so_cred)) != + if (jailed(inp->inp_cred) && + (htonl(prison_getip(inp->inp_cred)) != ip->ip_dst.s_addr)) { INP_RUNLOCK(inp); continue; @@ -315,6 +315,7 @@ rip_input(struct mbuf *m, int off) /* XXX count dropped packet */ INP_RUNLOCK(last); } + INP_RLOCK(inp); last = inp; } INP_INFO_RUNLOCK(&V_ripcbinfo); @@ -365,9 +366,9 @@ rip_output(struct mbuf *m, struct socket *so, u_long dst) ip->ip_off = 0; ip->ip_p = inp->inp_ip_p; ip->ip_len = m->m_pkthdr.len; - if (jailed(inp->inp_socket->so_cred)) + if (jailed(inp->inp_cred)) ip->ip_src.s_addr = - htonl(prison_getip(inp->inp_socket->so_cred)); + htonl(prison_getip(inp->inp_cred)); else ip->ip_src = inp->inp_laddr; ip->ip_dst.s_addr = dst; @@ -379,9 +380,9 @@ rip_output(struct mbuf *m, struct socket *so, u_long dst) } INP_RLOCK(inp); ip = mtod(m, struct ip *); - if (jailed(inp->inp_socket->so_cred)) { + if (jailed(inp->inp_cred)) { if (ip->ip_src.s_addr != - htonl(prison_getip(inp->inp_socket->so_cred))) { + htonl(prison_getip(inp->inp_cred))) { INP_RUNLOCK(inp); m_freem(m); return (EPERM); diff --git a/sys/netinet/tcp_subr.c b/sys/netinet/tcp_subr.c index f94e290ff1f..be9daae9db2 100644 --- a/sys/netinet/tcp_subr.c +++ b/sys/netinet/tcp_subr.c @@ -1107,7 +1107,7 @@ tcp_getcred(SYSCTL_HANDLER_ARGS) error = cr_canseesocket(req->td->td_ucred, inp->inp_socket); if (error == 0) - cru2x(inp->inp_socket->so_cred, &xuc); + cru2x(inp->inp_cred, &xuc); INP_RUNLOCK(inp); } else { INP_INFO_RUNLOCK(&V_tcbinfo); @@ -1171,7 +1171,7 @@ tcp6_getcred(SYSCTL_HANDLER_ARGS) error = cr_canseesocket(req->td->td_ucred, inp->inp_socket); if (error == 0) - cru2x(inp->inp_socket->so_cred, &xuc); + cru2x(inp->inp_cred, &xuc); INP_RUNLOCK(inp); } else { INP_INFO_RUNLOCK(&V_tcbinfo); diff --git a/sys/netinet/udp_usrreq.c b/sys/netinet/udp_usrreq.c index f33b7601a43..b5b9c553c28 100644 --- a/sys/netinet/udp_usrreq.c +++ b/sys/netinet/udp_usrreq.c @@ -761,7 +761,7 @@ udp_getcred(SYSCTL_HANDLER_ARGS) error = cr_canseesocket(req->td->td_ucred, inp->inp_socket); if (error == 0) - cru2x(inp->inp_socket->so_cred, &xuc); + cru2x(inp->inp_cred, &xuc); INP_RUNLOCK(inp); } else { INP_INFO_RUNLOCK(&V_udbinfo); diff --git a/sys/netinet6/in6_pcb.c b/sys/netinet6/in6_pcb.c index 404335b0a96..d241f1ed86f 100644 --- a/sys/netinet6/in6_pcb.c +++ b/sys/netinet6/in6_pcb.c @@ -189,7 +189,7 @@ in6_pcbbind(register struct inpcb *inp, struct sockaddr *nam, 0)) return (EACCES); if (!IN6_IS_ADDR_MULTICAST(&sin6->sin6_addr) && - priv_check_cred(so->so_cred, + priv_check_cred(inp->inp_cred, PRIV_NETINET_REUSEPORT, 0) != 0) { t = in6_pcblookup_local(pcbinfo, &sin6->sin6_addr, lport, @@ -201,8 +201,8 @@ in6_pcbbind(register struct inpcb *inp, struct sockaddr *nam, (!IN6_IS_ADDR_UNSPECIFIED(&sin6->sin6_addr) || !IN6_IS_ADDR_UNSPECIFIED(&t->in6p_laddr) || (t->inp_socket->so_options & SO_REUSEPORT) - == 0) && (so->so_cred->cr_uid != - t->inp_socket->so_cred->cr_uid)) + == 0) && (inp->inp_cred->cr_uid != + t->inp_cred->cr_uid)) return (EADDRINUSE); if ((inp->inp_flags & IN6P_IPV6_V6ONLY) == 0 && IN6_IS_ADDR_UNSPECIFIED(&sin6->sin6_addr)) { @@ -218,8 +218,8 @@ in6_pcbbind(register struct inpcb *inp, struct sockaddr *nam, (so->so_type != SOCK_STREAM || ntohl(t->inp_faddr.s_addr) == INADDR_ANY) && - (so->so_cred->cr_uid != - t->inp_socket->so_cred->cr_uid)) + (inp->inp_cred->cr_uid != + t->inp_cred->cr_uid)) return (EADDRINUSE); } } @@ -323,7 +323,7 @@ in6_pcbladdr(register struct inpcb *inp, struct sockaddr *nam, */ *plocal_addr6 = in6_selectsrc(sin6, inp->in6p_outputopts, inp, NULL, - inp->inp_socket->so_cred, + inp->inp_cred, &ifp, &error); if (ifp && scope_ambiguous && (error = in6_setscope(&sin6->sin6_addr, ifp, NULL)) != 0) { diff --git a/sys/netinet6/udp6_usrreq.c b/sys/netinet6/udp6_usrreq.c index 5056ef1d84b..f85fe5841ce 100644 --- a/sys/netinet6/udp6_usrreq.c +++ b/sys/netinet6/udp6_usrreq.c @@ -466,7 +466,7 @@ udp6_getcred(SYSCTL_HANDLER_ARGS) error = cr_canseesocket(req->td->td_ucred, inp->inp_socket); if (error == 0) - cru2x(inp->inp_socket->so_cred, &xuc); + cru2x(inp->inp_cred, &xuc); INP_RUNLOCK(inp); } else { INP_INFO_RUNLOCK(&V_udbinfo);