Message ID | 1561969747-8629-1-git-send-email-lirongqing@baidu.com |
---|---|
State | Awaiting Upstream |
Delegated to: | David Miller |
Headers | show |
Series | xfrm: use list_for_each_entry_safe in xfrm_policy_flush | expand |
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
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 ||
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(-)