> On 9 Oct 2016, at 09:35, Mike Belopuhov <[email protected]> wrote:
>
> After looking into alignment and other issues with encapsulated
> Ethernet frames, turns out that not all our encapsulation drivers
> do a good job of performing header length checks and can
> potentially pass truncated packets into the ether_input. I was a
> bit on a fence regarding whether or not ether_input should check
> for that or even check if there's anything following the ethernet
> header. Right now higher level protocols like MPLS and IP will
> check if the appropriate header is in the first mbuf before doing
> mtod() access and will pullup otherwise. m_pullup will handle
> the case where there's no header by freeing the chain and
> failing.
>
> Keeping this in mind, I'd still like to strenthen two checks in
> the input path.
>
> OK?
yeah.
long term i think it makes sense for the ethernet stack to do its own checks
rather than relying on everything above it to do them for it. i cant think of
another layer that gets to be lazy like this.
dlg
>
> diff --git sys/net/if_ethersubr.c sys/net/if_ethersubr.c
> index b800d6d..cd166eb 100644
> --- sys/net/if_ethersubr.c
> +++ sys/net/if_ethersubr.c
> @@ -316,10 +316,14 @@ ether_input(struct ifnet *ifp, struct mbuf *m, void
> *cookie)
> struct arpcom *ac;
> #if NPPPOE > 0
> struct ether_header *eh_tmp;
> #endif
>
> + /* Drop short frames */
> + if (m->m_len < ETHER_HDR_LEN)
> + goto dropanyway;
> +
> ac = (struct arpcom *)ifp;
> eh = mtod(m, struct ether_header *);
> m_adj(m, ETHER_HDR_LEN);
>
> if (ETHER_IS_MULTICAST(eh->ether_dhost)) {
> @@ -432,11 +436,12 @@ decapsulate:
> case ETHERTYPE_MPLS_MCAST:
> mpls_input(m);
> return (1);
> #endif
> default:
> - if (llcfound || etype > ETHERMTU)
> + if (llcfound || etype > ETHERMTU ||
> + (m->m_len < sizeof(struct llc)))
> goto dropanyway;
> llcfound = 1;
> l = mtod(m, struct llc *);
> switch (l->llc_dsap) {
> case LLC_SNAP_LSAP:
>
