Re: ip_input() in pr_input style
On 30/05/17(Tue) 11:40, Alexander Bluhm wrote: > On Tue, May 30, 2017 at 08:45:53AM +0200, Martin Pieuchot wrote: > > On 29/05/17(Mon) 23:45, Alexander Bluhm wrote: > > > Hi, > > > > > > Convert ip_input(), ip_our(), ip_deliver() functions to pr_input > > > parameter passing and protocol return style. Reset mp to NULL in > > > a few places to fail at mbuf use after free. Rename ipv4_input() > > > to ip_input(). > > > > > > Goal is to prepare the code that both mpi@'s and bluhm@'s diff > > > apply. > > > > > > ok? > > > > I don't understand how I'm suppose to rebase my diff on top of this > > one. ip_ours() is now taking multiple arguments. > > My diff has to pass down the mp from ip_input() to ip_deliver(). > Your diff places a queue into that path. The queue is temporary > and can go away when we unlock the protocol input path. In the meantime we cannot make in_ours() return anything. So I still don't understand how our diffs are compatible.
Re: ip_input() in pr_input style
On Tue, May 30, 2017 at 08:45:53AM +0200, Martin Pieuchot wrote: > On 29/05/17(Mon) 23:45, Alexander Bluhm wrote: > > Hi, > > > > Convert ip_input(), ip_our(), ip_deliver() functions to pr_input > > parameter passing and protocol return style. Reset mp to NULL in > > a few places to fail at mbuf use after free. Rename ipv4_input() > > to ip_input(). > > > > Goal is to prepare the code that both mpi@'s and bluhm@'s diff > > apply. > > > > ok? > > I don't understand how I'm suppose to rebase my diff on top of this > one. ip_ours() is now taking multiple arguments. My diff has to pass down the mp from ip_input() to ip_deliver(). Your diff places a queue into that path. The queue is temporary and can go away when we unlock the protocol input path. I am trying to create incremental diffs that cover both cases. The alternative is, that we commit my diff or your diff now and merge the other one. I can also convert my diff that all functions take the minimum number of arguments. Currently my goal is that all IPv4 and IPv6 input functions behave like pr_input for consistency. That is ip_input ip_ours ip_deliver ip6_input ip6_ours ip6_deliver ip6_hbhchcheck bluhm
Re: ip_input() in pr_input style
On 29/05/17(Mon) 23:45, Alexander Bluhm wrote:
> Hi,
>
> Convert ip_input(), ip_our(), ip_deliver() functions to pr_input
> parameter passing and protocol return style. Reset mp to NULL in
> a few places to fail at mbuf use after free. Rename ipv4_input()
> to ip_input().
>
> Goal is to prepare the code that both mpi@'s and bluhm@'s diff
> apply.
>
> ok?
I don't understand how I'm suppose to rebase my diff on top of this
one. ip_ours() is now taking multiple arguments.
> Index: netinet/ip_input.c
> ===
> RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/ip_input.c,v
> retrieving revision 1.306
> diff -u -p -r1.306 ip_input.c
> --- netinet/ip_input.c28 May 2017 12:22:54 - 1.306
> +++ netinet/ip_input.c29 May 2017 21:38:51 -
> @@ -126,7 +126,7 @@ int ip_sysctl_ipstat(void *, size_t *, v
>
> static struct mbuf_queue ipsend_mq;
>
> -void ip_ours(struct mbuf *);
> +int ip_ours(struct mbuf **, int *, int, int);
> int ip_dooptions(struct mbuf *, struct ifnet *);
> int in_ouraddr(struct mbuf *, struct ifnet *, struct rtentry **);
>
> @@ -211,6 +211,7 @@ void
> ipintr(void)
> {
> struct mbuf *m;
> + int off;
>
> /*
>* Get next datagram off input queue and get IP header
> @@ -221,7 +222,8 @@ ipintr(void)
> if ((m->m_flags & M_PKTHDR) == 0)
> panic("ipintr no HDR");
> #endif
> - ipv4_input(m);
> + off = 0;
> + ip_input(, , IPPROTO_IPV4, AF_UNSPEC);
> }
> }
>
> @@ -230,39 +232,42 @@ ipintr(void)
> *
> * Checksum and byte swap header. Process options. Forward or deliver.
> */
> -void
> -ipv4_input(struct mbuf *m)
> +int
> +ip_input(struct mbuf **mp, int *offp, int nxt, int af)
> {
> + struct mbuf *m = *mp;
> struct ifnet*ifp;
> struct rtentry *rt = NULL;
> struct ip *ip;
> int hlen, len;
> in_addr_t pfrdr = 0;
>
> + KASSERT(*offp == 0);
> +
> ifp = if_get(m->m_pkthdr.ph_ifidx);
> if (ifp == NULL)
> - goto bad;
> + goto done;
>
> ipstat_inc(ips_total);
> if (m->m_len < sizeof (struct ip) &&
> - (m = m_pullup(m, sizeof (struct ip))) == NULL) {
> + (m = *mp = m_pullup(m, sizeof (struct ip))) == NULL) {
> ipstat_inc(ips_toosmall);
> - goto out;
> + goto done;
> }
> ip = mtod(m, struct ip *);
> if (ip->ip_v != IPVERSION) {
> ipstat_inc(ips_badvers);
> - goto bad;
> + goto done;
> }
> hlen = ip->ip_hl << 2;
> if (hlen < sizeof(struct ip)) { /* minimum header length */
> ipstat_inc(ips_badhlen);
> - goto bad;
> + goto done;
> }
> if (hlen > m->m_len) {
> - if ((m = m_pullup(m, hlen)) == NULL) {
> + if ((m = *mp = m_pullup(m, hlen)) == NULL) {
> ipstat_inc(ips_badhlen);
> - goto out;
> + goto done;
> }
> ip = mtod(m, struct ip *);
> }
> @@ -272,20 +277,20 @@ ipv4_input(struct mbuf *m)
> (ntohl(ip->ip_src.s_addr) >> IN_CLASSA_NSHIFT) == IN_LOOPBACKNET) {
> if ((ifp->if_flags & IFF_LOOPBACK) == 0) {
> ipstat_inc(ips_badaddr);
> - goto bad;
> + goto done;
> }
> }
>
> if ((m->m_pkthdr.csum_flags & M_IPV4_CSUM_IN_OK) == 0) {
> if (m->m_pkthdr.csum_flags & M_IPV4_CSUM_IN_BAD) {
> ipstat_inc(ips_badsum);
> - goto bad;
> + goto done;
> }
>
> ipstat_inc(ips_inswcsum);
> if (in_cksum(m, hlen) != 0) {
> ipstat_inc(ips_badsum);
> - goto bad;
> + goto done;
> }
> }
>
> @@ -297,7 +302,7 @@ ipv4_input(struct mbuf *m)
>*/
> if (len < hlen) {
> ipstat_inc(ips_badlen);
> - goto bad;
> + goto done;
> }
>
> /*
> @@ -308,7 +313,7 @@ ipv4_input(struct mbuf *m)
>*/
> if (m->m_pkthdr.len < len) {
> ipstat_inc(ips_tooshort);
> - goto bad;
> + goto done;
> }
> if (m->m_pkthdr.len > len) {
> if (m->m_len == m->m_pkthdr.len) {
> @@ -321,7 +326,7 @@ ipv4_input(struct mbuf *m)
> #if NCARP > 0
> if (ifp->if_type == IFT_CARP && ip->ip_p != IPPROTO_ICMP &&
> carp_lsdrop(m, AF_INET, >ip_src.s_addr, >ip_dst.s_addr))
> - goto bad;
> + goto done;
> #endif
>
> #if NPF > 0
> @@ -329,10 +334,11 @@ ipv4_input(struct mbuf *m)
>* Packet filter
>*/
> pfrdr = ip->ip_dst.s_addr;
> - if (pf_test(AF_INET, PF_IN, ifp, ) !=
ip_input() in pr_input style
Hi,
Convert ip_input(), ip_our(), ip_deliver() functions to pr_input
parameter passing and protocol return style. Reset mp to NULL in
a few places to fail at mbuf use after free. Rename ipv4_input()
to ip_input().
Goal is to prepare the code that both mpi@'s and bluhm@'s diff
apply.
ok?
bluhm
Index: netinet/ip_input.c
===
RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/ip_input.c,v
retrieving revision 1.306
diff -u -p -r1.306 ip_input.c
--- netinet/ip_input.c 28 May 2017 12:22:54 - 1.306
+++ netinet/ip_input.c 29 May 2017 21:38:51 -
@@ -126,7 +126,7 @@ int ip_sysctl_ipstat(void *, size_t *, v
static struct mbuf_queue ipsend_mq;
-void ip_ours(struct mbuf *);
+intip_ours(struct mbuf **, int *, int, int);
intip_dooptions(struct mbuf *, struct ifnet *);
intin_ouraddr(struct mbuf *, struct ifnet *, struct rtentry **);
@@ -211,6 +211,7 @@ void
ipintr(void)
{
struct mbuf *m;
+ int off;
/*
* Get next datagram off input queue and get IP header
@@ -221,7 +222,8 @@ ipintr(void)
if ((m->m_flags & M_PKTHDR) == 0)
panic("ipintr no HDR");
#endif
- ipv4_input(m);
+ off = 0;
+ ip_input(, , IPPROTO_IPV4, AF_UNSPEC);
}
}
@@ -230,39 +232,42 @@ ipintr(void)
*
* Checksum and byte swap header. Process options. Forward or deliver.
*/
-void
-ipv4_input(struct mbuf *m)
+int
+ip_input(struct mbuf **mp, int *offp, int nxt, int af)
{
+ struct mbuf *m = *mp;
struct ifnet*ifp;
struct rtentry *rt = NULL;
struct ip *ip;
int hlen, len;
in_addr_t pfrdr = 0;
+ KASSERT(*offp == 0);
+
ifp = if_get(m->m_pkthdr.ph_ifidx);
if (ifp == NULL)
- goto bad;
+ goto done;
ipstat_inc(ips_total);
if (m->m_len < sizeof (struct ip) &&
- (m = m_pullup(m, sizeof (struct ip))) == NULL) {
+ (m = *mp = m_pullup(m, sizeof (struct ip))) == NULL) {
ipstat_inc(ips_toosmall);
- goto out;
+ goto done;
}
ip = mtod(m, struct ip *);
if (ip->ip_v != IPVERSION) {
ipstat_inc(ips_badvers);
- goto bad;
+ goto done;
}
hlen = ip->ip_hl << 2;
if (hlen < sizeof(struct ip)) { /* minimum header length */
ipstat_inc(ips_badhlen);
- goto bad;
+ goto done;
}
if (hlen > m->m_len) {
- if ((m = m_pullup(m, hlen)) == NULL) {
+ if ((m = *mp = m_pullup(m, hlen)) == NULL) {
ipstat_inc(ips_badhlen);
- goto out;
+ goto done;
}
ip = mtod(m, struct ip *);
}
@@ -272,20 +277,20 @@ ipv4_input(struct mbuf *m)
(ntohl(ip->ip_src.s_addr) >> IN_CLASSA_NSHIFT) == IN_LOOPBACKNET) {
if ((ifp->if_flags & IFF_LOOPBACK) == 0) {
ipstat_inc(ips_badaddr);
- goto bad;
+ goto done;
}
}
if ((m->m_pkthdr.csum_flags & M_IPV4_CSUM_IN_OK) == 0) {
if (m->m_pkthdr.csum_flags & M_IPV4_CSUM_IN_BAD) {
ipstat_inc(ips_badsum);
- goto bad;
+ goto done;
}
ipstat_inc(ips_inswcsum);
if (in_cksum(m, hlen) != 0) {
ipstat_inc(ips_badsum);
- goto bad;
+ goto done;
}
}
@@ -297,7 +302,7 @@ ipv4_input(struct mbuf *m)
*/
if (len < hlen) {
ipstat_inc(ips_badlen);
- goto bad;
+ goto done;
}
/*
@@ -308,7 +313,7 @@ ipv4_input(struct mbuf *m)
*/
if (m->m_pkthdr.len < len) {
ipstat_inc(ips_tooshort);
- goto bad;
+ goto done;
}
if (m->m_pkthdr.len > len) {
if (m->m_len == m->m_pkthdr.len) {
@@ -321,7 +326,7 @@ ipv4_input(struct mbuf *m)
#if NCARP > 0
if (ifp->if_type == IFT_CARP && ip->ip_p != IPPROTO_ICMP &&
carp_lsdrop(m, AF_INET, >ip_src.s_addr, >ip_dst.s_addr))
- goto bad;
+ goto done;
#endif
#if NPF > 0
@@ -329,10 +334,11 @@ ipv4_input(struct mbuf *m)
* Packet filter
*/
pfrdr = ip->ip_dst.s_addr;
- if (pf_test(AF_INET, PF_IN, ifp, ) != PF_PASS)
- goto bad;
+ if (pf_test(AF_INET, PF_IN, ifp, mp) != PF_PASS)
+ goto done;
+ m = *mp;
if (m == NULL)
- goto out;
+ goto done;
ip = mtod(m, struct ip *);
hlen = ip->ip_hl << 2;
@@
