Re: sppp: add size to free() calls
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 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 - 1.185
> +++ if_spppsubr.c 22 Aug 2020 14:55:49 -
> @@ -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 = >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 = >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,
Re: sppp: add size to free() calls
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 - 1.185
+++ if_spppsubr.c 22 Aug 2020 14:55:49 -
@@ -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 = >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 = >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) {
-
Re: sppp: add size to free() calls
ok mvs@
> On 22 Aug 2020, at 15:32, 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?
>
>
> 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 - 1.185
> +++ if_spppsubr.c 22 Aug 2020 12:25:37 -
> @@ -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 = >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 = >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);
> +
