- Do not raise IFF_DRV_OACTIVE flag in vlan_start, because this
can lead to stalled interface
- Explain this fact in a comment.
Reviewed by: rwatson, thompsa, yar
- most of the kernel code will not care about the actual encoding of
scope zone IDs and won't touch "s6_addr16[1]" directly.
- similarly, most of the kernel code will not care about link-local
scoped addresses as a special case.
- scope boundary check will be stricter. For example, the current
*BSD code allows a packet with src=::1 and dst=(some global IPv6
address) to be sent outside of the node, if the application do:
s = socket(AF_INET6);
bind(s, "::1");
sendto(s, some_global_IPv6_addr);
This is clearly wrong, since ::1 is only meaningful within a single
node, but the current implementation of the *BSD kernel cannot
reject this attempt.
sys/net/if_gif.c: 1.53
sys/net/if_spppsubr.c: 1.120
sys/netinet/icmp6.h: 1.19
sys/netinet/ip_carp.c: 1.28,1.29
sys/netinet/ip_fw2.c: 1.107
sys/netinet/tcp_subr.c: 1.230,1.231,1.235
sys/netinet/tcp_usrreq.c: 1.125
sys/netinet6/ah_core.c: 1.26
sys/netinet6/icmp6.c: 1.63,1.64
sys/netinet6/in6.c: 1.52
sys/netinet6/in6.h: 1.38
sys/netinet6/in6_cksum.c: 1.11
sys/netinet6/in6_ifattach.c: 1.27
sys/netinet6/in6_pcb.c: 1.63
sys/netinet6/in6_proto.c: 1.33
sys/netinet6/in6_src.c: 1.31,1.32
sys/netinet6/in6_var.h: 1.22
sys/netinet6/ip6_forward.c: 1.29
sys/netinet6/ip6_input.c: 1.83
sys/netinet6/ip6_mroute.c: 1.30
sys/netinet6/ip6_output.c: 1.95
sys/netinet6/ip6_var.h: 1.33
sys/netinet6/ipsec.c: 1.43
sys/netinet6/mld6.c: 1.21
sys/netinet6/nd6.c: 1.50
sys/netinet6/nd6_nbr.c: 1.30
sys/netinet6/nd6_rtr.c: 1.27
sys/netinet6/raw_ip6.c: 1.54
sys/netinet6/route6.c: 1.12
sys/netinet6/scope6.c: 1.13,1.14,1.15
sys/netinet6/scope6_var.h: 1.5
sys/netinet6/udp6_output.c: 1.23
sys/netinet6/udp6_usrreq.c: 1.55
sys/netkey/key.c: 1.72,1.73
"firewire" to "if_firewire." According to the PR originator and Cai's
test, this bug prevents a RELENG_6 GENERIC kernel from loading if_fwip.ko
and will panic the kernel if users compile "device fwip" into their
GENERIC kernel.
Approved by: re (scottl)
> If we have been called from ether_ifdetach() then do not try and clear the
> promisc flag from the member interface, this is a no-op anyway since the
> interface is disappearing. The driver may have already released
> its resources such as miibus and this is likely to panic the kernel.
Approved by: re (scottl)
> Use bridge_ifdetach() to notify the bridge that a member has been detached.
> The bridge can then remove it from its interface list and not try to send
> out via a dead pointer.
Approved by: re (scottl)
> Do not packet filter in the bridge_start() routine, locally generated packets
> are already filtered by the higher layers.
Approved by: re (kensmith)
Rename net.isr.enable to net.isr.direct.
No compatibility code is provided, as this will be the production name
as of 6.0. Previously this has been an experimental and unsupported
feature.
Requested by: scottl
Approved by: re (kensmith)
Take a first cut at cleaning up ifnet removal and multicast socket
panics, which occur when stale ifnet pointers are left in struct
moptions hung off of inpcbs:
- Add in_ifdetach(), which matches in6_ifdetach(), and allows the
protocol to perform early tear-down on the interface early in
if_detach().
- Annotate that if_detach() needs careful consideration.
- Remove calls to in_pcbpurgeif0() in the handling of SIOCDIFADDR --
this is not the place to detect interface removal! This also
removes what is basically a nasty (and now unnecessary) hack.
- Invoke in_pcbpurgeif0() from in_ifdetach(), in both raw and UDP
IPv4 sockets.
It is now possible to run the msocket_ifnet_remove regression test
using HEAD without panicking.
Reported by: Gavin Atkinson <gavin dot atkinson at ury dot york dot ac dot uk>
Approved by: re (scottl)
Several fixes to rt_setgate(), that fix problems with route changing:
- Rearrange code so that in a case of failure the affected
route is not changed. Otherwise, a bogus rtentry will be
left and later rt_check() can recurse on its lock. [1]
- Remove comment about protocol cloning.
- Fix two places where rtentry mutex was recursed on, because
accessed via two different pointers, that were actually pointing
to the same rtentry in some cases. [1]
- Return EADDRINUSE instead of bogus EDQUOT, in case when gateway
uses the same route. [2]
Reported & tested by: ps, Andrej Zverev <az inec.ru> [1]
PR: kern/64090 [2]
Approved by: re (scottl)
> Fix an alignment panic my preserving the 2byte padding (ETHER_ALIGN) on our
> copied mbuf, which keeps the IP header 32-bit aligned. This copied mbuf is
> reinjected back into ether_input and off to the IP routines.
>
> Reported and tested by: Peter van Dijk
Approved by: re (scottl), mlaier (mentor)
Test the new M_VLANTAG packet flag before calling
m_tag_locate(). This adds little overhead of a simple
bitwise operation in case hardware VLAN acceleration
is on, yet saves the more expensive function call if
the acceleration is off.
Reviewed by: ru, glebius
Approved by: re (kensmith)
In netkqfilter(), return EINVAL instead of 1 (EPERM) when a filter type
is requested on a network interface file descriptor that is
non-applicable.
Approved by: re (kensmith)
nd6.c, r1.55; nd6_nbr.c, r1.33
> Add support for multicast to the bridge and allow inet6 addresses to be
> assigned to the interface.
>
> IPv6 auto-configuration is disabled. An IPv6 link-local address has a
> link-local scope within one link, the spec is unclear for the bridge case and
> it may cause scope violation.
>
> An address can be assigned in the usual way;
> ifconfig bridge0 inet6 xxxx:...
>
> Tested by: bmah
> Reviewed by: ume (netinet6)
> Approved by: mlaier (mentor)
Approved by: re (kensmith), mlaier (mentor)
> Fix a panic in softclock() if the interface is destroyed with a bpf consumer
> attached.
>
> This is caused by bpf_detachd clearing IFF_PROMISC on the interface which does
> a SIOCSIFFLAGS ioctl. The problem here is that while the interface has been
> stopped, IFF_UP has not been cleared so IFF_UP != IFF_DRV_RUNNING, this causes
> the ioctl function to init() the interface which resets the callouts.
>
> The destroy then completes and frees the softc but softclock will panic on a
> dead callout pointer.
>
> Ensure ifp->if_flags matches reality by clearing IFF_UP when we destroy.
Approved by: re (kensmith), mlaier (mentor)
> Mark the callouts as MPSAFE as if_bridge has been giant-free since day 1.
>
> Use the SMP friendly callout_init_mtx() while we are here.
>
Approved by: re (kensmith), mlaier (mentor)
Don't loop back packets that have been routed by pf. This fixes an
endless loop where the same packet is sent over and over again.
Obtained from: OpenBSD
Reported by: Sergey Lapin
Tested by: Sergey Lapin
Approved by: re (scottl)
De-spl parts of the routing socket code now generally protected
through locking; leave some spl references around code where there
are open questions about global variable references. Also, add
an XXX regarding locking in sysctl.
Approved by: re (scottl)
> The mtu check in bridge_enqueue is bogus as the maximum Ethernet frame is
> actually 1514, so comparing the mbuf length which includes the Ethernet
> header
> to the interface MTU is wrong.
>
> The check was a little over the top so just remove it.
Approved by: re (scottl), mlaier (mentor)
For each interface flag, indicate whether or not it is owned by the
device driver, owned by the network stack, or initialized by the device
driver before attach and read-only from then on.
Not all device drivers and network stack components currently follow
these rules, especially with respect to IFF_UP, and a few exceptions
with IFF_ALLMULTI.
Approved by: re (scottl)
to RELENG_6:
Rename IFF_RUNNING to IFF_DRV_RUNNING, IFF_OACTIVE to IFF_DRV_OACTIVE,
and move both flags from ifnet.if_flags to ifnet.if_drv_flags, making
and documenting the locking of these flags the responsibility of the
device driver, not the network stack. The flags for these two fields
will be mutually exclusive so that they can be exposed to user space as
though they were stored in the same variable.
Provide #defines to provide the old names #ifndef _KERNEL, so that user
applications (such as ifconfig) can use the old flag names. Using the
old names in a device driver will result in a compile error in order to
help device driver writers adopt the new model.
When exposing the interface flags to user space, via interface ioctls
or routing sockets, or the two fields together. Since the driver flags
cannot currently be set for user space, no new logic is currently
required to handle this case.
Add some assertions that general purpose network stack routines, such
as if_setflags(), are not improperly used on driver-owned flags.
With this change, a large number of very minor network stack races are
closed, subject to correct device driver locking. Most were likely
never triggered.
Driver sweep to follow; many thanks to pjd and bz for the line-by-line
review they gave this patch.
Reviewed by: pjd, bz
Approved by: re (scottl)
Lock down netnatm and mark as MPSAFE:
- Introduce a subsystem mutex, natm_mtx, manipulated with accessor macros
NATM_LOCK_INIT(), NATM_LOCK(), NATM_UNLOCK(), NATM_LOCK_ASSERT(). It
protects the consistency of pcb-related data structures. Finer grained
locking is possible, but should be done in the context of specific
measurements (as very little work is done in netnatm -- most is in the
ATM device driver or socket layer, so there's probably not much
contention).
- Remove GIANT_REQUIRED, mark as NETISR_MPSAFE, remove
NET_NEEDS_GIANT("netnatm").
- Conditionally acquire Giant when entering network interfaces for
ifp->if_ioctl() using IFF_LOCKGIANT(ifp)/IFF_UNLOCKGIANT(ifp) in order
to coexist with non-MPSAFE atm ifnet drivers..
- De-spl.
Reviewed by: harti, bms (various versions)
Approved by: re (hrs)
if_iso88025subr.c:1.68 from HEAD to RELENG_6:
When allocating link layer ifnet address list entries in
ifp->if_resolvemulti(), do so with M_NOWAIT rather than M_WAITOK, so
that a mutex can be held over the call. In the FDDI code, add a
missing M_ZERO. Consumers are already aware that if_resolvemulti()
can fail.
Approved by: re (scottl)
- Rename ifmaof_ifpforaddr() to if_findmulti(); assert if_addr_mtx.
Staticize.
Problem reported by: Ed Maste <emaste at phaedrus dot sandvine dot ca>
Approved by: re (scottl)
correct nits in the addition of if_addr_mtx:
if.c:1.240:
Initialize the if_addr mutex in if_alloc() rather than waiting until
if_attach(). This allows ethernet drivers to use it in their routines
to program their MAC filters before ether_ifattach() is called (de(4) is
one such driver). Also, the if_addr mutex is destroyed in if_free()
rather than if_detach(), so there was another potential bug in that a
driver that failed during attach and called if_free() without having
called ether_ifattach() would have tried to destroy an uninitialized
mutex.
Reported by: Holm Tiffe holm at freibergnet dot de
Discussed with: rwatson
if.c:1.241:
destroy lock _before_ free'ing the structure it resides in
if.c:1.242:
- Move IF_ADDR_LOCK_DESTROY(ifp) from if_free to if_free_type.
- Add a note that additions should be made to if_free_type and not
if_free to help avoid this in the future.
This apparently fixes a use after free in if_bridge and may fix bugs
in other direct if_free_type consumers.
Reported by: thompsa
Approved by: re (hrs)
Protect link layer network interface multicast address list manipulation
using ifp->if_addr_mtx:
- Initialize if_addr_mtx when ifnet is initialized.
- Destroy if_addr_mtx when ifnet is torn down.
- Rename ifmaof_ifpforaddr() to if_findmulti(); assert if_addr_mtx.
Staticize.
- Extract ifmultiaddr allocation and initialization into if_allocmulti();
accept a 'mflags' argument to indicate whether or not sleeping is
permitted. This centralizes error handling and address duplication.
- Extract ifmultiaddr tear-down and deallocation in if_freemulti().
- Re-structure if_addmulti() to hold if_addr_mtx around manipulation of
the ifnet multicast address list and reference count manipulation.
Make use of non-sleeping allocations. Annotate the fact that we only
generate routing socket events for explicit address addition, not
implicit link layer address addition.
- Re-structure if_delmulti() to hold if_addr_mtx around manipulation of
the ifnet multicast address list and reference count manipulation.
Annotate the lack of a routing socket event for implicit link layer
address removal.
- De-spl all and sundry.
Problem reported by: Ed Maste <emaste at phaedrus dot sandvine dot ca>
Approved by: re (hrs)
- Introduce a helper function if_setflag() containing the code common
to ifpromisc() and if_allmulti() instead of duplicating the code poorly,
with different bugs.
- Call ifp->if_ioctl() in a consistent way: always use more compatible C
syntax and check whether ifp->if_ioctl is not NULL prior to the call.
Discussed with: yar
Approved by: re (scottl)
Allocate one of the spare ifnet integer fields to hold if_drv_flags,
which in the future will hold IFF_OACTIVE and IFF_RUNNING, and have
its access synchronized by the device driver rather than the
protocol stack. This will avoid potential races in the management
of flags in if_flags.
Discussed with: various (scottl, jhb, ...)
Approved by: re (kensmith)
calls before an if_attach.
Partial MFC of 1.243. The change it was a response to has not been
MFCd, but the comment is relevent without it.
Approved by: re (kensmith)
Axe ppp_for_tty(). Use tty->t_lsc pointer to store sc. This
also eliminates recursive use of ppp_softc_list_mtx.
PR: kern/84686
Reviewed by: phk
Approved by: re (kensmith)
date: 2005/08/18 22:30:52; author: csjp; state: Exp; lines: +4 -2
Add missing braces around bpf_filter which were missed when I
merged the bpfstat code.
Approved by: re (kensmith)
> Ensure that we are holding the lock when initialising the bridge interface. We
> could initialise while unlocked if the bridge is not up when setting the inet
> address, ether_ioctl() would call bridge_init.
>
> Change it so bridge_init is always called unlocked and then locks before
> calling bstp_initialization().
Approved by: re (kensmith), mlaier (mentor)
In multicast routines:
Compare pointers with NULL rather than treating them as booleans.
Compare pointers with NULL rather than 0 to make it more clear
they are pointers.
Assign pointers value of NULL rather than 0 to make it more clear
they are pointers.
Approved by: re (kensmith)
Rename equal() macro to sa_equal(), which matches the definitions
of sa_equal() in other files, and makes it more clear what equal()
is comparing.
Approved by: re (kensmith)
