Re: make sosetopt responsible for m_free
On Mon, Feb 06, 2017 at 01:16:45PM +0100, Martin Pieuchot wrote: > On 03/02/17(Fri) 11:02, David Hill wrote: > > On Fri, Feb 03, 2017 at 09:50:40AM +0100, Martin Pieuchot wrote: > > > On 02/02/17(Thu) 12:12, David Hill wrote: > > > > On Thu, Feb 02, 2017 at 09:34:07AM +0100, Martin Pieuchot wrote: > > > > > On 01/02/17(Wed) 19:27, David Hill wrote: > > > > > > Hello - > > > > > > > > > > > > This diff makes sosetopt responsible for m_free which is much > > > > > > simpler. > > > > > > Requested by bluhm@ > > > > > > > > > > I'd suggest to move the m_free(9) calls to sys_setsockopt(). This > > > > > simplifies the existing code even more and will make it easier to use > > > > > the stack for this temporary storage. > > > > > > > > > > > > > New diff with mpi@'s suggestion. > > > > > > You forgot NFS and BFD that should now call m_free(9) after sosetopt(9). > > > > > > > Indeed! Now with BFD and NFS... > > You're introducing a use after-free in ip_pcbopts(). You need to > allocate/copy the mbuf there. > > I must say I'm a bit afraid of this change because the amount of code it > touches. There might be another use after free somewhere that I missed. > > Maybe we should first split our huge *ctloutput functions. > > One easy move is to split setopt/getopt. > > Then introduce more per-protocol functions instead of having everything > in ip{,6}_ctloutput(). > > For example move all the IPSEC craziness out of these functions. Same > thing with ICMP6... This might sound superfluous but it will help > if/when we decide to have a fine graining for different subsystems. > > I'd also suggest to change the 'struct protosw' declaration to use C99 > initializer. So we can have: > > .pr_ctloutput = ipsec_ctloutput > > This would allow us to grep for "pr_ctloutput" (or pr_setopt) and know > directly which functions to review. > If you are OK with first splitting each *ctloutput into *getopt/*setopt, I will send each diff individually to make review easier. Here is the first split, route_ctloutput. Index: net/rtsock.c === RCS file: /cvs/src/sys/net/rtsock.c,v retrieving revision 1.222 diff -u -p -r1.222 rtsock.c --- net/rtsock.c1 Feb 2017 20:59:47 - 1.222 +++ net/rtsock.c21 Feb 2017 15:52:07 - @@ -98,6 +98,8 @@ struct walkarg { caddr_t w_where, w_tmem; }; +introute_getopt(struct socket *, int, int, struct mbuf *); +introute_setopt(struct socket *, int, int, struct mbuf *); introute_ctloutput(int, struct socket *, int, int, struct mbuf *); void route_input(struct mbuf *m0, sa_family_t); introute_arp_conflict(struct rtentry *, struct rt_addrinfo *); @@ -233,62 +235,86 @@ route_usrreq(struct socket *so, int req, } int -route_ctloutput(int op, struct socket *so, int level, int optname, -struct mbuf *m) +route_getopt(struct socket *so, int level, int optname, struct mbuf *m) +{ + struct routecb *rop = sotoroutecb(so); + int error = 0; + + if (level != AF_ROUTE) + return EINVAL; + + switch (optname) { + case ROUTE_MSGFILTER: + m->m_len = sizeof(unsigned int); + *mtod(m, unsigned int *) = rop->msgfilter; + break; + case ROUTE_TABLEFILTER: + m->m_len = sizeof(unsigned int); + *mtod(m, unsigned int *) = rop->rtableid; + break; + default: + error = ENOPROTOOPT; + break; + } + return error; +} + +int +route_setopt(struct socket *so, int level, int optname, struct mbuf *m) { struct routecb *rop = sotoroutecb(so); int error = 0; unsigned int tid; if (level != AF_ROUTE) { - error = EINVAL; - if (op == PRCO_SETOPT && m) - m_free(m); - return (error); + m_free(m); + return EINVAL; } - switch (op) { - case PRCO_SETOPT: - switch (optname) { - case ROUTE_MSGFILTER: - if (m == NULL || m->m_len != sizeof(unsigned int)) - error = EINVAL; - else - rop->msgfilter = *mtod(m, unsigned int *); - break; - case ROUTE_TABLEFILTER: - if (m == NULL || m->m_len != sizeof(unsigned int)) { - error = EINVAL; - break; - } - tid = *mtod(m, unsigned int *); - if (tid != RTABLE_ANY && !rtable_exists(tid)) - error = ENOENT; - else - rop->rtableid = tid; - break; - default: - error = ENOPROTOOPT; + switch (optname) { +
Re: make sosetopt responsible for m_free
On 03/02/17(Fri) 11:02, David Hill wrote: > On Fri, Feb 03, 2017 at 09:50:40AM +0100, Martin Pieuchot wrote: > > On 02/02/17(Thu) 12:12, David Hill wrote: > > > On Thu, Feb 02, 2017 at 09:34:07AM +0100, Martin Pieuchot wrote: > > > > On 01/02/17(Wed) 19:27, David Hill wrote: > > > > > Hello - > > > > > > > > > > This diff makes sosetopt responsible for m_free which is much simpler. > > > > > Requested by bluhm@ > > > > > > > > I'd suggest to move the m_free(9) calls to sys_setsockopt(). This > > > > simplifies the existing code even more and will make it easier to use > > > > the stack for this temporary storage. > > > > > > > > > > New diff with mpi@'s suggestion. > > > > You forgot NFS and BFD that should now call m_free(9) after sosetopt(9). > > > > Indeed! Now with BFD and NFS... You're introducing a use after-free in ip_pcbopts(). You need to allocate/copy the mbuf there. I must say I'm a bit afraid of this change because the amount of code it touches. There might be another use after free somewhere that I missed. Maybe we should first split our huge *ctloutput functions. One easy move is to split setopt/getopt. Then introduce more per-protocol functions instead of having everything in ip{,6}_ctloutput(). For example move all the IPSEC craziness out of these functions. Same thing with ICMP6... This might sound superfluous but it will help if/when we decide to have a fine graining for different subsystems. I'd also suggest to change the 'struct protosw' declaration to use C99 initializer. So we can have: .pr_ctloutput = ipsec_ctloutput This would allow us to grep for "pr_ctloutput" (or pr_setopt) and know directly which functions to review. > Index: kern/uipc_socket.c > === > RCS file: /cvs/src/sys/kern/uipc_socket.c,v > retrieving revision 1.176 > diff -u -p -r1.176 uipc_socket.c > --- kern/uipc_socket.c1 Feb 2017 20:59:47 - 1.176 > +++ kern/uipc_socket.c3 Feb 2017 16:00:26 - > @@ -1551,16 +1551,15 @@ 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 s, error = 0; > - struct mbuf *m = m0; > > if (level != SOL_SOCKET) { > if (so->so_proto && so->so_proto->pr_ctloutput) { > NET_LOCK(s); > error = (*so->so_proto->pr_ctloutput)(PRCO_SETOPT, so, > - level, optname, m0); > + level, optname, m); > NET_UNLOCK(s); > return (error); > } > @@ -1707,7 +1706,7 @@ sosetopt(struct socket *so, int level, i > level = dom->dom_protosw->pr_protocol; > NET_LOCK(s); > error = (*so->so_proto->pr_ctloutput) > - (PRCO_SETOPT, so, level, optname, m0); > + (PRCO_SETOPT, so, level, optname, m); > NET_UNLOCK(s); > return (error); > } > @@ -1739,14 +1738,11 @@ sosetopt(struct socket *so, int level, i > if (error == 0 && so->so_proto && so->so_proto->pr_ctloutput) { > NET_LOCK(s); > (*so->so_proto->pr_ctloutput)(PRCO_SETOPT, so, > - level, optname, m0); > + level, optname, m); > NET_UNLOCK(s); > - m = NULL; /* freed by protocol */ > } > } > 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.148 > diff -u -p -r1.148 uipc_syscalls.c > --- kern/uipc_syscalls.c 26 Jan 2017 01:58:00 - 1.148 > +++ kern/uipc_syscalls.c 3 Feb 2017 16:00:26 - > @@ -962,19 +962,13 @@ sys_setsockopt(struct proc *p, void *v, > goto bad; > } > } > - if (m == NULL) { > - error = ENOBUFS; > - goto bad; > - } > error = copyin(SCARG(uap, val), mtod(m, caddr_t), > SCARG(uap, valsize)); > - if (error) { > + if (error) > goto bad; > - } > m->m_len = SCARG(uap, valsize); > } > error = sosetopt(fp->f_data, SCARG(uap, level), SCARG(uap, name), m); > - m = NULL; > bad: > m_freem(m); > FRELE(fp, p); > Index: net/bfd.c > === > RCS file: /cvs/src/sys/net/bfd.c,v > retrie
Re: make sosetopt responsible for m_free
On Fri, Feb 03, 2017 at 09:50:40AM +0100, Martin Pieuchot wrote: > On 02/02/17(Thu) 12:12, David Hill wrote: > > On Thu, Feb 02, 2017 at 09:34:07AM +0100, Martin Pieuchot wrote: > > > On 01/02/17(Wed) 19:27, David Hill wrote: > > > > Hello - > > > > > > > > This diff makes sosetopt responsible for m_free which is much simpler. > > > > Requested by bluhm@ > > > > > > I'd suggest to move the m_free(9) calls to sys_setsockopt(). This > > > simplifies the existing code even more and will make it easier to use > > > the stack for this temporary storage. > > > > > > > New diff with mpi@'s suggestion. > > You forgot NFS and BFD that should now call m_free(9) after sosetopt(9). > Indeed! Now with BFD and NFS... Index: kern/uipc_socket.c === RCS file: /cvs/src/sys/kern/uipc_socket.c,v retrieving revision 1.176 diff -u -p -r1.176 uipc_socket.c --- kern/uipc_socket.c 1 Feb 2017 20:59:47 - 1.176 +++ kern/uipc_socket.c 3 Feb 2017 16:00:26 - @@ -1551,16 +1551,15 @@ 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 s, error = 0; - struct mbuf *m = m0; if (level != SOL_SOCKET) { if (so->so_proto && so->so_proto->pr_ctloutput) { NET_LOCK(s); error = (*so->so_proto->pr_ctloutput)(PRCO_SETOPT, so, - level, optname, m0); + level, optname, m); NET_UNLOCK(s); return (error); } @@ -1707,7 +1706,7 @@ sosetopt(struct socket *so, int level, i level = dom->dom_protosw->pr_protocol; NET_LOCK(s); error = (*so->so_proto->pr_ctloutput) - (PRCO_SETOPT, so, level, optname, m0); + (PRCO_SETOPT, so, level, optname, m); NET_UNLOCK(s); return (error); } @@ -1739,14 +1738,11 @@ sosetopt(struct socket *so, int level, i if (error == 0 && so->so_proto && so->so_proto->pr_ctloutput) { NET_LOCK(s); (*so->so_proto->pr_ctloutput)(PRCO_SETOPT, so, - level, optname, m0); + level, optname, m); NET_UNLOCK(s); - m = NULL; /* freed by protocol */ } } 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.148 diff -u -p -r1.148 uipc_syscalls.c --- kern/uipc_syscalls.c26 Jan 2017 01:58:00 - 1.148 +++ kern/uipc_syscalls.c3 Feb 2017 16:00:26 - @@ -962,19 +962,13 @@ sys_setsockopt(struct proc *p, void *v, goto bad; } } - if (m == NULL) { - error = ENOBUFS; - goto bad; - } error = copyin(SCARG(uap, val), mtod(m, caddr_t), SCARG(uap, valsize)); - if (error) { + if (error) goto bad; - } m->m_len = SCARG(uap, valsize); } error = sosetopt(fp->f_data, SCARG(uap, level), SCARG(uap, name), m); - 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.58 diff -u -p -r1.58 bfd.c --- net/bfd.c 24 Jan 2017 10:08:30 - 1.58 +++ net/bfd.c 3 Feb 2017 16:00:26 - @@ -418,7 +418,7 @@ bfd_listener(struct bfd_config *bfd, uns struct sockaddr_in *sin; struct sockaddr_in6 *sin6; struct socket *so; - struct mbuf *m = NULL, *mopt = NULL; + struct mbuf *m = NULL, *mopt; int *ip, error; /* sa_family and sa_len must be equal */ @@ -437,6 +437,7 @@ bfd_listener(struct bfd_config *bfd, uns ip = mtod(mopt, int *); *ip = MAXTTL; error = sosetopt(so, IPPROTO_IP, IP_MINTTL, mopt); + m_free(mopt); if (error) { printf("%s: sosetopt error %d\n", __func__, error); @@ -487,7 +488,7 @@ bfd_sender(struct bfd_config *bfd, unsig struct socket *so; struct rtentry *rt = bfd->bc_rt; struct proc *p = curproc; - struct mbuf *m = NULL, *
Re: make sosetopt responsible for m_free
On 02/02/17(Thu) 12:12, David Hill wrote: > On Thu, Feb 02, 2017 at 09:34:07AM +0100, Martin Pieuchot wrote: > > On 01/02/17(Wed) 19:27, David Hill wrote: > > > Hello - > > > > > > This diff makes sosetopt responsible for m_free which is much simpler. > > > Requested by bluhm@ > > > > I'd suggest to move the m_free(9) calls to sys_setsockopt(). This > > simplifies the existing code even more and will make it easier to use > > the stack for this temporary storage. > > > > New diff with mpi@'s suggestion. You forgot NFS and BFD that should now call m_free(9) after sosetopt(9). > Index: kern/uipc_socket.c > === > RCS file: /cvs/src/sys/kern/uipc_socket.c,v > retrieving revision 1.176 > diff -u -p -r1.176 uipc_socket.c > --- kern/uipc_socket.c1 Feb 2017 20:59:47 - 1.176 > +++ kern/uipc_socket.c2 Feb 2017 16:59:45 - > @@ -1551,16 +1551,15 @@ 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 s, error = 0; > - struct mbuf *m = m0; > > if (level != SOL_SOCKET) { > if (so->so_proto && so->so_proto->pr_ctloutput) { > NET_LOCK(s); > error = (*so->so_proto->pr_ctloutput)(PRCO_SETOPT, so, > - level, optname, m0); > + level, optname, m); > NET_UNLOCK(s); > return (error); > } > @@ -1707,7 +1706,7 @@ sosetopt(struct socket *so, int level, i > level = dom->dom_protosw->pr_protocol; > NET_LOCK(s); > error = (*so->so_proto->pr_ctloutput) > - (PRCO_SETOPT, so, level, optname, m0); > + (PRCO_SETOPT, so, level, optname, m); > NET_UNLOCK(s); > return (error); > } > @@ -1739,14 +1738,11 @@ sosetopt(struct socket *so, int level, i > if (error == 0 && so->so_proto && so->so_proto->pr_ctloutput) { > NET_LOCK(s); > (*so->so_proto->pr_ctloutput)(PRCO_SETOPT, so, > - level, optname, m0); > + level, optname, m); > NET_UNLOCK(s); > - m = NULL; /* freed by protocol */ > } > } > 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.148 > diff -u -p -r1.148 uipc_syscalls.c > --- kern/uipc_syscalls.c 26 Jan 2017 01:58:00 - 1.148 > +++ kern/uipc_syscalls.c 2 Feb 2017 16:59:45 - > @@ -962,19 +962,13 @@ sys_setsockopt(struct proc *p, void *v, > goto bad; > } > } > - if (m == NULL) { > - error = ENOBUFS; > - goto bad; > - } > error = copyin(SCARG(uap, val), mtod(m, caddr_t), > SCARG(uap, valsize)); > - if (error) { > + if (error) > goto bad; > - } > m->m_len = SCARG(uap, valsize); > } > error = sosetopt(fp->f_data, SCARG(uap, level), SCARG(uap, name), m); > - m = NULL; > bad: > m_freem(m); > FRELE(fp, p); > Index: net/rtsock.c > === > RCS file: /cvs/src/sys/net/rtsock.c,v > retrieving revision 1.222 > diff -u -p -r1.222 rtsock.c > --- net/rtsock.c 1 Feb 2017 20:59:47 - 1.222 > +++ net/rtsock.c 2 Feb 2017 16:59:45 - > @@ -240,12 +240,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: > @@ -271,7 +267,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.108 > diff -u -p -r1.108 ip_mroute.c > --- netinet/ip_mroute.c 1 Feb 2017 20:59:47 -
Re: make sosetopt responsible for m_free
On Thu, Feb 02, 2017 at 09:34:07AM +0100, Martin Pieuchot wrote: > On 01/02/17(Wed) 19:27, David Hill wrote: > > Hello - > > > > This diff makes sosetopt responsible for m_free which is much simpler. > > Requested by bluhm@ > > I'd suggest to move the m_free(9) calls to sys_setsockopt(). This > simplifies the existing code even more and will make it easier to use > the stack for this temporary storage. > New diff with mpi@'s suggestion. Index: kern/uipc_socket.c === RCS file: /cvs/src/sys/kern/uipc_socket.c,v retrieving revision 1.176 diff -u -p -r1.176 uipc_socket.c --- kern/uipc_socket.c 1 Feb 2017 20:59:47 - 1.176 +++ kern/uipc_socket.c 2 Feb 2017 16:59:45 - @@ -1551,16 +1551,15 @@ 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 s, error = 0; - struct mbuf *m = m0; if (level != SOL_SOCKET) { if (so->so_proto && so->so_proto->pr_ctloutput) { NET_LOCK(s); error = (*so->so_proto->pr_ctloutput)(PRCO_SETOPT, so, - level, optname, m0); + level, optname, m); NET_UNLOCK(s); return (error); } @@ -1707,7 +1706,7 @@ sosetopt(struct socket *so, int level, i level = dom->dom_protosw->pr_protocol; NET_LOCK(s); error = (*so->so_proto->pr_ctloutput) - (PRCO_SETOPT, so, level, optname, m0); + (PRCO_SETOPT, so, level, optname, m); NET_UNLOCK(s); return (error); } @@ -1739,14 +1738,11 @@ sosetopt(struct socket *so, int level, i if (error == 0 && so->so_proto && so->so_proto->pr_ctloutput) { NET_LOCK(s); (*so->so_proto->pr_ctloutput)(PRCO_SETOPT, so, - level, optname, m0); + level, optname, m); NET_UNLOCK(s); - m = NULL; /* freed by protocol */ } } 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.148 diff -u -p -r1.148 uipc_syscalls.c --- kern/uipc_syscalls.c26 Jan 2017 01:58:00 - 1.148 +++ kern/uipc_syscalls.c2 Feb 2017 16:59:45 - @@ -962,19 +962,13 @@ sys_setsockopt(struct proc *p, void *v, goto bad; } } - if (m == NULL) { - error = ENOBUFS; - goto bad; - } error = copyin(SCARG(uap, val), mtod(m, caddr_t), SCARG(uap, valsize)); - if (error) { + if (error) goto bad; - } m->m_len = SCARG(uap, valsize); } error = sosetopt(fp->f_data, SCARG(uap, level), SCARG(uap, name), m); - m = NULL; bad: m_freem(m); FRELE(fp, p); Index: net/rtsock.c === RCS file: /cvs/src/sys/net/rtsock.c,v retrieving revision 1.222 diff -u -p -r1.222 rtsock.c --- net/rtsock.c1 Feb 2017 20:59:47 - 1.222 +++ net/rtsock.c2 Feb 2017 16:59:45 - @@ -240,12 +240,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: @@ -271,7 +267,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.108 diff -u -p -r1.108 ip_mroute.c --- netinet/ip_mroute.c 1 Feb 2017 20:59:47 - 1.108 +++ netinet/ip_mroute.c 2 Feb 2017 16:59:46 - @@ -209,7 +209,6 @@ ip_mrouter_set(struct socket *so, int op break; } - m_free(m); return (error); } Index: netinet/ip_outp
Re: make sosetopt responsible for m_free
On 01/02/17(Wed) 19:27, David Hill wrote: > Hello - > > This diff makes sosetopt responsible for m_free which is much simpler. > Requested by bluhm@ I'd suggest to move the m_free(9) calls to sys_setsockopt(). This simplifies the existing code even more and will make it easier to use the stack for this temporary storage. > > Index: kern/uipc_socket.c > === > RCS file: /cvs/src/sys/kern/uipc_socket.c,v > retrieving revision 1.176 > diff -u -p -r1.176 uipc_socket.c > --- kern/uipc_socket.c1 Feb 2017 20:59:47 - 1.176 > +++ kern/uipc_socket.c2 Feb 2017 00:13:23 - > @@ -1562,6 +1562,7 @@ sosetopt(struct socket *so, int level, i > error = (*so->so_proto->pr_ctloutput)(PRCO_SETOPT, so, > level, optname, m0); > NET_UNLOCK(s); > + m_free(m0); > return (error); > } > error = ENOPROTOOPT; > @@ -1709,6 +1710,7 @@ sosetopt(struct socket *so, int level, i > error = (*so->so_proto->pr_ctloutput) > (PRCO_SETOPT, so, level, optname, m0); > NET_UNLOCK(s); > + m_free(m0); > return (error); > } > error = ENOPROTOOPT; > @@ -1741,7 +1743,8 @@ sosetopt(struct socket *so, int level, i > (*so->so_proto->pr_ctloutput)(PRCO_SETOPT, so, > level, optname, m0); > NET_UNLOCK(s); > - m = NULL; /* freed by protocol */ > + m_free(m0); > + m = NULL; > } > } > bad: > Index: net/rtsock.c > === > RCS file: /cvs/src/sys/net/rtsock.c,v > retrieving revision 1.222 > diff -u -p -r1.222 rtsock.c > --- net/rtsock.c 1 Feb 2017 20:59:47 - 1.222 > +++ net/rtsock.c 2 Feb 2017 00:13:23 - > @@ -240,12 +240,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: > @@ -271,7 +267,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.108 > diff -u -p -r1.108 ip_mroute.c > --- netinet/ip_mroute.c 1 Feb 2017 20:59:47 - 1.108 > +++ netinet/ip_mroute.c 2 Feb 2017 00:13:23 - > @@ -209,7 +209,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.335 > diff -u -p -r1.335 ip_output.c > --- netinet/ip_output.c 1 Feb 2017 20:59:47 - 1.335 > +++ netinet/ip_output.c 2 Feb 2017 00:13:23 - > @@ -853,11 +853,10 @@ ip_ctloutput(int op, struct socket *so, > int error = 0; > u_int rtid = 0; > > - if (level != IPPROTO_IP) { > - error = EINVAL; > - if (op == PRCO_SETOPT) > - (void) m_free(m); > - } else switch (op) { > + if (level != IPPROTO_IP) > + return EINVAL; > + > + switch (op) { > case PRCO_SETOPT: > switch (optname) { > case IP_OPTIONS: > @@ -1073,7 +1072,6 @@ ip_ctloutput(int op, struct socket *so, > error = ENOPROTOOPT; > break; > } > - m_free(m); > break; > > case PRCO_GETOPT: > @@ -1235,12 +1233,11 @@ ip_pcbopts(struct mbuf **pcbopt, struct > > /* turn off any old options */ > m_free(*pcbopt); > - *pcbopt = 0; > + *pcbopt = NULL; > if (m == NULL || m->m_len == 0) { > /* >* Only turning off any previous options. >*/ > - m_free(m); > return (0); > } > > @@ -1316,7 +1313,6 @@ ip_pcbopts(struct mbuf **pcbopt, struct > return (0); > > bad: > - (void)m_free(m); > return (EINVAL); > } > > Index: netinet/raw_ip.c > ==
make sosetopt responsible for m_free
Hello - This diff makes sosetopt responsible for m_free which is much simpler. Requested by bluhm@ Index: kern/uipc_socket.c === RCS file: /cvs/src/sys/kern/uipc_socket.c,v retrieving revision 1.176 diff -u -p -r1.176 uipc_socket.c --- kern/uipc_socket.c 1 Feb 2017 20:59:47 - 1.176 +++ kern/uipc_socket.c 2 Feb 2017 00:13:23 - @@ -1562,6 +1562,7 @@ sosetopt(struct socket *so, int level, i error = (*so->so_proto->pr_ctloutput)(PRCO_SETOPT, so, level, optname, m0); NET_UNLOCK(s); + m_free(m0); return (error); } error = ENOPROTOOPT; @@ -1709,6 +1710,7 @@ sosetopt(struct socket *so, int level, i error = (*so->so_proto->pr_ctloutput) (PRCO_SETOPT, so, level, optname, m0); NET_UNLOCK(s); + m_free(m0); return (error); } error = ENOPROTOOPT; @@ -1741,7 +1743,8 @@ sosetopt(struct socket *so, int level, i (*so->so_proto->pr_ctloutput)(PRCO_SETOPT, so, level, optname, m0); NET_UNLOCK(s); - m = NULL; /* freed by protocol */ + m_free(m0); + m = NULL; } } bad: Index: net/rtsock.c === RCS file: /cvs/src/sys/net/rtsock.c,v retrieving revision 1.222 diff -u -p -r1.222 rtsock.c --- net/rtsock.c1 Feb 2017 20:59:47 - 1.222 +++ net/rtsock.c2 Feb 2017 00:13:23 - @@ -240,12 +240,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: @@ -271,7 +267,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.108 diff -u -p -r1.108 ip_mroute.c --- netinet/ip_mroute.c 1 Feb 2017 20:59:47 - 1.108 +++ netinet/ip_mroute.c 2 Feb 2017 00:13:23 - @@ -209,7 +209,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.335 diff -u -p -r1.335 ip_output.c --- netinet/ip_output.c 1 Feb 2017 20:59:47 - 1.335 +++ netinet/ip_output.c 2 Feb 2017 00:13:23 - @@ -853,11 +853,10 @@ ip_ctloutput(int op, struct socket *so, int error = 0; u_int rtid = 0; - if (level != IPPROTO_IP) { - error = EINVAL; - if (op == PRCO_SETOPT) - (void) m_free(m); - } else switch (op) { + if (level != IPPROTO_IP) + return EINVAL; + + switch (op) { case PRCO_SETOPT: switch (optname) { case IP_OPTIONS: @@ -1073,7 +1072,6 @@ ip_ctloutput(int op, struct socket *so, error = ENOPROTOOPT; break; } - m_free(m); break; case PRCO_GETOPT: @@ -1235,12 +1233,11 @@ ip_pcbopts(struct mbuf **pcbopt, struct /* turn off any old options */ m_free(*pcbopt); - *pcbopt = 0; + *pcbopt = NULL; if (m == NULL || m->m_len == 0) { /* * Only turning off any previous options. */ - m_free(m); return (0); } @@ -1316,7 +1313,6 @@ ip_pcbopts(struct mbuf **pcbopt, struct return (0); bad: - (void)m_free(m); return (EINVAL); } Index: netinet/raw_ip.c === RCS file: /cvs/src/sys/netinet/raw_ip.c,v retrieving revision 1.95 diff -u -p -r1.95 raw_ip.c --- netinet/raw_ip.c1 Feb 2017 20:59:47 - 1.95 +++ netinet/raw_ip.c2 Feb 2017 00:13:23 - @@ -305,11 +305,8 @@ rip_ctloutput(int op, struct socket *so, int error = 0; int dir; - if (level != IPPROTO_I