[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openbsd-tech
Subject:    Re: pipex fails to send ack-only GRE message from npppd
From:       Claudio Jeker <cjeker () diehard ! n-r-g ! com>
Date:       2010-09-29 7:44:37
Message-ID: 20100929074437.GC18123 () diehard ! n-r-g ! com
[Download RAW message or body]

On Mon, Sep 27, 2010 at 11:31:30AM +0900, Hiroki Suenaga wrote:
> Can we assume m0 have at least sizeof(struct pipex_gre_header) bytes?
> If m0 is too short, m_coypdata() will cause panic.
> 
> I'm not sure, but to check m_pktlen is safe.

It is save because pipex_pptp_userland_lookup_session() is called
previously and there is a check to ensure that sizeof(struct
pipex_gre_header) bytes are available.

> 
> 2010/9/27 YASUOKA Masahiko <yasuoka@openbsd.org>
> 
> >
> > pipex_pptp_userland_output() calls always m_pullup() 16 bytes to the
> > GRE message.  But when npppd send a ack-only GRE message, the message
> > will be only 12 bytes, so the m_pullup() will fail.  call m_pullup()
> > with proper length.
> >
> > ok?
> >
> > --- pipex.c-ORIG        Mon Sep 27 09:32:16 2010
> > +++ pipex.c     Mon Sep 27 09:46:54 2010
> > @@ -1808,22 +1808,30 @@ pipex_pptp_userland_lookup_session(struct mbuf *m0,
> > st
> >  struct mbuf *
> >  pipex_pptp_userland_output(struct mbuf *m0, struct pipex_session *session)
> >  {
> > -       struct pipex_gre_header *gre;
> > +       int len;
> > +       struct pipex_gre_header *gre, gre0;
> >        uint16_t flags;
> >        u_char *cp, *cp0;
> >        uint32_t val32;
> >
> > +       len = sizeof(struct pipex_gre_header);
> > +       m_copydata(m0, 0, len, (caddr_t)&gre0);
> > +       gre = &gre0;
> > +       flags = ntohs(gre->flags);
> > +       if ((flags & PIPEX_GRE_SFLAG) != 0)
> > +               len += 4;
> > +       if ((flags & PIPEX_GRE_AFLAG) != 0)
> > +               len += 4;
> > +
> >        /* check length */
> > -       PIPEX_PULLUP(m0, sizeof(struct pipex_gre_header) + 8);
> > +       PIPEX_PULLUP(m0, len);
> >        if (m0 == NULL) {
> > -               PIPEX_DBG((session, LOG_DEBUG,
> > -                   "gre header is too short."));
> > +               PIPEX_DBG((session, LOG_DEBUG, "gre header is too
> > short."));
> >                return (NULL);
> >        }
> >
> >        gre = mtod(m0, struct pipex_gre_header *);
> >        cp = PIPEX_SEEK_NEXTHDR(gre, sizeof(struct pipex_gre_header), u_char
> > *);
> > -       flags = ntohs(gre->flags);
> >
> >        /*
> >         * overwrite sequence numbers to adjust a gap between pipex and
> >
> 
> 
> 
> -- 
> -----
> SUENAGA Hiroki
> hsuenaga@openbsd.org
> 

-- 
:wq Claudio

[prev in list] [next in list] [prev in thread] [next in thread]