ok mvs@
> On 22 Aug 2020, at 15:32, Klemens Nanni <k...@openbsd.org> 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?
>
>
> 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 12:25:37 -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;
> }
>
> @@ -4592,7 +4594,7 @@ sppp_set_params(struct sppp *sp, struct
> p = malloc(len, M_DEVBUF, M_WAITOK);
> strlcpy(p, spa->name, len);
> if (auth->name != NULL)
> - free(auth->name, M_DEVBUF, 0);
> + free(auth->name, M_DEVBUF, len);
> auth->name = p;
>
> if (spa->secret[0] != '\0') {
> @@ -4601,7 +4603,7 @@ sppp_set_params(struct sppp *sp, struct
> p = malloc(len, M_DEVBUF, M_WAITOK);
> strlcpy(p, spa->secret, len);
> if (auth->secret != NULL)
> - free(auth->secret, M_DEVBUF, 0);
> + free(auth->secret, M_DEVBUF, len);
> auth->secret = p;
> } else if (!auth->secret) {
> p = malloc(1, M_DEVBUF, M_WAITOK);
> @@ -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:
>
