On 21/08/17(Mon) 16:21, Martin Pieuchot wrote:
> In order to stop allocating multiple mbufs and possibly waiting with
> the socket lock held in NFS, I'd like sosetopt() to no longer free the
> mbuf it receives.
>
> This diff makes sosetopt() similar to sogetopt(), the caller is
> responsible for allocating & freeing the option mbuf.
Thanks for all the encouragements. visa@ spotted two places in net/bfd.c
where I missed a m_freem(). Updated diff below fixes that. I'd
appreciates more reviews to make sure I don't introduce any double free.
ok?
Index: kern/uipc_socket.c
===================================================================
RCS file: /cvs/src/sys/kern/uipc_socket.c,v
retrieving revision 1.201
diff -u -p -r1.201 uipc_socket.c
--- kern/uipc_socket.c 10 Aug 2017 19:20:43 -0000 1.201
+++ kern/uipc_socket.c 21 Aug 2017 14:02:56 -0000
@@ -1558,17 +1558,16 @@ sowwakeup(struct socket *so)
}
int
-sosetopt(struct socket *so, int level, int optname, struct mbuf *m0)
+sosetopt(struct socket *so, int level, int optname, struct mbuf *m)
{
int error = 0;
- struct mbuf *m = m0;
soassertlocked(so);
if (level != SOL_SOCKET) {
if (so->so_proto->pr_ctloutput) {
error = (*so->so_proto->pr_ctloutput)(PRCO_SETOPT, so,
- level, optname, m0);
+ level, optname, m);
return (error);
}
error = ENOPROTOOPT;
@@ -1576,7 +1575,7 @@ sosetopt(struct socket *so, int level, i
switch (optname) {
case SO_BINDANY:
if ((error = suser(curproc, 0)) != 0) /* XXX */
- goto bad;
+ return (error);
break;
}
@@ -1585,10 +1584,8 @@ sosetopt(struct socket *so, int level, i
case SO_LINGER:
if (m == NULL || m->m_len != sizeof (struct linger) ||
mtod(m, struct linger *)->l_linger < 0 ||
- mtod(m, struct linger *)->l_linger > SHRT_MAX) {
- error = EINVAL;
- goto bad;
- }
+ mtod(m, struct linger *)->l_linger > SHRT_MAX)
+ return (EINVAL);
so->so_linger = mtod(m, struct linger *)->l_linger;
/* FALLTHROUGH */
@@ -1602,10 +1599,8 @@ sosetopt(struct socket *so, int level, i
case SO_OOBINLINE:
case SO_TIMESTAMP:
case SO_ZEROIZE:
- if (m == NULL || m->m_len < sizeof (int)) {
- error = EINVAL;
- goto bad;
- }
+ if (m == NULL || m->m_len < sizeof (int))
+ return (EINVAL);
if (*mtod(m, int *))
so->so_options |= optname;
else
@@ -1613,10 +1608,8 @@ sosetopt(struct socket *so, int level, i
break;
case SO_DONTROUTE:
- if (m == NULL || m->m_len < sizeof (int)) {
- error = EINVAL;
- goto bad;
- }
+ if (m == NULL || m->m_len < sizeof (int))
+ return (EINVAL);
if (*mtod(m, int *))
error = EOPNOTSUPP;
break;
@@ -1628,38 +1621,28 @@ sosetopt(struct socket *so, int level, i
{
u_long cnt;
- if (m == NULL || m->m_len < sizeof (int)) {
- error = EINVAL;
- goto bad;
- }
+ if (m == NULL || m->m_len < sizeof (int))
+ return (EINVAL);
cnt = *mtod(m, int *);
if ((long)cnt <= 0)
cnt = 1;
switch (optname) {
case SO_SNDBUF:
- if (so->so_state & SS_CANTSENDMORE) {
- error = EINVAL;
- goto bad;
- }
+ if (so->so_state & SS_CANTSENDMORE)
+ return (EINVAL);
if (sbcheckreserve(cnt, so->so_snd.sb_wat) ||
- sbreserve(so, &so->so_snd, cnt)) {
- error = ENOBUFS;
- goto bad;
- }
+ sbreserve(so, &so->so_snd, cnt))
+ return (ENOBUFS);
so->so_snd.sb_wat = cnt;
break;
case SO_RCVBUF:
- if (so->so_state & SS_CANTRCVMORE) {
- error = EINVAL;
- goto bad;
- }
+ if (so->so_state & SS_CANTRCVMORE)
+ return (EINVAL);
if (sbcheckreserve(cnt, so->so_rcv.sb_wat) ||
- sbreserve(so, &so->so_rcv, cnt)) {
- error = ENOBUFS;
- goto bad;
- }
+ sbreserve(so, &so->so_rcv, cnt))
+ return (ENOBUFS);
so->so_rcv.sb_wat = cnt;
break;
@@ -1683,16 +1666,12 @@ sosetopt(struct socket *so, int level, i
struct timeval tv;
int val;
- if (m == NULL || m->m_len < sizeof (tv)) {
- error = EINVAL;
- goto bad;
- }
+ if (m == NULL || m->m_len < sizeof (tv))
+ return (EINVAL);
memcpy(&tv, mtod(m, struct timeval *), sizeof tv);
val = tvtohz(&tv);
- if (val > USHRT_MAX) {
- error = EDOM;
- goto bad;
- }
+ if (val > USHRT_MAX)
+ return (EDOM);
switch (optname) {
@@ -1714,7 +1693,7 @@ sosetopt(struct socket *so, int level, i
level = dom->dom_protosw->pr_protocol;
error = (*so->so_proto->pr_ctloutput)
- (PRCO_SETOPT, so, level, optname, m0);
+ (PRCO_SETOPT, so, level, optname, m);
return (error);
}
error = ENOPROTOOPT;
@@ -1725,8 +1704,7 @@ sosetopt(struct socket *so, int level, i
if (m == NULL) {
error = sosplice(so, -1, 0, NULL);
} else if (m->m_len < sizeof(int)) {
- error = EINVAL;
- goto bad;
+ return (EINVAL);
} else if (m->m_len < sizeof(struct splice)) {
error = sosplice(so, *mtod(m, int *), 0, NULL);
} else {
@@ -1744,13 +1722,10 @@ sosetopt(struct socket *so, int level, i
}
if (error == 0 && so->so_proto->pr_ctloutput) {
(*so->so_proto->pr_ctloutput)(PRCO_SETOPT, so,
- level, optname, m0);
- m = NULL; /* freed by protocol */
+ level, optname, m);
}
}
-bad:
- if (m)
- (void) m_free(m);
+
return (error);
}
Index: kern/uipc_syscalls.c
===================================================================
RCS file: /cvs/src/sys/kern/uipc_syscalls.c,v
retrieving revision 1.158
diff -u -p -r1.158 uipc_syscalls.c
--- kern/uipc_syscalls.c 10 Aug 2017 19:20:43 -0000 1.158
+++ kern/uipc_syscalls.c 21 Aug 2017 14:02:56 -0000
@@ -979,7 +979,6 @@ sys_setsockopt(struct proc *p, void *v,
s = solock(so);
error = sosetopt(so, SCARG(uap, level), SCARG(uap, name), m);
sounlock(s);
- m = NULL;
bad:
m_freem(m);
FRELE(fp, p);
Index: net/bfd.c
===================================================================
RCS file: /cvs/src/sys/net/bfd.c,v
retrieving revision 1.63
diff -u -p -r1.63 bfd.c
--- net/bfd.c 10 Aug 2017 16:38:37 -0000 1.63
+++ net/bfd.c 22 Aug 2017 08:10:29 -0000
@@ -454,6 +454,7 @@ bfd_listener(struct bfd_config *bfd, uns
ip = mtod(mopt, int *);
*ip = MAXTTL;
error = sosetopt(so, IPPROTO_IP, IP_MINTTL, mopt);
+ m_freem(mopt);
if (error) {
printf("%s: sosetopt error %d\n",
__func__, error);
@@ -528,6 +529,7 @@ bfd_sender(struct bfd_config *bfd, unsig
ip = mtod(mopt, int *);
*ip = IP_PORTRANGE_HIGH;
error = sosetopt(so, IPPROTO_IP, IP_PORTRANGE, mopt);
+ m_freem(mopt);
if (error) {
printf("%s: sosetopt error %d\n",
__func__, error);
@@ -539,6 +541,7 @@ bfd_sender(struct bfd_config *bfd, unsig
ip = mtod(mopt, int *);
*ip = MAXTTL;
error = sosetopt(so, IPPROTO_IP, IP_TTL, mopt);
+ m_freem(mopt);
if (error) {
printf("%s: sosetopt error %d\n",
__func__, error);
@@ -550,6 +553,7 @@ bfd_sender(struct bfd_config *bfd, unsig
ip = mtod(mopt, int *);
*ip = IPTOS_PREC_INTERNETCONTROL;
error = sosetopt(so, IPPROTO_IP, IP_TOS, mopt);
+ m_freem(mopt);
if (error) {
printf("%s: sosetopt error %d\n",
__func__, error);
Index: net/rtsock.c
===================================================================
RCS file: /cvs/src/sys/net/rtsock.c,v
retrieving revision 1.248
diff -u -p -r1.248 rtsock.c
--- net/rtsock.c 11 Aug 2017 21:24:19 -0000 1.248
+++ net/rtsock.c 21 Aug 2017 14:02:56 -0000
@@ -276,12 +276,8 @@ route_ctloutput(int op, struct socket *s
int error = 0;
unsigned int tid;
- if (level != AF_ROUTE) {
- error = EINVAL;
- if (op == PRCO_SETOPT && m)
- m_free(m);
- return (error);
- }
+ if (level != AF_ROUTE)
+ return (EINVAL);
switch (op) {
case PRCO_SETOPT:
@@ -307,7 +303,6 @@ route_ctloutput(int op, struct socket *s
error = ENOPROTOOPT;
break;
}
- m_free(m);
break;
case PRCO_GETOPT:
switch (optname) {
Index: netinet/ip_mroute.c
===================================================================
RCS file: /cvs/src/sys/netinet/ip_mroute.c,v
retrieving revision 1.120
diff -u -p -r1.120 ip_mroute.c
--- netinet/ip_mroute.c 26 Jun 2017 09:32:32 -0000 1.120
+++ netinet/ip_mroute.c 21 Aug 2017 14:02:56 -0000
@@ -216,7 +216,6 @@ ip_mrouter_set(struct socket *so, int op
break;
}
- m_free(m);
return (error);
}
Index: netinet/ip_output.c
===================================================================
RCS file: /cvs/src/sys/netinet/ip_output.c,v
retrieving revision 1.340
diff -u -p -r1.340 ip_output.c
--- netinet/ip_output.c 29 May 2017 14:36:22 -0000 1.340
+++ netinet/ip_output.c 21 Aug 2017 14:02:56 -0000
@@ -72,6 +72,8 @@
#endif
#endif /* IPSEC */
+int ip_pcbopts(struct mbuf **, struct mbuf *);
+int ip_setmoptions(int, struct ip_moptions **, struct mbuf *, u_int);
void ip_mloopback(struct ifnet *, struct mbuf *, struct sockaddr_in *);
static __inline u_int16_t __attribute__((__unused__))
in_cksum_phdr(u_int32_t, u_int32_t, u_int32_t);
@@ -849,8 +851,6 @@ ip_ctloutput(int op, struct socket *so,
if (level != IPPROTO_IP) {
error = EINVAL;
- if (op == PRCO_SETOPT)
- (void) m_free(m);
} else switch (op) {
case PRCO_SETOPT:
switch (optname) {
@@ -1067,7 +1067,6 @@ ip_ctloutput(int op, struct socket *so,
error = ENOPROTOOPT;
break;
}
- m_free(m);
break;
case PRCO_GETOPT:
@@ -1234,7 +1233,6 @@ ip_pcbopts(struct mbuf **pcbopt, struct
/*
* Only turning off any previous options.
*/
- m_free(m);
return (0);
}
@@ -1310,7 +1308,6 @@ ip_pcbopts(struct mbuf **pcbopt, struct
return (0);
bad:
- (void)m_free(m);
return (EINVAL);
}
Index: netinet/ip_var.h
===================================================================
RCS file: /cvs/src/sys/netinet/ip_var.h,v
retrieving revision 1.80
diff -u -p -r1.80 ip_var.h
--- netinet/ip_var.h 14 Jul 2017 16:50:41 -0000 1.80
+++ netinet/ip_var.h 21 Aug 2017 14:02:56 -0000
@@ -230,13 +230,11 @@ int ip_mforward(struct mbuf *, struct i
int ip_optcopy(struct ip *, struct ip *);
int ip_output(struct mbuf *, struct mbuf *, struct route *, int,
struct ip_moptions *, struct inpcb *, u_int32_t);
-int ip_pcbopts(struct mbuf **, struct mbuf *);
struct mbuf *
ip_reass(struct ipqent *, struct ipq *);
u_int16_t
ip_randomid(void);
void ip_send(struct mbuf *);
-int ip_setmoptions(int, struct ip_moptions **, struct mbuf *, u_int);
void ip_slowtimo(void);
struct mbuf *
ip_srcroute(struct mbuf *);
Index: netinet/raw_ip.c
===================================================================
RCS file: /cvs/src/sys/netinet/raw_ip.c,v
retrieving revision 1.101
diff -u -p -r1.101 raw_ip.c
--- netinet/raw_ip.c 11 Aug 2017 19:53:02 -0000 1.101
+++ netinet/raw_ip.c 21 Aug 2017 14:02:56 -0000
@@ -301,11 +301,8 @@ rip_ctloutput(int op, struct socket *so,
int error = 0;
int dir;
- if (level != IPPROTO_IP) {
- if (op == PRCO_SETOPT)
- (void) m_free(m);
+ if (level != IPPROTO_IP)
return (EINVAL);
- }
switch (optname) {
@@ -318,7 +315,6 @@ rip_ctloutput(int op, struct socket *so,
inp->inp_flags |= INP_HDRINCL;
else
inp->inp_flags &= ~INP_HDRINCL;
- m_free(m);
} else {
m->m_len = sizeof(int);
*mtod(m, int *) = inp->inp_flags & INP_HDRINCL;
@@ -353,8 +349,6 @@ rip_ctloutput(int op, struct socket *so,
break;
}
- if (op == PRCO_SETOPT)
- (void)m_free(m);
return (error);
case MRT_INIT:
@@ -381,8 +375,6 @@ rip_ctloutput(int op, struct socket *so,
}
return (error);
#else
- if (op == PRCO_SETOPT)
- m_free(m);
return (EOPNOTSUPP);
#endif
}
Index: netinet/tcp_usrreq.c
===================================================================
RCS file: /cvs/src/sys/netinet/tcp_usrreq.c,v
retrieving revision 1.153
diff -u -p -r1.153 tcp_usrreq.c
--- netinet/tcp_usrreq.c 15 Aug 2017 17:47:15 -0000 1.153
+++ netinet/tcp_usrreq.c 21 Aug 2017 14:02:56 -0000
@@ -437,11 +437,8 @@ tcp_ctloutput(int op, struct socket *so,
int i;
inp = sotoinpcb(so);
- if (inp == NULL) {
- if (op == PRCO_SETOPT)
- (void) m_free(m);
+ if (inp == NULL)
return (ECONNRESET);
- }
if (level != IPPROTO_TCP) {
switch (so->so_proto->pr_domain->dom_family) {
#ifdef INET6
@@ -547,7 +544,6 @@ tcp_ctloutput(int op, struct socket *so,
error = ENOPROTOOPT;
break;
}
- m_free(m);
break;
case PRCO_GETOPT:
Index: netinet6/icmp6.c
===================================================================
RCS file: /cvs/src/sys/netinet6/icmp6.c,v
retrieving revision 1.215
diff -u -p -r1.215 icmp6.c
--- netinet6/icmp6.c 10 Aug 2017 02:26:26 -0000 1.215
+++ netinet6/icmp6.c 21 Aug 2017 14:02:56 -0000
@@ -1687,11 +1687,8 @@ icmp6_ctloutput(int op, struct socket *s
int error = 0;
struct inpcb *in6p = sotoinpcb(so);
- if (level != IPPROTO_ICMPV6) {
- if (op == PRCO_SETOPT)
- (void)m_free(m);
+ if (level != IPPROTO_ICMPV6)
return EINVAL;
- }
switch (op) {
case PRCO_SETOPT:
@@ -1719,7 +1716,6 @@ icmp6_ctloutput(int op, struct socket *s
error = ENOPROTOOPT;
break;
}
- m_freem(m);
break;
case PRCO_GETOPT:
Index: netinet6/ip6_output.c
===================================================================
RCS file: /cvs/src/sys/netinet6/ip6_output.c,v
retrieving revision 1.231
diff -u -p -r1.231 ip6_output.c
--- netinet6/ip6_output.c 9 May 2017 09:32:21 -0000 1.231
+++ netinet6/ip6_output.c 21 Aug 2017 14:02:56 -0000
@@ -1045,11 +1045,8 @@ ip6_ctloutput(int op, struct socket *so,
privileged = (inp->inp_socket->so_state & SS_PRIV);
uproto = (int)so->so_proto->pr_protocol;
- if (level != IPPROTO_IPV6) {
- if (op == PRCO_SETOPT)
- m_free(m);
+ if (level != IPPROTO_IPV6)
return (EINVAL);
- }
switch (op) {
case PRCO_SETOPT:
@@ -1378,7 +1375,6 @@ do { \
error = ENOPROTOOPT;
break;
}
- m_free(m);
break;
case PRCO_GETOPT:
@@ -1590,11 +1586,8 @@ ip6_raw_ctloutput(int op, struct socket
const int icmp6off = offsetof(struct icmp6_hdr, icmp6_cksum);
struct inpcb *inp = sotoinpcb(so);
- if (level != IPPROTO_IPV6) {
- if (op == PRCO_SETOPT)
- (void)m_free(m);
+ if (level != IPPROTO_IPV6)
return (EINVAL);
- }
switch (optname) {
case IPV6_CHECKSUM:
@@ -1643,9 +1636,6 @@ ip6_raw_ctloutput(int op, struct socket
error = ENOPROTOOPT;
break;
}
-
- if (op == PRCO_SETOPT)
- (void)m_free(m);
return (error);
}
Index: netinet6/raw_ip6.c
===================================================================
RCS file: /cvs/src/sys/netinet6/raw_ip6.c,v
retrieving revision 1.117
diff -u -p -r1.117 raw_ip6.c
--- netinet6/raw_ip6.c 15 Aug 2017 17:47:15 -0000 1.117
+++ netinet6/raw_ip6.c 21 Aug 2017 14:02:56 -0000
@@ -525,8 +525,6 @@ rip6_ctloutput(int op, struct socket *so
break;
}
- if (op == PRCO_SETOPT)
- (void)m_free(m);
return (error);
#ifdef MROUTING
@@ -538,7 +536,6 @@ rip6_ctloutput(int op, struct socket *so
case MRT6_DEL_MFC:
if (op == PRCO_SETOPT) {
error = ip6_mrouter_set(optname, so, m);
- m_free(m);
} else if (op == PRCO_GETOPT)
error = ip6_mrouter_get(optname, so, m);
else
@@ -559,8 +556,6 @@ rip6_ctloutput(int op, struct socket *so
return (icmp6_ctloutput(op, so, level, optname, m));
default:
- if (op == PRCO_SETOPT)
- m_free(m);
return EINVAL;
}
}
Index: sys/socketvar.h
===================================================================
RCS file: /cvs/src/sys/sys/socketvar.h,v
retrieving revision 1.74
diff -u -p -r1.74 socketvar.h
--- sys/socketvar.h 12 Jul 2017 10:56:47 -0000 1.74
+++ sys/socketvar.h 21 Aug 2017 14:02:56 -0000
@@ -330,8 +330,7 @@ int soreserve(struct socket *so, u_long
void sorflush(struct socket *so);
int sosend(struct socket *so, struct mbuf *addr, struct uio *uio,
struct mbuf *top, struct mbuf *control, int flags);
-int sosetopt(struct socket *so, int level, int optname,
- struct mbuf *m0);
+int sosetopt(struct socket *so, int level, int optname, struct mbuf *m);
int soshutdown(struct socket *so, int how);
void sowakeup(struct socket *so, struct sockbuf *sb);
void sorwakeup(struct socket *);
Index: nfs/krpc_subr.c
===================================================================
RCS file: /cvs/src/sys/nfs/krpc_subr.c,v
retrieving revision 1.31
diff -u -p -r1.31 krpc_subr.c
--- nfs/krpc_subr.c 10 Aug 2017 19:20:43 -0000 1.31
+++ nfs/krpc_subr.c 21 Aug 2017 14:02:56 -0000
@@ -242,6 +242,7 @@ krpc_call(struct sockaddr_in *sa, u_int
s = solock(so);
error = sosetopt(so, SOL_SOCKET, SO_RCVTIMEO, m);
sounlock(s);
+ m_freem(m);
if (error)
goto out;
@@ -257,6 +258,7 @@ krpc_call(struct sockaddr_in *sa, u_int
s = solock(so);
error = sosetopt(so, SOL_SOCKET, SO_BROADCAST, m);
sounlock(s);
+ m_freem(m);
if (error)
goto out;
}
@@ -273,6 +275,7 @@ krpc_call(struct sockaddr_in *sa, u_int
s = solock(so);
error = sosetopt(so, IPPROTO_IP, IP_PORTRANGE, mopt);
sounlock(s);
+ m_freem(mopt);
if (error)
goto out;
@@ -299,6 +302,7 @@ krpc_call(struct sockaddr_in *sa, u_int
s = solock(so);
error = sosetopt(so, IPPROTO_IP, IP_PORTRANGE, mopt);
sounlock(s);
+ m_freem(mopt);
if (error)
goto out;
Index: nfs/nfs_socket.c
===================================================================
RCS file: /cvs/src/sys/nfs/nfs_socket.c,v
retrieving revision 1.125
diff -u -p -r1.125 nfs_socket.c
--- nfs/nfs_socket.c 14 Aug 2017 16:56:57 -0000 1.125
+++ nfs/nfs_socket.c 21 Aug 2017 14:02:56 -0000
@@ -270,6 +270,7 @@ nfs_connect(struct nfsmount *nmp, struct
s = solock(so);
error = sosetopt(so, IPPROTO_IP, IP_PORTRANGE, mopt);
sounlock(s);
+ m_freem(mopt);
if (error)
goto bad;
@@ -294,6 +295,7 @@ nfs_connect(struct nfsmount *nmp, struct
s = solock(so);
error = sosetopt(so, IPPROTO_IP, IP_PORTRANGE, mopt);
sounlock(s);
+ m_freem(mopt);
if (error)
goto bad;
}
@@ -358,12 +360,14 @@ nfs_connect(struct nfsmount *nmp, struct
*mtod(m, int32_t *) = 1;
m->m_len = sizeof(int32_t);
sosetopt(so, SOL_SOCKET, SO_KEEPALIVE, m);
+ m_freem(m);
}
if (so->so_proto->pr_protocol == IPPROTO_TCP) {
MGET(m, M_WAIT, MT_SOOPTS);
*mtod(m, int32_t *) = 1;
m->m_len = sizeof(int32_t);
sosetopt(so, IPPROTO_TCP, TCP_NODELAY, m);
+ m_freem(m);
}
sndreserve = (nmp->nm_wsize + NFS_MAXPKTHDR +
sizeof (u_int32_t)) * 2;
Index: nfs/nfs_syscalls.c
===================================================================
RCS file: /cvs/src/sys/nfs/nfs_syscalls.c,v
retrieving revision 1.110
diff -u -p -r1.110 nfs_syscalls.c
--- nfs/nfs_syscalls.c 9 Aug 2017 14:22:58 -0000 1.110
+++ nfs/nfs_syscalls.c 21 Aug 2017 14:02:56 -0000
@@ -265,6 +265,7 @@ nfssvc_addsock(struct file *fp, struct m
*mtod(m, int32_t *) = 1;
m->m_len = sizeof(int32_t);
sosetopt(so, SOL_SOCKET, SO_KEEPALIVE, m);
+ m_freem(m);
}
if (so->so_proto->pr_domain->dom_family == AF_INET &&
so->so_proto->pr_protocol == IPPROTO_TCP) {
@@ -272,6 +273,7 @@ nfssvc_addsock(struct file *fp, struct m
*mtod(m, int32_t *) = 1;
m->m_len = sizeof(int32_t);
sosetopt(so, IPPROTO_TCP, TCP_NODELAY, m);
+ m_freem(m);
}
so->so_rcv.sb_flags &= ~SB_NOINTR;
so->so_rcv.sb_timeo = 0;
