Page MenuHomeFreeBSD
Differential D15966

Introduce test-program for process-control group of system calls
ClosedPublic
Actions

Authored by aniketp on Jun 22 2018, 3:45 PM.
Tags
None
Referenced Files
Unknown Object (File)
Sun, Mar 30, 4:24 PM
Unknown Object (File)
Fri, Mar 28, 10:52 PM
Unknown Object (File)
Thu, Mar 20, 12:18 PM
Unknown Object (File)
Sat, Mar 15, 3:33 AM
Unknown Object (File)
Feb 27 2025, 8:12 PM
Unknown Object (File)
Feb 22 2025, 4:29 AM
Unknown Object (File)
Feb 20 2025, 5:28 PM
Unknown Object (File)
Feb 17 2025, 1:57 AM
View All Files
Subscribers
None

Details

Reviewers
asomers
rwatson
gnn
Commits
rS335679: audit(4): add tests for the process-control audit class
Summary

This revision introduces a new (and final) test-program for syscalls categorized within
process-control audit class. A lot of these syscalls don't have tests for either
success or failure case due to various reasons

List of System calls

  1. Both cases
  • rfork(2)
  • chdir(2)
  • fchdir(2)
  • chroot(2)
  • getresuid(2)
  • getresgid(2)
  • setpriority(2)
  • setgroups(2)
  • setpgrp(2)
  • setrlimit(2)
  • setlogin(2)
  • mlock(2)
  • munlock(2)
  • minherit(2)
  • rtprio(2)
  • profil(2)
  • ktrace(2)
  • ptrace(2)
  1. Success case
  • fork(2)
  • umask(2)
  • setuid(2)
  • setgid(2)
  • seteuid(2)
  • setegid(2)
Test Plan

Execute make && make install from test/sys/audit.
Execute kyua test from /usr/tests/sys/audit. All testcases should succeed.

Diff Detail

Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 17545
Build 17366: arc lint + arc unit

Event Timeline

aniketp created this revision.Jun 22 2018, 3:45 PM
Harbormaster completed remote builds in B17543: Diff 44306.Jun 22 2018, 3:45 PM
aniketp edited the summary of this revision. (Show Details)Jun 22 2018, 3:45 PM
aniketp set the repository for this revision to rS FreeBSD src repository - subversion.
Herald added a subscriber: imp. · View Herald TranscriptJun 22 2018, 3:45 PM
aniketp updated this revision to Diff 44309.Jun 22 2018, 4:04 PM

Add test for successful invocation of setlogin(2)

Harbormaster completed remote builds in B17545: Diff 44309.Jun 22 2018, 4:04 PM
aniketp edited the summary of this revision. (Show Details)Jun 22 2018, 4:05 PM
asomers requested changes to this revision.Jun 22 2018, 5:39 PM
asomers added inline comments.
tests/sys/audit/process-control.c
72

You could move check_audit up a line. In fact, it isn't really necessary to call wait at all.

75

FYI, calling exit(3) after a fork is surprisingly risky. That's because it tries to do various cleanup activities. Usually those cleanup activities will work. But if the parent process was multithreaded, then the child will be single-threaded, and its only thread will be a copy of the parent thread that forked. That means that mutexes could end up leaked and other nastiness. That's why it's a common pattern to instead call _exit(2) after forking.

It doesn't matter here, because your program is single-threaded and doesn't have any mutexes or other stuff that could interfere with the child. Just keep it in mind for the future.

158

Kyua is smart enough to chdir back to the test directory for cleanup. So you could simplify this test by simply chdir()ing to "/".

660

It's fine to set priority to a hardcoded value like 0. This process will exit pretty soon anyway.

707

gidset is used uninitialized.

740

Don't forget to add this testcase.

1156

Probably safer to use pid instead of hardcoding 0.

This revision now requires changes to proceed.Jun 22 2018, 5:39 PM
aniketp updated this revision to Diff 44334.Jun 23 2018, 2:26 AM
aniketp marked 5 inline comments as done.

Changes according to the inline comments

Harbormaster completed remote builds in B17557: Diff 44334.Jun 23 2018, 2:26 AM
asomers added inline comments.Jun 23 2018, 6:48 PM
tests/sys/audit/process-control.c
700

Safer to pass the full gidset to setgroups instead of only the first group.

aniketp updated this revision to Diff 44355.Jun 23 2018, 7:26 PM

Update ngroups for setgroups_success as 5 instead of 100

Harbormaster completed remote builds in B17572: Diff 44355.Jun 23 2018, 7:26 PM
aniketp added inline comments.Jun 23 2018, 7:27 PM
tests/sys/audit/process-control.c
700

@asomers setgroups is peculiar in this regard.
If I pass in entire gidset array as the maximum limit it will never let the audit record complete itself.

The piece of code here:

	case AUE_SETGROUPS:
		if (ARG_IS_VALID(kar, ARG_GROUPSET)) {
			for(ctr = 0; ctr < ar->ar_arg_groups.gidset_size; ctr++)
			{
				tok = au_to_arg32(1, "setgroups",
				    ar->ar_arg_groups.gidset[ctr]);
				kau_write(rec, tok);
			}
		}
		break;

Loops till all gidset elements are inserted in the audit record. So for 100 such groups, I get an incomplete audit record and tests fail.
This doesn't happen with 1 or 5 as the limit.

With ngroups = 100;

Looking for 'setgroups.*1844.*ret.*success' in 'header,1868,11,setgroups(2),0,Sat Jun 23 19:15:30 2018, + 663 msecargument,1,0x0,setgroupsargument,1,0x0,setgroupsargument,1,0x5,setgroupsargument,1,0x0,setgroupsargument,1,0xe7913e01,setgroupsargument,1,0x8078a49c,setgroupsargument,1,0x635090,setgroupsargument,1,0x8,setgroupsargument,1,0xffffd480,setgroupsargument,1,0x7fff,setgroupsargument,1,0x77bf89,setgroupsargument,1,0x8,setgroupsargument,1,0x635090,setgroupsargument,1,0x8,setgroupsargument,1,0xe7913ef0,setgroupsargument,1,0x8078a49c,setgroupsargument,1,0xe7913ef0,setgroupsargument,1,0x8078a49c,setgroupsargument,1,0x1,setgroupsargument,1,0x0,setgroupsargument,1,0x635250,setgroupsargument,1,0x8,setgroupsargument,1,0xe7913ef0,setgroupsargument,1,0x8078a49c,setgroupsargument,1,0x1,setgroupsargument,1,0x0,setgroupsargument,1,0x64d010,setgroupsargument,1,0x8,setgroupsargument,1,0x635090,setgroupsargument,1,0x8,setgroupsargument,1,0xffffd3e0,setgroupsargument,1,0x7fff,setgroupsargument,1,0xffffd3a0,setgroupsargument,1,0x7fff,setgroupsargument,1,0x7d6869,setgro����'

process-control:setgroups_success  ->  failed: setgroups.*1844.*ret.*success not found in auditpipe within the time limit

Note: for the sake of the test case, I'm using ngroups as 5 instead of 100.

asomers accepted this revision.Jun 23 2018, 7:57 PM
asomers added inline comments.
tests/sys/audit/process-control.c
700

Looks like you're running up against the 1024 character limit imposed in utils.c:52. So, not a bug in audit(4). A 5-group limit is fine.

BTW, you might see a 16-group limit some places. That comes from NFSv2 and NFSv3, which set that limit in the RPCs.

This revision is now accepted and ready to land.Jun 23 2018, 7:57 PM
aniketp updated this revision to Diff 44357.Jun 23 2018, 8:35 PM

Add test for setpgrp_success

This revision now requires review to proceed.Jun 23 2018, 8:35 PM
Harbormaster completed remote builds in B17573: Diff 44357.Jun 23 2018, 8:35 PM
aniketp edited the summary of this revision. (Show Details)Jun 23 2018, 8:36 PM
aniketp set the repository for this revision to rS FreeBSD src repository - subversion.
aniketp removed a subscriber: imp.
asomers accepted this revision.Jun 25 2018, 12:42 AM
This revision is now accepted and ready to land.Jun 25 2018, 12:42 AM
Closed by commit rS335679: audit(4): add tests for the process-control audit class (authored by asomers). · Explain WhyJun 26 2018, 8:27 PM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathSize
tests/
sys/
audit/
3 lines
1247 lines

Diff 44309

View Options

tests/sys/audit/Makefile

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

tests/sys/audit/process-control.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