Re: git: a37e0e6de652 - main - pf: fix more syncookie memory leaks

On 3/06/2022 4:18 am, Kristof Provost wrote:
> The branch main has been updated by kp:
> 
> URL: https://cgit.FreeBSD.org/src/commit/?id=a37e0e6de6527a7eaddea8e28f5e4b3427fba1a4
> 
> commit a37e0e6de6527a7eaddea8e28f5e4b3427fba1a4
> Author:     Franco Fichtner <franco@opnsense.org>
> AuthorDate: 2022-06-02 16:27:43 +0000
> Commit:     Kristof Provost <kp@FreeBSD.org>
> CommitDate: 2022-06-02 18:17:25 +0000
> 
>      pf: fix more syncookie memory leaks
>      
>      Allocate memory for packed nvlists in M_NVLIST, as nvlist_pack() does
>      this as well, and we use the same variable interchangable with the
>      memory we allocate. When we free it we can end up freeing from the wrong
>      zone, leaking memory.
>      
>      Reviewed by:    kp
>      Differential Revision:  https://reviews.freebsd.org/D35385

Hi Kristof,

Are stable{13,12} affected or only introduced in main?

> ---
>   sys/netpfil/pf/pf_ioctl.c      | 20 ++++++++++----------
>   sys/netpfil/pf/pf_syncookies.c |  6 +++---
>   2 files changed, 13 insertions(+), 13 deletions(-)
> 
> diff --git a/sys/netpfil/pf/pf_ioctl.c b/sys/netpfil/pf/pf_ioctl.c
> index 745b9b69060b..1ccbbd3814ac 100644
> --- a/sys/netpfil/pf/pf_ioctl.c
> +++ b/sys/netpfil/pf/pf_ioctl.c
> @@ -2722,7 +2722,7 @@ DIOCGETETHRULES_error:
>   		if (nv->len > pf_ioctl_maxcount)
>   			ERROUT(ENOMEM);
>   
> -		nvlpacked = malloc(nv->len, M_TEMP, M_WAITOK);
> +		nvlpacked = malloc(nv->len, M_NVLIST, M_WAITOK);
>   		if (nvlpacked == NULL)
>   			ERROUT(ENOMEM);
>   
> @@ -2763,7 +2763,7 @@ DIOCGETETHRULES_error:
>   
>   		nvlist_destroy(nvl);
>   		nvl = NULL;
> -		free(nvlpacked, M_TEMP);
> +		free(nvlpacked, M_NVLIST);
>   		nvlpacked = NULL;
>   
>   		rule = TAILQ_FIRST(rs->active.rules);
> @@ -2803,7 +2803,7 @@ DIOCGETETHRULES_error:
>   
>   #undef ERROUT
>   DIOCGETETHRULE_error:
> -		free(nvlpacked, M_TEMP);
> +		free(nvlpacked, M_NVLIST);
>   		nvlist_destroy(nvl);
>   		break;
>   	}
> @@ -2819,7 +2819,7 @@ DIOCGETETHRULE_error:
>   
>   #define ERROUT(x)	ERROUT_IOCTL(DIOCADDETHRULE_error, x)
>   
> -		nvlpacked = malloc(nv->len, M_TEMP, M_WAITOK);
> +		nvlpacked = malloc(nv->len, M_NVLIST, M_WAITOK);
>   		if (nvlpacked == NULL)
>   			ERROUT(ENOMEM);
>   
> @@ -2922,7 +2922,7 @@ DIOCGETETHRULE_error:
>   #undef ERROUT
>   DIOCADDETHRULE_error:
>   		nvlist_destroy(nvl);
> -		free(nvlpacked, M_TEMP);
> +		free(nvlpacked, M_NVLIST);
>   		break;
>   	}
>   
> @@ -3117,7 +3117,7 @@ DIOCGETETHRULESET_error:
>   		if (nv->len > pf_ioctl_maxcount)
>   			ERROUT(ENOMEM);
>   
> -		nvlpacked = malloc(nv->len, M_TEMP, M_WAITOK);
> +		nvlpacked = malloc(nv->len, M_NVLIST, M_WAITOK);
>   		error = copyin(nv->data, nvlpacked, nv->len);
>   		if (error)
>   			ERROUT(error);
> @@ -3156,13 +3156,13 @@ DIOCGETETHRULESET_error:
>   		    anchor_call, td);
>   
>   		nvlist_destroy(nvl);
> -		free(nvlpacked, M_TEMP);
> +		free(nvlpacked, M_NVLIST);
>   		break;
>   #undef ERROUT
>   DIOCADDRULENV_error:
>   		pf_krule_free(rule);
>   		nvlist_destroy(nvl);
> -		free(nvlpacked, M_TEMP);
> +		free(nvlpacked, M_NVLIST);
>   
>   		break;
>   	}
> @@ -6018,7 +6018,7 @@ pf_keepcounters(struct pfioc_nv *nv)
>   	if (nv->len > pf_ioctl_maxcount)
>   		ERROUT(ENOMEM);
>   
> -	nvlpacked = malloc(nv->len, M_TEMP, M_WAITOK);
> +	nvlpacked = malloc(nv->len, M_NVLIST, M_WAITOK);
>   	if (nvlpacked == NULL)
>   		ERROUT(ENOMEM);
>   
> @@ -6037,7 +6037,7 @@ pf_keepcounters(struct pfioc_nv *nv)
>   
>   on_error:
>   	nvlist_destroy(nvl);
> -	free(nvlpacked, M_TEMP);
> +	free(nvlpacked, M_NVLIST);
>   	return (error);
>   }
>   
> diff --git a/sys/netpfil/pf/pf_syncookies.c b/sys/netpfil/pf/pf_syncookies.c
> index 5230502be30c..6a375411d8ea 100644
> --- a/sys/netpfil/pf/pf_syncookies.c
> +++ b/sys/netpfil/pf/pf_syncookies.c
> @@ -171,7 +171,7 @@ pf_get_syncookies(struct pfioc_nv *nv)
>   #undef ERROUT
>   errout:
>   	nvlist_destroy(nvl);
> -	free(nvlpacked, M_TEMP);
> +	free(nvlpacked, M_NVLIST);
>   
>   	return (error);
>   }
> @@ -191,7 +191,7 @@ pf_set_syncookies(struct pfioc_nv *nv)
>   	if (nv->len > pf_ioctl_maxcount)
>   		return (ENOMEM);
>   
> -	nvlpacked = malloc(nv->len, M_TEMP, M_WAITOK);
> +	nvlpacked = malloc(nv->len, M_NVLIST, M_WAITOK);
>   	if (nvlpacked == NULL)
>   		return (ENOMEM);
>   
> @@ -232,7 +232,7 @@ pf_set_syncookies(struct pfioc_nv *nv)
>   #undef ERROUT
>   errout:
>   	nvlist_destroy(nvl);
> -	free(nvlpacked, M_TEMP);
> +	free(nvlpacked, M_NVLIST);
>   
>   	return (error);
>   }
>