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
>
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:
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 !=
