Attachment 239857 Details for Bug 266442 – Also add to the fr_bad packet count.

[patch] Also add to the fr_bad packet count.

PR266442.diff (text/plain), 3.94 KB, created by Cy Schubert on 2023-02-02 02:47:28 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Cy Schubert

Created: 2023-02-02 02:47:28 UTC

Size: 3.94 KB

patch

obsolete

>From 45ba7eee00c82f9f867983c902802285fb082435 Mon Sep 17 00:00:00 2001
>From: Cy Schubert <cy@FreeBSD.org>
>Date: Tue, 31 Jan 2023 11:09:00 -0800
>Subject: [PATCH 1/2] ipfilter: Correctly type ipf_pullup()
>
>ipf_pullup() outputs a pointer to ip_t. Though returning a pointer to
>void does work, it is imprecise not completely correct.
>
>MFC after:	1 week
>---
> sys/netpfil/ipfilter/netinet/ip_fil.h         |  2 +-
> sys/netpfil/ipfilter/netinet/ip_fil_freebsd.c | 11 ++++++-----
> 2 files changed, 7 insertions(+), 6 deletions(-)
>
>diff --git a/sys/netpfil/ipfilter/netinet/ip_fil.h b/sys/netpfil/ipfilter/netinet/ip_fil.h
>index 85a79eda1172..002ddfdc8348 100644
>--- a/sys/netpfil/ipfilter/netinet/ip_fil.h
>+++ b/sys/netpfil/ipfilter/netinet/ip_fil.h
>@@ -1700,7 +1700,7 @@ extern	int	ipf_outobj(ipf_main_softc_t *, void *, void *, int);
> extern	int	ipf_outobjk(ipf_main_softc_t *, ipfobj_t *, void *);
> extern	int	ipf_outobjsz(ipf_main_softc_t *, void *, void *,
> 				  int, int);
>-extern	void	*ipf_pullup(mb_t *, fr_info_t *, int);
>+extern	ip_t	*ipf_pullup(mb_t *, fr_info_t *, int);
> extern	int	ipf_resolvedest(ipf_main_softc_t *, char *,
> 				     struct frdest *, int);
> extern	int	ipf_resolvefunc(ipf_main_softc_t *, void *);
>diff --git a/sys/netpfil/ipfilter/netinet/ip_fil_freebsd.c b/sys/netpfil/ipfilter/netinet/ip_fil_freebsd.c
>index b2ee855c3854..0dfc23d3dd1f 100644
>--- a/sys/netpfil/ipfilter/netinet/ip_fil_freebsd.c
>+++ b/sys/netpfil/ipfilter/netinet/ip_fil_freebsd.c
>@@ -1178,17 +1178,17 @@ mbufchainlen(struct mbuf *m0)
> /* We assume that 'xmin' is a pointer to a buffer that is part of the chain */
> /* of buffers that starts at *fin->fin_mp.                                  */
> /* ------------------------------------------------------------------------ */
>-void *
>+ip_t *
> ipf_pullup(mb_t *xmin, fr_info_t *fin, int len)
> {
> 	int dpoff, ipoff;
> 	mb_t *m = xmin;
>-	char *ip;
>+	ip_t *ip;
> 
> 	if (m == NULL)
> 		return (NULL);
> 
>-	ip = (char *)fin->fin_ip;
>+	ip = fin->fin_ip;
> 	if ((fin->fin_flx & FI_COALESCE) != 0)
> 		return (ip);
> 
>@@ -1233,6 +1233,7 @@ ipf_pullup(mb_t *xmin, fr_info_t *fin, int len)
> #endif
> 		} else
> 		{
>+
> 			m = m_pullup(m, len);
> 		}
> 		if (n != NULL)
>@@ -1259,9 +1260,9 @@ ipf_pullup(mb_t *xmin, fr_info_t *fin, int len)
> 			m = m->m_next;
> 		}
> 		fin->fin_m = m;
>-		ip = MTOD(m, char *) + ipoff;
>+		ip = MTOD(m, ip_t *) + ipoff;
> 
>-		fin->fin_ip = (ip_t *)ip;
>+		fin->fin_ip = ip;
> 		if (fin->fin_dp != NULL)
> 			fin->fin_dp = (char *)fin->fin_ip + dpoff;
> 		if (fin->fin_fraghdr != NULL)
>-- 
>2.39.1
>
>From 6c2983167f530b88690e3a8062411b7a64c3abe8 Mon Sep 17 00:00:00 2001
>From: Cy Schubert <cy@FreeBSD.org>
>Date: Wed, 1 Feb 2023 16:49:08 -0800
>Subject: [PATCH 2/2] ipfilter: Fix use after free on packet with broken
> lengths
>
>Under the scenario with a packet with length of 67 bytes, a header length
>using the default of 20 bytes and a TCP data offset (th_off) of 48 will
>cause m_pullup() to fail to make sure bytes are arragned contiguously.
>m_pullup() will free the mbuf chain and return a null. ipfilter stores
>the resultant mbuf address (or the resulting NULL) in its fr_info_t
>structure. Unfortuntely the eroneous packet is not flagged for drop.
>This results in a kernel page fault at line 410 of sys/netinet/ip_fastfwd.c
>as it tries to use a now previously freed, by m_pullup(), mbuf.
>
>PR:		266442
>Reported by:	Robert Morris <rtm@lcs.mit.edu>
>MFC after:	1 week
>---
> sys/netpfil/ipfilter/netinet/fil.c | 7 +++++++
> 1 file changed, 7 insertions(+)
>
>diff --git a/sys/netpfil/ipfilter/netinet/fil.c b/sys/netpfil/ipfilter/netinet/fil.c
>index 5ced528d1263..0d8410e08425 100644
>--- a/sys/netpfil/ipfilter/netinet/fil.c
>+++ b/sys/netpfil/ipfilter/netinet/fil.c
>@@ -3180,6 +3180,13 @@ ipf_check(void *ctx, ip_t *ip, int hlen, struct ifnet *ifp, int out
> 
> 	SPL_X(s);
> 
>+	if (fin->fin_m == NULL) {
>+		LBUMP(ipf_stats[out].fr_bad);
>+		/* m_pullup() has freed the mbuf */
>+		DT1(ipf_mbuf_null, fr_info_t *, fin);
>+		return (-1);
>+	}
>+
> #ifdef _KERNEL
> 	if (FR_ISPASS(pass))
> 		return (0);
>-- 
>2.39.1
>

Actions: View | Diff

Attachments on bug 266442: 236590 | 239852 | 239853 | 239857