[RTEMS Project] #1943: NULL pointer access in if_ppp.c

[RTEMS trac]

#1943: NULL pointer access in if_ppp.c
-----------------------------+------------------------------
 Reporter:  sebastian.huber  |       Owner:  sebastian.huber
     Type:  defect           |      Status:  assigned
 Priority:  normal           |   Milestone:  4.9.5
Component:  networking       |     Version:  4.11
 Severity:  normal           |  Resolution:
 Keywords:                   |
-----------------------------+------------------------------
Changes (by gedare):

 * owner:  norume => sebastian.huber
 * status:  new => assigned
 * version:  HEAD => 4.11
 * milestone:  4.11 => 4.9.5


Old description:

> In if_ppp.c (ppp_rxdaemon) we have:
>
> [...]
>       /* allocate a new mbuf to replace one */
>       if ( mp == NULL ) {
>         pppallocmbuf(sc, &mp);
>       }
>
>       /* place mbuf on freeq */
>       rtems_interrupt_disable(level);
>       IF_ENQUEUE(&sc->sc_freeq, mp);
>       rtems_interrupt_enable(level);
>       mp = (struct mbuf *)0;
> [...]
>
> In ppp_tty.c we have:
>
> [...]
> void
> pppallocmbuf(struct ppp_softc *sc, struct mbuf **mp)
> {
>   int            ilen;
>   struct mbuf   *m;
>
>   /* loop over length value */
>   ilen = sc->sc_mru + PPP_HDRLEN + PPP_FCSLEN;
>   while ( ilen > 0 ) {
>     /* see if this is end of the chain */
>     m = *mp;
>     if ( m == NULL ) {
>       /* get mbuf header */
>       MGETHDR(m, M_DONTWAIT, MT_DATA);
>       if ( m == NULL ) {
>         /* error - set condition to break out */
>         printf("pppallocmbuf: MGETHDR failed\n");
>         break;
>       }
>       MCLGET(m, M_DONTWAIT);
>       m->m_next = NULL;
>       *mp = m;
>     }
>
>     /* update loop variables */
>     mp    = &m->m_next;
>     ilen -= M_DATASIZE(m);
>   }
> }
> [...]
>
> In case no mbufs are available, the pppallocmbuf() prints an error and
> leaves the *mp value untouched.  This leads to a NULL pointer access in
> if_ppp.c.
>
> I propose to change the mbuf and cluster allocation to use M_WAIT instead
> of M_DONTWAIT.

New description:

 In if_ppp.c (ppp_rxdaemon) we have:

 [...]
       /* allocate a new mbuf to replace one */
       if ( mp == NULL ) {
         pppallocmbuf(sc, &mp);
       }

       /* place mbuf on freeq */
       rtems_interrupt_disable(level);
       IF_ENQUEUE(&sc->sc_freeq, mp);
       rtems_interrupt_enable(level);
       mp = (struct mbuf *)0;
 [...]

 In ppp_tty.c we have:

 [...]
 void
 pppallocmbuf(struct ppp_softc *sc, struct mbuf **mp)
 {
   int            ilen;
   struct mbuf   *m;

   /* loop over length value */
   ilen = sc->sc_mru + PPP_HDRLEN + PPP_FCSLEN;
   while ( ilen > 0 ) {
     /* see if this is end of the chain */
     m = *mp;
     if ( m == NULL ) {
       /* get mbuf header */
       MGETHDR(m, M_DONTWAIT, MT_DATA);
       if ( m == NULL ) {
         /* error - set condition to break out */
         printf("pppallocmbuf: MGETHDR failed\n");
         break;
       }
       MCLGET(m, M_DONTWAIT);
       m->m_next = NULL;
       *mp = m;
     }

     /* update loop variables */
     mp    = &m->m_next;
     ilen -= M_DATASIZE(m);
   }
 }
 [...]

 In case no mbufs are available, the pppallocmbuf() prints an error and
 leaves the *mp value untouched.  This leads to a NULL pointer access in
 if_ppp.c.

 I propose to change the mbuf and cluster allocation to use M_WAIT instead
 of M_DONTWAIT.

--

Comment:

 Sebastian, please back-port your patch to 4.10 and 4.9 branches.

--
Ticket URL: <http://devel.rtems.org/ticket/1943#comment:2>
RTEMS Project <http://www.rtems.org/>
RTEMS Project