diff mbox series

[1/2] xfrm: xfrm_policy: fix a possible double xfrm_pols_put() in xfrm_bundle_lookup()

Message ID 20220720080912.1183369-2-steffen.klassert@secunet.com (mailing list archive)
State Accepted
Commit f85daf0e725358be78dfd208dea5fd665d8cb901
Delegated to: Netdev Maintainers
Headers show
Series [1/2] xfrm: xfrm_policy: fix a possible double xfrm_pols_put() in xfrm_bundle_lookup() | expand

Checks

Context Check Description
netdev/tree_selection success Guessed tree name to be net-next
netdev/fixes_present success Fixes tag not required for -next series
netdev/subject_prefix warning Target tree name not specified in the subject
netdev/cover_letter success Pull request is its own cover letter
netdev/patch_count success Link
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 12 this patch: 12
netdev/cc_maintainers fail 1 blamed authors not CCed: timo.teras@iki.fi; 3 maintainers not CCed: timo.teras@iki.fi pabeni@redhat.com edumazet@google.com
netdev/build_clang success Errors and warnings before: 6 this patch: 6
netdev/module_param success Was 0 now: 0
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success Fixes tag looks correct
netdev/build_allmodconfig_warn success Errors and warnings before: 12 this patch: 12
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 18 lines checked
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Steffen Klassert July 20, 2022, 8:09 a.m. UTC
From: Hangyu Hua <hbh25y@gmail.com>

xfrm_policy_lookup() will call xfrm_pol_hold_rcu() to get a refcount of
pols[0]. This refcount can be dropped in xfrm_expand_policies() when
xfrm_expand_policies() return error. pols[0]'s refcount is balanced in
here. But xfrm_bundle_lookup() will also call xfrm_pols_put() with
num_pols == 1 to drop this refcount when xfrm_expand_policies() return
error.

This patch also fix an illegal address access. pols[0] will save a error
point when xfrm_policy_lookup fails. This lead to xfrm_pols_put to resolve
an illegal address in xfrm_bundle_lookup's error path.

Fix these by setting num_pols = 0 in xfrm_expand_policies()'s error path.

Fixes: 80c802f3073e ("xfrm: cache bundles instead of policies for outgoing flows")
Signed-off-by: Hangyu Hua <hbh25y@gmail.com>
Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
---
 net/xfrm/xfrm_policy.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

Comments

patchwork-bot+netdevbpf@kernel.org July 20, 2022, 9:20 a.m. UTC | #1
Hello:

This series was applied to netdev/net.git (master)
by Steffen Klassert <steffen.klassert@secunet.com>:

On Wed, 20 Jul 2022 10:09:11 +0200 you wrote:
> From: Hangyu Hua <hbh25y@gmail.com>
> 
> xfrm_policy_lookup() will call xfrm_pol_hold_rcu() to get a refcount of
> pols[0]. This refcount can be dropped in xfrm_expand_policies() when
> xfrm_expand_policies() return error. pols[0]'s refcount is balanced in
> here. But xfrm_bundle_lookup() will also call xfrm_pols_put() with
> num_pols == 1 to drop this refcount when xfrm_expand_policies() return
> error.
> 
> [...]

Here is the summary with links:
  - [1/2] xfrm: xfrm_policy: fix a possible double xfrm_pols_put() in xfrm_bundle_lookup()
    https://git.kernel.org/netdev/net/c/f85daf0e7253
  - [2/2] net: ipv4: fix clang -Wformat warnings
    https://git.kernel.org/netdev/net/c/e79b9473e9b5

You are awesome, thank you!
diff mbox series

Patch

diff --git a/net/xfrm/xfrm_policy.c b/net/xfrm/xfrm_policy.c
index f1876ea61fdc..f1a0bab920a5 100644
--- a/net/xfrm/xfrm_policy.c
+++ b/net/xfrm/xfrm_policy.c
@@ -2678,8 +2678,10 @@  static int xfrm_expand_policies(const struct flowi *fl, u16 family,
 		*num_xfrms = 0;
 		return 0;
 	}
-	if (IS_ERR(pols[0]))
+	if (IS_ERR(pols[0])) {
+		*num_pols = 0;
 		return PTR_ERR(pols[0]);
+	}
 
 	*num_xfrms = pols[0]->xfrm_nr;
 
@@ -2694,6 +2696,7 @@  static int xfrm_expand_policies(const struct flowi *fl, u16 family,
 		if (pols[1]) {
 			if (IS_ERR(pols[1])) {
 				xfrm_pols_put(pols, *num_pols);
+				*num_pols = 0;
 				return PTR_ERR(pols[1]);
 			}
 			(*num_pols)++;