Yeah, we override both of 'auth->name' and 'auth->secret’.

Since there is the only difference against your previous diff and
the only place where you touch them I have no objections.

> On 22 Aug 2020, at 18:00, Klemens Nanni <k...@openbsd.org> wrote:
> 
> On Sat, Aug 22, 2020 at 02:32:17PM +0200, Klemens Nanni wrote:
>> Another round, this time obvious sizes which are in immediate scope of
>> the free() call, e.g. right below the malloc() call.
>> 
>> This leaves only a few selected free() calls with size zero in
>> if_spppsubr.c due to the fact that there is currently no variable to
>> keep track of username and password string lengths.
>> 
>> Feedback? OK?
> Sorry, here's the correct version of the diff omitting sizes for the
> very string buffers mentioned above.
> 
> 
> Index: if_spppsubr.c
> ===================================================================
> RCS file: /cvs/src/sys/net/if_spppsubr.c,v
> retrieving revision 1.185
> diff -u -p -r1.185 if_spppsubr.c
> --- if_spppsubr.c     14 Aug 2020 12:17:34 -0000      1.185
> +++ if_spppsubr.c     22 Aug 2020 14:55:49 -0000
> @@ -1737,7 +1737,7 @@ sppp_lcp_RCR(struct sppp *sp, struct lcp
> 
>       len -= 4;
>       origlen = len;
> -     buf = r = malloc (len, M_TEMP, M_NOWAIT);
> +     buf = r = malloc (origlen, M_TEMP, M_NOWAIT);
>       if (! buf)
>               return (0);
> 
> @@ -1749,7 +1749,7 @@ sppp_lcp_RCR(struct sppp *sp, struct lcp
>       p = (void*) (h+1);
>       for (rlen = 0; len > 1; len -= p[1], p += p[1]) {
>               if (p[1] < 2 || p[1] > len) {
> -                     free(buf, M_TEMP, 0);
> +                     free(buf, M_TEMP, origlen);
>                       return (-1);
>               }
>               if (debug)
> @@ -1926,7 +1926,7 @@ sppp_lcp_RCR(struct sppp *sp, struct lcp
>       }
> 
>  end:
> -     free(buf, M_TEMP, 0);
> +     free(buf, M_TEMP, origlen);
>       return (rlen == 0);
> }
> 
> @@ -2312,7 +2312,7 @@ sppp_ipcp_RCR(struct sppp *sp, struct lc
> {
>       u_char *buf, *r, *p;
>       struct ifnet *ifp = &sp->pp_if;
> -     int rlen, origlen, debug = ifp->if_flags & IFF_DEBUG;
> +     int rlen, origlen, buflen, debug = ifp->if_flags & IFF_DEBUG;
>       u_int32_t hisaddr, desiredaddr;
> 
>       len -= 4;
> @@ -2321,7 +2321,8 @@ sppp_ipcp_RCR(struct sppp *sp, struct lc
>        * Make sure to allocate a buf that can at least hold a
>        * conf-nak with an `address' option.  We might need it below.
>        */
> -     buf = r = malloc ((len < 6? 6: len), M_TEMP, M_NOWAIT);
> +     buflen = len < 6? 6: len;
> +     buf = r = malloc (buflen, M_TEMP, M_NOWAIT);
>       if (! buf)
>               return (0);
> 
> @@ -2332,7 +2333,7 @@ sppp_ipcp_RCR(struct sppp *sp, struct lc
>       p = (void*) (h+1);
>       for (rlen = 0; len > 1; len -= p[1], p += p[1]) {
>               if (p[1] < 2 || p[1] > len) {
> -                     free(buf, M_TEMP, 0);
> +                     free(buf, M_TEMP, buflen);
>                       return (-1);
>               }
>               if (debug)
> @@ -2476,7 +2477,7 @@ sppp_ipcp_RCR(struct sppp *sp, struct lc
>       }
> 
>  end:
> -     free(buf, M_TEMP, 0);
> +     free(buf, M_TEMP, buflen);
>       return (rlen == 0);
> }
> 
> @@ -2773,7 +2774,7 @@ sppp_ipv6cp_RCR(struct sppp *sp, struct 
> {
>       u_char *buf, *r, *p;
>       struct ifnet *ifp = &sp->pp_if;
> -     int rlen, origlen, debug = ifp->if_flags & IFF_DEBUG;
> +     int rlen, origlen, buflen, debug = ifp->if_flags & IFF_DEBUG;
>       struct in6_addr myaddr, desiredaddr, suggestaddr;
>       int ifidcount;
>       int type;
> @@ -2786,7 +2787,8 @@ sppp_ipv6cp_RCR(struct sppp *sp, struct 
>        * Make sure to allocate a buf that can at least hold a
>        * conf-nak with an `address' option.  We might need it below.
>        */
> -     buf = r = malloc ((len < 6? 6: len), M_TEMP, M_NOWAIT);
> +     buflen = len < 6? 6: len;
> +     buf = r = malloc (buflen, M_TEMP, M_NOWAIT);
>       if (! buf)
>               return (0);
> 
> @@ -2799,7 +2801,7 @@ sppp_ipv6cp_RCR(struct sppp *sp, struct 
>       for (rlen=0; len>1 && p[1]; len-=p[1], p+=p[1]) {
>               /* Sanity check option length */
>               if (p[1] < 2 || p[1] > len) {
> -                     free(buf, M_TEMP, 0);
> +                     free(buf, M_TEMP, buflen);
>                       return (-1);
>               }
>               if (debug)
> @@ -2933,7 +2935,7 @@ sppp_ipv6cp_RCR(struct sppp *sp, struct 
>       }
> 
> end:
> -     free(buf, M_TEMP, 0);
> +     free(buf, M_TEMP, buflen);
>       return (rlen == 0);
> }
> 
> @@ -4475,10 +4477,10 @@ sppp_get_params(struct sppp *sp, struct 
>               spr->phase = sp->pp_phase;
> 
>               if (copyout(spr, (caddr_t)ifr->ifr_data, sizeof(*spr)) != 0) {
> -                     free(spr, M_DEVBUF, 0);
> +                     free(spr, M_DEVBUF, sizeof(*spr));
>                       return EFAULT;
>               }
> -             free(spr, M_DEVBUF, 0);
> +             free(spr, M_DEVBUF, sizeof(*spr));
>               break;
>       }
>       case SPPPIOGMAUTH:
> @@ -4498,10 +4500,10 @@ sppp_get_params(struct sppp *sp, struct 
>                       strlcpy(spa->name, auth->name, sizeof(spa->name));
> 
>               if (copyout(spa, (caddr_t)ifr->ifr_data, sizeof(*spa)) != 0) {
> -                     free(spa, M_DEVBUF, 0);
> +                     free(spa, M_DEVBUF, sizeof(*spa));
>                       return EFAULT;
>               }
> -             free(spa, M_DEVBUF, 0);
> +             free(spa, M_DEVBUF, sizeof(*spa));
>               break;
>       }
>       default:
> @@ -4528,7 +4530,7 @@ sppp_set_params(struct sppp *sp, struct 
>               spr = malloc(sizeof(*spr), M_DEVBUF, M_WAITOK);
> 
>               if (copyin((caddr_t)ifr->ifr_data, spr, sizeof(*spr)) != 0) {
> -                     free(spr, M_DEVBUF, 0);
> +                     free(spr, M_DEVBUF, sizeof(*spr));
>                       return EFAULT;
>               }
>               /*
> @@ -4537,7 +4539,7 @@ sppp_set_params(struct sppp *sp, struct 
>                *
>                * XXX Should allow to set or clear pp_flags.
>                */
> -             free(spr, M_DEVBUF, 0);
> +             free(spr, M_DEVBUF, sizeof(*spr));
>               break;
>       }
>       case SPPPIOSMAUTH:
> @@ -4564,13 +4566,13 @@ sppp_set_params(struct sppp *sp, struct 
>               auth = (cmd == SPPPIOSMAUTH) ? &sp->myauth : &sp->hisauth;
> 
>               if (copyin((caddr_t)ifr->ifr_data, spa, sizeof(*spa)) != 0) {
> -                     free(spa, M_DEVBUF, 0);
> +                     free(spa, M_DEVBUF, sizeof(*spa));
>                       return EFAULT;
>               }
> 
>               if (spa->proto != 0 && spa->proto != PPP_PAP &&
>                   spa->proto != PPP_CHAP) {
> -                     free(spa, M_DEVBUF, 0);
> +                     free(spa, M_DEVBUF, sizeof(*spa));
>                       return EINVAL;
>               }
> 
> @@ -4609,7 +4611,7 @@ sppp_set_params(struct sppp *sp, struct 
>                               auth->secret = p;
>                       }
>               }
> -             free(spa, M_DEVBUF, 0);
> +             free(spa, M_DEVBUF, sizeof(*spa));
>               break;
>       }
>       default:
> 

Reply via email to