xfrm: use list_for_each_entry_safe in xfrm_policy_flush

The iterated pol maybe be freed since it is not protected
by RCU or spinlock when put it, lead to UAF, so use _safe
function to iterate over it against removal

Signed-off-by: Li RongQing <lirongqing@baidu.com>
---
 net/xfrm/xfrm_policy.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Li RongQing <lirongqing@baidu.com> wrote:
> The iterated pol maybe be freed since it is not protected
> by RCU or spinlock when put it, lead to UAF, so use _safe
> function to iterate over it against removal
> 
> Signed-off-by: Li RongQing <lirongqing@baidu.com>
> ---
>  net/xfrm/xfrm_policy.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/net/xfrm/xfrm_policy.c b/net/xfrm/xfrm_policy.c
> index 3235562f6588..87d770dab1f5 100644
> --- a/net/xfrm/xfrm_policy.c
> +++ b/net/xfrm/xfrm_policy.c
> @@ -1772,7 +1772,7 @@ xfrm_policy_flush_secctx_check(struct net *net, u8 type, bool task_valid)
>  int xfrm_policy_flush(struct net *net, u8 type, bool task_valid)
>  {
>  	int dir, err = 0, cnt = 0;
> -	struct xfrm_policy *pol;
> +	struct xfrm_policy *pol, *tmp;
>  
>  	spin_lock_bh(&net->xfrm.xfrm_policy_lock);
>  
> @@ -1781,7 +1781,7 @@ int xfrm_policy_flush(struct net *net, u8 type, bool task_valid)
>  		goto out;
>  
>  again:
> -	list_for_each_entry(pol, &net->xfrm.policy_all, walk.all) {
> +	list_for_each_entry_safe(pol, tmp, &net->xfrm.policy_all, walk.all) {
>  		dir = xfrm_policy_id2dir(pol->index);
>  		if (pol->walk.dead ||
>  		    dir >= XFRM_POLICY_MAX ||

This function drops the lock, but after re-acquire jumps to the 'again'
label, so I do not see the UAF as the entire loop gets restarted.

> -----邮件原件-----
> 发件人: Florian Westphal [mailto:fw@strlen.de]
> 发送时间: 2019年7月1日 17:04
> 收件人: Li,Rongqing <lirongqing@baidu.com>
> 抄送: netdev@vger.kernel.org
> 主题: Re: [PATCH] xfrm: use list_for_each_entry_safe in xfrm_policy_flush
> 
> Li RongQing <lirongqing@baidu.com> wrote:
> > The iterated pol maybe be freed since it is not protected by RCU or
> > spinlock when put it, lead to UAF, so use _safe function to iterate
> > over it against removal
> >
> > Signed-off-by: Li RongQing <lirongqing@baidu.com>
> > ---
> >  net/xfrm/xfrm_policy.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/net/xfrm/xfrm_policy.c b/net/xfrm/xfrm_policy.c index
> > 3235562f6588..87d770dab1f5 100644
> > --- a/net/xfrm/xfrm_policy.c
> > +++ b/net/xfrm/xfrm_policy.c
> > @@ -1772,7 +1772,7 @@ xfrm_policy_flush_secctx_check(struct net *net,
> > u8 type, bool task_valid)  int xfrm_policy_flush(struct net *net, u8
> > type, bool task_valid)  {
> >  	int dir, err = 0, cnt = 0;
> > -	struct xfrm_policy *pol;
> > +	struct xfrm_policy *pol, *tmp;
> >
> >  	spin_lock_bh(&net->xfrm.xfrm_policy_lock);
> >
> > @@ -1781,7 +1781,7 @@ int xfrm_policy_flush(struct net *net, u8 type, bool
> task_valid)
> >  		goto out;
> >
> >  again:
> > -	list_for_each_entry(pol, &net->xfrm.policy_all, walk.all) {
> > +	list_for_each_entry_safe(pol, tmp, &net->xfrm.policy_all, walk.all)
> > +{
> >  		dir = xfrm_policy_id2dir(pol->index);
> >  		if (pol->walk.dead ||
> >  		    dir >= XFRM_POLICY_MAX ||
> 
> This function drops the lock, but after re-acquire jumps to the 'again'
> label, so I do not see the UAF as the entire loop gets restarted.

You are right, sorry for the noise

-Li