netdev - 答复: [PATCH] xfrm: use list_for_each_entry_safe in xfrm_policy

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:   Mon, 1 Jul 2019 09:27:20 +0000
From:   "Li,Rongqing" <lirongqing@...du.com>
To:     Florian Westphal <fw@...len.de>
CC:     "netdev@...r.kernel.org" <netdev@...r.kernel.org>
Subject: 答复: [PATCH] xfrm: use list_for_each_entry_safe in xfrm_policy_flush



> -----邮件原件-----
> 发件人: Florian Westphal [mailto:fw@...len.de]
> 发送时间: 2019年7月1日 17:04
> 收件人: Li,Rongqing <lirongqing@...du.com>
> 抄送: netdev@...r.kernel.org
> 主题: Re: [PATCH] xfrm: use list_for_each_entry_safe in xfrm_policy_flush
> 
> Li RongQing <lirongqing@...du.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@...du.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