On Tue, Jun 14, 2016 at 06:26:00PM +0200, Martin Pieuchot wrote:
> On 14/06/16(Tue) 15:18, Florian Obser wrote:
> > Hi,
> > I'm seeing this panic on my v6 gateway running in a vm (don't ask):
> > It has a v6 tunnel via HE on gif0.
> >
> > I hope I copied all relevant information, if not, my appologies, I'm
> > in a hurry currently, please just ask for more.
> >
> > I will probably investigate more when I'm home :)
> >
> > panic: trap type 6, code=0, pc=ffffffff812fe70f
> > Starting stack trace...
> > panic() at panic+0x10b
> > trap() at trap+0x7b8
> > --- trap (number 6) ---
> > ip6_output_ipsec_lookup() at ip6_output_ipsec_lookup+0x6f
> > ip6_output() at ip6_output+0x21c
> > esp_output_cb() at esp_output_cb+0x135
> > taskq_thread() at taskq_thread+0x6c
> > end trace frame: 0x0, count: 251
> > End of stack trace.
> > syncing disks... done
>
> This seems to be an invalid `'tdbi'' dereference in ip6_output_ipsec_lookup():
>
> 2890: tdbi = (struct tdb_ident *)(mtag + 1);
> 2891: HERE -> if (tdbi->spi == tdb->tdb_spi &&
> 2892: tdbi->proto == tdb->tdb_sproto &&
> ...
>
> Markus, Mike any idea how this could happen?
I tracked it down to ref 1.89 of ip6_forward.c / ref 1.205 ip6_output.c:
"factor out ipsec into ip6_output_ipsec_{lookup,send}(); ok mpi@, naddy@"
The problem is that we are not exiting the "loop detection" for loop
when tdb is set to NULL. We enter again and dereference tdb -> boom.
The following diff makes ip6_output_ipsec_lookup() similar to
ip_output_ipsec_lookup().
It's easier to see what the diff is doing by applying and doing diff -b.
OK?
p.s. I also note that the v4 and v6 version are really similiar, we
can probably merge them. Wonder if it's worth it or if it's best to
keep v4 and v6 seperate...
diff --git ip6_output.c ip6_output.c
index 64eea86..3adaa7d 100644
--- ip6_output.c
+++ ip6_output.c
@@ -2882,21 +2882,21 @@ ip6_output_ipsec_lookup(struct mbuf *m, int *error,
struct inpcb *inp)
tdb = ipsp_spd_lookup(m, AF_INET6, sizeof(struct ip6_hdr),
error, IPSP_DIRECTION_OUT, NULL, inp, 0);
- if (tdb != NULL) {
- /* Loop detection */
- for (mtag = m_tag_first(m); mtag != NULL;
- mtag = m_tag_next(m, mtag)) {
- if (mtag->m_tag_id != PACKET_TAG_IPSEC_OUT_DONE)
- continue;
- tdbi = (struct tdb_ident *)(mtag + 1);
- if (tdbi->spi == tdb->tdb_spi &&
- tdbi->proto == tdb->tdb_sproto &&
- tdbi->rdomain == tdb->tdb_rdomain &&
- !bcmp(&tdbi->dst, &tdb->tdb_dst,
- sizeof(union sockaddr_union)))
- tdb = NULL;
+ if (tdb == NULL)
+ return NULL;
+ /* Loop detection */
+ for (mtag = m_tag_first(m); mtag != NULL; mtag = m_tag_next(m, mtag)) {
+ if (mtag->m_tag_id != PACKET_TAG_IPSEC_OUT_DONE)
+ continue;
+ tdbi = (struct tdb_ident *)(mtag + 1);
+ if (tdbi->spi == tdb->tdb_spi &&
+ tdbi->proto == tdb->tdb_sproto &&
+ tdbi->rdomain == tdb->tdb_rdomain &&
+ !memcmp(&tdbi->dst, &tdb->tdb_dst,
+ sizeof(union sockaddr_union))) {
+ /* no IPsec needed */
+ return NULL;
}
- /* We need to do IPsec */
}
return tdb;
}
--
I'm not entirely sure you are real.
