Page MenuHomeFreeBSD
Differential D45619

net80211: separate out the net80211 ioctl key size from the kernel definition
Needs ReviewPublic
Actions

Authored by adrian on Jun 16 2024, 11:30 PM.
Tags
Referenced Files
Unknown Object (File)
Fri, Apr 11, 10:39 AM
Unknown Object (File)
Tue, Apr 8, 5:50 PM
Unknown Object (File)
Sat, Apr 5, 10:38 AM
Unknown Object (File)
Sat, Apr 5, 3:11 AM
Unknown Object (File)
Wed, Mar 19, 4:33 PM
Unknown Object (File)
Feb 9 2025, 7:03 PM
Unknown Object (File)
Feb 4 2025, 11:08 AM
Unknown Object (File)
Feb 3 2025, 3:12 AM
View All 79 Files
Subscribers
glebius
imp
melifaro

Details

Reviewers
None
Group Reviewers
wireless
Summary

This WIP just redefines the struct ieee80211req_key used in key get/set
ioctl calls to a separate set of values. The eventual goal is to fully
separate out the ioctl API from the internal key definition sizes so
things like 256 / 384 bit key infrastructure can be included without
needing to also break the default userland ABI.

Note this diff is a WIP - I haven't yet updated the ioctl key get and
set functions themselves to take the copy sizes into account when
copying the key and MIC contents. I will also need to ensure any
unused buffer contents are bzero'ed out rather than copied and leaked.

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Skipped
Unit
Tests Skipped
Build Status
Buildable 58222
Build 55110: arc lint + arc unit

Event Timeline

adrian created this revision.Jun 16 2024, 11:30 PM
Herald added subscribers: glebius, melifaro, imp. · View Herald TranscriptJun 16 2024, 11:30 PM
adrian requested review of this revision.Jun 16 2024, 11:30 PM
Harbormaster completed remote builds in B58203: Diff 139915.Jun 16 2024, 11:31 PM
adrian updated this revision to Diff 139916.Jun 16 2024, 11:31 PM

add url

adrian added a parent revision: D45516: net80211: migrate the group/unicast key check into inline functions.Jun 16 2024, 11:31 PM
adrian added a reviewer: wireless.
adrian added a project: wireless.
Harbormaster completed remote builds in B58204: Diff 139916.Jun 16 2024, 11:32 PM
adrian updated this revision to Diff 139939.Jun 17 2024, 11:49 PM

address comments from bz@

Harbormaster completed remote builds in B58211: Diff 139939.Jun 17 2024, 11:49 PM
adrian updated this revision to Diff 139959.Jun 18 2024, 6:53 PM

more updates

Harbormaster completed remote builds in B58222: Diff 139959.Jun 18 2024, 6:53 PM
adrian added a parent revision: D45629: net80211: add maximum key length for each cipher.Jun 18 2024, 7:30 PM
adrian removed a parent revision: D45516: net80211: migrate the group/unicast key check into inline functions.
adrian updated this revision to Diff 139966.Jun 18 2024, 8:43 PM

update

Harbormaster completed remote builds in B58228: Diff 139966.Jun 18 2024, 8:44 PM
adrian added a child revision: D45887: net80211: add a placeholder check routine for whether a key is an IGTK key.Jul 5 2024, 8:53 PM

Revision Contents
Changeset List

PathSize
sys/
net80211/
10 lines
31 lines

Diff 139959

View Options

sys/net80211/ieee80211_ioctl.h

Show First 20 Lines • Show All 327 Lines • ▼ Show 20 Lines
static int bridge_ioctl_sproto(struct bridge_softc *, void *); static int bridge_ioctl_sproto(struct bridge_softc *, void *);
static int bridge_ioctl_stxhc(struct bridge_softc *, void *); static int bridge_ioctl_stxhc(struct bridge_softc *, void *);
static int bridge_pfil(struct mbuf **, struct ifnet *, struct ifnet *, static int bridge_pfil(struct mbuf **, struct ifnet *, struct ifnet *,
int); int);
static int bridge_ip_checkbasic(struct mbuf **mp); static int bridge_ip_checkbasic(struct mbuf **mp);
#ifdef INET6 #ifdef INET6
static int bridge_ip6_checkbasic(struct mbuf **mp); static int bridge_ip6_checkbasic(struct mbuf **mp);
#endif /* INET6 */ #endif /* INET6 */
static int bridge_fragment(struct ifnet *, struct mbuf *, static int bridge_fragment(struct ifnet *, struct mbuf **mp,
struct ether_header *, int, struct llc *); struct ether_header *, int, struct llc *);
static void bridge_linkstate(struct ifnet *ifp); static void bridge_linkstate(struct ifnet *ifp);
static void bridge_linkcheck(struct bridge_softc *sc); static void bridge_linkcheck(struct bridge_softc *sc);
extern void (*bridge_linkstate_p)(struct ifnet *ifp); extern void (*bridge_linkstate_p)(struct ifnet *ifp);
/* The default bridge vlan is 1 (IEEE 802.1Q-2003 Table 9-2) */ /* The default bridge vlan is 1 (IEEE 802.1Q-2003 Table 9-2) */
#define VLANTAGOF(_m) \ #define VLANTAGOF(_m) \
▲ Show 20 Lines • Show All 1,567 Lines • ▼ Show 20 Lines if ((m->m_flags & M_VLANTAG) &&
if_printf(dst_ifp, if_printf(dst_ifp,
"unable to prepend VLAN header\n"); "unable to prepend VLAN header\n");
if_inc_counter(dst_ifp, IFCOUNTER_OERRORS, 1); if_inc_counter(dst_ifp, IFCOUNTER_OERRORS, 1);
continue; continue;
} }
m->m_flags &= ~M_VLANTAG; m->m_flags &= ~M_VLANTAG;
} }
M_ASSERTPKTHDR(m); /* We shouldn't transmit mbuf without pkthdr */
if ((err = dst_ifp->if_transmit(dst_ifp, m))) { if ((err = dst_ifp->if_transmit(dst_ifp, m))) {
m_freem(m0); m_freem(m0);
if_inc_counter(sc->sc_ifp, IFCOUNTER_OERRORS, 1); if_inc_counter(sc->sc_ifp, IFCOUNTER_OERRORS, 1);
break; break;
} }
if_inc_counter(sc->sc_ifp, IFCOUNTER_OPACKETS, 1); if_inc_counter(sc->sc_ifp, IFCOUNTER_OPACKETS, 1);
if_inc_counter(sc->sc_ifp, IFCOUNTER_OBYTES, len); if_inc_counter(sc->sc_ifp, IFCOUNTER_OBYTES, len);
▲ Show 20 Lines • Show All 1,301 Lines • ▼ Show 20 Lines case ETHERTYPE_IP:
if (V_pfil_bridge && dir == PFIL_IN && bifp != NULL) if (V_pfil_bridge && dir == PFIL_IN && bifp != NULL)
error = pfil_run_hooks(&V_inet_pfil_hook, mp, bifp, error = pfil_run_hooks(&V_inet_pfil_hook, mp, bifp,
dir, NULL); dir, NULL);
if (*mp == NULL || error != 0) /* filter may consume */ if (*mp == NULL || error != 0) /* filter may consume */
break; break;
/* check if we need to fragment the packet */ /* check if we need to fragment the packet */
/* bridge_fragment generates a mbuf chain of packets */
/* that already include eth headers */
if (V_pfil_member && ifp != NULL && dir == PFIL_OUT) { if (V_pfil_member && ifp != NULL && dir == PFIL_OUT) {
i = (*mp)->m_pkthdr.len; i = (*mp)->m_pkthdr.len;
if (i > ifp->if_mtu) { if (i > ifp->if_mtu) {
error = bridge_fragment(ifp, *mp, &eh2, snap, error = bridge_fragment(ifp, mp, &eh2, snap,
&llc1); &llc1);
return (error); return (error);
} }
} }
/* Recalculate the ip checksum. */ /* Recalculate the ip checksum. */
ip = mtod(*mp, struct ip *); ip = mtod(*mp, struct ip *);
hlen = ip->ip_hl << 2; hlen = ip->ip_hl << 2;
▲ Show 20 Lines • Show All 222 Lines • ▼ Show 20 Linesbad:
*mp = m; *mp = m;
return (-1); return (-1);
} }
#endif /* INET6 */ #endif /* INET6 */
/* /*
* bridge_fragment: * bridge_fragment:
* *
* Return a fragmented mbuf chain. * Fragment mbuf chain in multiple packets and prepend ethernet header.
*/ */
static int static int
bridge_fragment(struct ifnet *ifp, struct mbuf *m, struct ether_header *eh, bridge_fragment(struct ifnet *ifp, struct mbuf **mp, struct ether_header *eh,
int snap, struct llc *llc) int snap, struct llc *llc)
{ {
struct mbuf *m0; struct mbuf *m = *mp, *m0 = NULL, *mprev = NULL, *mcur = NULL;
struct ip *ip; struct ip *ip;
int error = -1; int error = -1;
if (m->m_len < sizeof(struct ip) && if (m->m_len < sizeof(struct ip) &&
(m = m_pullup(m, sizeof(struct ip))) == NULL) (m = m_pullup(m, sizeof(struct ip))) == NULL)
goto out; goto dropit;
ip = mtod(m, struct ip *); ip = mtod(m, struct ip *);
m->m_pkthdr.csum_flags |= CSUM_IP; m->m_pkthdr.csum_flags |= CSUM_IP;
error = ip_fragment(ip, &m, ifp->if_mtu, ifp->if_hwassist); error = ip_fragment(ip, &m, ifp->if_mtu, ifp->if_hwassist);
if (error) if (error)
goto out; goto dropit;
/* walk the chain and re-add the Ethernet header */ /*
for (m0 = m; m0; m0 = m0->m_nextpkt) { * walk the chain and re-add the Ethernet header for
if (error == 0) { * each mbuf packets
*/
for (mcur = m; mcur; mcur = mcur->m_nextpkt) {
m0 = mcur;
if (snap) { if (snap) {
M_PREPEND(m0, sizeof(struct llc), M_NOWAIT); M_PREPEND(mcur, sizeof(struct llc), M_NOWAIT);
if (m0 == NULL) {
if (mcur == NULL) {
error = ENOBUFS; error = ENOBUFS;
continue; goto dropit;
} }
bcopy(llc, mtod(m0, caddr_t), bcopy(llc, mtod(mcur, caddr_t),sizeof(struct llc));
sizeof(struct llc));
} }
kpUnsubmitted
Done
Inline Actions

Doesn't this need to go before the if (snap) block? If we end up prepending an mbuf for the snap header but not for the ethernet header we still need to do the m_next/m_nextpkt pointer reshuffle.

kp: Doesn't this need to go before the if (snap) block? If we end up prepending an mbuf for the…
M_PREPEND(m0, ETHER_HDR_LEN, M_NOWAIT);
if (m0 == NULL) { M_PREPEND(mcur, ETHER_HDR_LEN, M_NOWAIT);
if (mcur == NULL) {
error = ENOBUFS; error = ENOBUFS;
continue; goto dropit;
} }
bcopy(eh, mtod(m0, caddr_t), ETHER_HDR_LEN); bcopy(eh, mtod(mcur, caddr_t), ETHER_HDR_LEN);
} else /*
m_freem(m); * if M_PREPEND had to create a new mbuf, we need to:
* 1. update previous mbuf packet to use this new one
* 2. moving m_nextpkt from the old mbuf to the new one
* But we can have two new mbufs prepending: LLC and Ethernet HDR!
*/
kpUnsubmitted
Done
Inline Actions

Won't this break in a (worst-case) scenario where both the snap and ethernet M_PREPEND()s end up adding a new mbuf?

kp: Won't this break in a (worst-case) scenario where both the snap and ethernet M_PREPEND()s end…
if ( m0 != mcur ) {
if ( mcur->m_next->m_next && mcur->m_next->m_next->m_nextpkt ) {
/* If two new mbufs insered (LLC and Ethernet header) */
mcur->m_nextpkt = mcur->m_next->m_next->m_nextpkt;
mcur->m_next->m_next->m_nextpkt = NULL;
} else {
/* If only one mbuf insered (Ethernet header) */
mcur->m_nextpkt = mcur->m_next->m_nextpkt;
mcur->m_next->m_nextpkt = NULL;
} }
if (mprev != NULL) { /* It's not the first packet in the chain*/
mprev->m_nextpkt = mcur;
} else { /* the first mbuf in the original chain needs to be updated */
*mp = mcur;
}
}
mprev = mcur;
}
if (error == 0)
KMOD_IPSTAT_INC(ips_fragmented); KMOD_IPSTAT_INC(ips_fragmented);
return (error); return (error);
out: dropit:
if (m != NULL) for (mcur = *mp; mcur; mcur = m) { /* droping the full packet chain */
kpUnsubmitted
Done
Inline Actions

You don't need this check, the for loop has the same effect.

kp: You don't need this check, the for loop has the same effect.
m_freem(m); m = mcur->m_nextpkt;
m_freem(mcur);
}
return (error); return (error);
} }
static void static void
bridge_linkstate(struct ifnet *ifp) bridge_linkstate(struct ifnet *ifp)
{ {
struct bridge_softc *sc = ifp->if_bridge; struct bridge_softc *sc = ifp->if_bridge;
struct bridge_iflist *bif; struct bridge_iflist *bif;
Show All 37 Lines
View Options

sys/net80211/ieee80211_ioctl.c

Show First 20 Lines • Show All 327 Lines • ▼ Show 20 Lines
static int bridge_ioctl_sproto(struct bridge_softc *, void *); static int bridge_ioctl_sproto(struct bridge_softc *, void *);
static int bridge_ioctl_stxhc(struct bridge_softc *, void *); static int bridge_ioctl_stxhc(struct bridge_softc *, void *);
static int bridge_pfil(struct mbuf **, struct ifnet *, struct ifnet *, static int bridge_pfil(struct mbuf **, struct ifnet *, struct ifnet *,
int); int);
static int bridge_ip_checkbasic(struct mbuf **mp); static int bridge_ip_checkbasic(struct mbuf **mp);
#ifdef INET6 #ifdef INET6
static int bridge_ip6_checkbasic(struct mbuf **mp); static int bridge_ip6_checkbasic(struct mbuf **mp);
#endif /* INET6 */ #endif /* INET6 */
static int bridge_fragment(struct ifnet *, struct mbuf *, static int bridge_fragment(struct ifnet *, struct mbuf **mp,
struct ether_header *, int, struct llc *); struct ether_header *, int, struct llc *);
static void bridge_linkstate(struct ifnet *ifp); static void bridge_linkstate(struct ifnet *ifp);
static void bridge_linkcheck(struct bridge_softc *sc); static void bridge_linkcheck(struct bridge_softc *sc);
extern void (*bridge_linkstate_p)(struct ifnet *ifp); extern void (*bridge_linkstate_p)(struct ifnet *ifp);
/* The default bridge vlan is 1 (IEEE 802.1Q-2003 Table 9-2) */ /* The default bridge vlan is 1 (IEEE 802.1Q-2003 Table 9-2) */
#define VLANTAGOF(_m) \ #define VLANTAGOF(_m) \
▲ Show 20 Lines • Show All 1,567 Lines • ▼ Show 20 Lines if ((m->m_flags & M_VLANTAG) &&
if_printf(dst_ifp, if_printf(dst_ifp,
"unable to prepend VLAN header\n"); "unable to prepend VLAN header\n");
if_inc_counter(dst_ifp, IFCOUNTER_OERRORS, 1); if_inc_counter(dst_ifp, IFCOUNTER_OERRORS, 1);
continue; continue;
} }
m->m_flags &= ~M_VLANTAG; m->m_flags &= ~M_VLANTAG;
} }
M_ASSERTPKTHDR(m); /* We shouldn't transmit mbuf without pkthdr */
if ((err = dst_ifp->if_transmit(dst_ifp, m))) { if ((err = dst_ifp->if_transmit(dst_ifp, m))) {
m_freem(m0); m_freem(m0);
if_inc_counter(sc->sc_ifp, IFCOUNTER_OERRORS, 1); if_inc_counter(sc->sc_ifp, IFCOUNTER_OERRORS, 1);
break; break;
} }
if_inc_counter(sc->sc_ifp, IFCOUNTER_OPACKETS, 1); if_inc_counter(sc->sc_ifp, IFCOUNTER_OPACKETS, 1);
if_inc_counter(sc->sc_ifp, IFCOUNTER_OBYTES, len); if_inc_counter(sc->sc_ifp, IFCOUNTER_OBYTES, len);
▲ Show 20 Lines • Show All 1,301 Lines • ▼ Show 20 Lines case ETHERTYPE_IP:
if (V_pfil_bridge && dir == PFIL_IN && bifp != NULL) if (V_pfil_bridge && dir == PFIL_IN && bifp != NULL)
error = pfil_run_hooks(&V_inet_pfil_hook, mp, bifp, error = pfil_run_hooks(&V_inet_pfil_hook, mp, bifp,
dir, NULL); dir, NULL);
if (*mp == NULL || error != 0) /* filter may consume */ if (*mp == NULL || error != 0) /* filter may consume */
break; break;
/* check if we need to fragment the packet */ /* check if we need to fragment the packet */
/* bridge_fragment generates a mbuf chain of packets */
/* that already include eth headers */
if (V_pfil_member && ifp != NULL && dir == PFIL_OUT) { if (V_pfil_member && ifp != NULL && dir == PFIL_OUT) {
i = (*mp)->m_pkthdr.len; i = (*mp)->m_pkthdr.len;
if (i > ifp->if_mtu) { if (i > ifp->if_mtu) {
error = bridge_fragment(ifp, *mp, &eh2, snap, error = bridge_fragment(ifp, mp, &eh2, snap,
&llc1); &llc1);
return (error); return (error);
} }
} }
/* Recalculate the ip checksum. */ /* Recalculate the ip checksum. */
ip = mtod(*mp, struct ip *); ip = mtod(*mp, struct ip *);
hlen = ip->ip_hl << 2; hlen = ip->ip_hl << 2;
▲ Show 20 Lines • Show All 222 Lines • ▼ Show 20 Linesbad:
*mp = m; *mp = m;
return (-1); return (-1);
} }
#endif /* INET6 */ #endif /* INET6 */
/* /*
* bridge_fragment: * bridge_fragment:
* *
* Return a fragmented mbuf chain. * Fragment mbuf chain in multiple packets and prepend ethernet header.
*/ */
static int static int
bridge_fragment(struct ifnet *ifp, struct mbuf *m, struct ether_header *eh, bridge_fragment(struct ifnet *ifp, struct mbuf **mp, struct ether_header *eh,
int snap, struct llc *llc) int snap, struct llc *llc)
{ {
struct mbuf *m0; struct mbuf *m = *mp, *m0 = NULL, *mprev = NULL, *mcur = NULL;
struct ip *ip; struct ip *ip;
int error = -1; int error = -1;
if (m->m_len < sizeof(struct ip) && if (m->m_len < sizeof(struct ip) &&
(m = m_pullup(m, sizeof(struct ip))) == NULL) (m = m_pullup(m, sizeof(struct ip))) == NULL)
goto out; goto dropit;
ip = mtod(m, struct ip *); ip = mtod(m, struct ip *);
m->m_pkthdr.csum_flags |= CSUM_IP; m->m_pkthdr.csum_flags |= CSUM_IP;
error = ip_fragment(ip, &m, ifp->if_mtu, ifp->if_hwassist); error = ip_fragment(ip, &m, ifp->if_mtu, ifp->if_hwassist);
if (error) if (error)
goto out; goto dropit;
/* walk the chain and re-add the Ethernet header */ /*
for (m0 = m; m0; m0 = m0->m_nextpkt) { * walk the chain and re-add the Ethernet header for
if (error == 0) { * each mbuf packets
*/
for (mcur = m; mcur; mcur = mcur->m_nextpkt) {
m0 = mcur;
if (snap) { if (snap) {
M_PREPEND(m0, sizeof(struct llc), M_NOWAIT); M_PREPEND(mcur, sizeof(struct llc), M_NOWAIT);
if (m0 == NULL) {
if (mcur == NULL) {
error = ENOBUFS; error = ENOBUFS;
continue; goto dropit;
} }
bcopy(llc, mtod(m0, caddr_t), bcopy(llc, mtod(mcur, caddr_t),sizeof(struct llc));
sizeof(struct llc));
} }
kpUnsubmitted
Done
Inline Actions

Doesn't this need to go before the if (snap) block? If we end up prepending an mbuf for the snap header but not for the ethernet header we still need to do the m_next/m_nextpkt pointer reshuffle.

kp: Doesn't this need to go before the if (snap) block? If we end up prepending an mbuf for the…
M_PREPEND(m0, ETHER_HDR_LEN, M_NOWAIT);
if (m0 == NULL) { M_PREPEND(mcur, ETHER_HDR_LEN, M_NOWAIT);
if (mcur == NULL) {
error = ENOBUFS; error = ENOBUFS;
continue; goto dropit;
} }
bcopy(eh, mtod(m0, caddr_t), ETHER_HDR_LEN); bcopy(eh, mtod(mcur, caddr_t), ETHER_HDR_LEN);
} else /*
m_freem(m); * if M_PREPEND had to create a new mbuf, we need to:
* 1. update previous mbuf packet to use this new one
* 2. moving m_nextpkt from the old mbuf to the new one
* But we can have two new mbufs prepending: LLC and Ethernet HDR!
*/
kpUnsubmitted
Done
Inline Actions

Won't this break in a (worst-case) scenario where both the snap and ethernet M_PREPEND()s end up adding a new mbuf?

kp: Won't this break in a (worst-case) scenario where both the snap and ethernet M_PREPEND()s end…
if ( m0 != mcur ) {
if ( mcur->m_next->m_next && mcur->m_next->m_next->m_nextpkt ) {
/* If two new mbufs insered (LLC and Ethernet header) */
mcur->m_nextpkt = mcur->m_next->m_next->m_nextpkt;
mcur->m_next->m_next->m_nextpkt = NULL;
} else {
/* If only one mbuf insered (Ethernet header) */
mcur->m_nextpkt = mcur->m_next->m_nextpkt;
mcur->m_next->m_nextpkt = NULL;
} }
if (mprev != NULL) { /* It's not the first packet in the chain*/
mprev->m_nextpkt = mcur;
} else { /* the first mbuf in the original chain needs to be updated */
*mp = mcur;
}
}
mprev = mcur;
}
if (error == 0)
KMOD_IPSTAT_INC(ips_fragmented); KMOD_IPSTAT_INC(ips_fragmented);
return (error); return (error);
out: dropit:
if (m != NULL) for (mcur = *mp; mcur; mcur = m) { /* droping the full packet chain */
kpUnsubmitted
Done
Inline Actions

You don't need this check, the for loop has the same effect.

kp: You don't need this check, the for loop has the same effect.
m_freem(m); m = mcur->m_nextpkt;
m_freem(mcur);
}
return (error); return (error);
} }
static void static void
bridge_linkstate(struct ifnet *ifp) bridge_linkstate(struct ifnet *ifp)
{ {
struct bridge_softc *sc = ifp->if_bridge; struct bridge_softc *sc = ifp->if_bridge;
struct bridge_iflist *bif; struct bridge_iflist *bif;
Show All 37 Lines