Thread (9 messages) flat view 9 messages, 4 authors, 2022-02-22
STALE1630d

Revision v1 of 139 in this series.

Revisions (139)
  1. v1 [diff vs current]
  2. v1 [diff vs current]
  3. v1 [diff vs current]
  4. v1 [diff vs current]
  5. v1 [diff vs current]
  6. v1 [diff vs current]
  7. v1 [diff vs current]
  8. v1 [diff vs current]
  9. v1 [diff vs current]
  10. v1 [diff vs current]
  11. v1 [diff vs current]
  12. v1 [diff vs current]
  13. v1 [diff vs current]
  14. v1 [diff vs current]
  15. v1 [diff vs current]
  16. v1 [diff vs current]
  17. v1 [diff vs current]
  18. v1 [diff vs current]
  19. v1 [diff vs current]
  20. v1 [diff vs current]
  21. v1 [diff vs current]
  22. v1 [diff vs current]
  23. v1 [diff vs current]
  24. v1 [diff vs current]
  25. v1 [diff vs current]
  26. v1 [diff vs current]
  27. v1 [diff vs current]
  28. v1 [diff vs current]
  29. v1 [diff vs current]
  30. v1 [diff vs current]
  31. v1 [diff vs current]
  32. v1 [diff vs current]
  33. v1 [diff vs current]
  34. v1 [diff vs current]
  35. v1 [diff vs current]
  36. v1 [diff vs current]
  37. v1 [diff vs current]
  38. v1 [diff vs current]
  39. v1 [diff vs current]
  40. v1 [diff vs current]
  41. v1 [diff vs current]
  42. v1 [diff vs current]
  43. v1 [diff vs current]
  44. v1 [diff vs current]
  45. v1 [diff vs current]
  46. v1 [diff vs current]
  47. v1 [diff vs current]
  48. v1 [diff vs current]
  49. v1 [diff vs current]
  50. v1 [diff vs current]
  51. v1 [diff vs current]
  52. v1 [diff vs current]
  53. v1 [diff vs current]
  54. v1 [diff vs current]
  55. v1 [diff vs current]
  56. v1 [diff vs current]
  57. v1 [diff vs current]
  58. v1 [diff vs current]
  59. v1 [diff vs current]
  60. v1 [diff vs current]
  61. v1 [diff vs current]
  62. v1 [diff vs current]
  63. v1 [diff vs current]
  64. v1 [diff vs current]
  65. v1 [diff vs current]
  66. v1 [diff vs current]
  67. v1 [diff vs current]
  68. v1 [diff vs current]
  69. v1 [diff vs current]
  70. v1 [diff vs current]
  71. v1 [diff vs current]
  72. v1 [diff vs current]
  73. v1 [diff vs current]
  74. v1 [diff vs current]
  75. v1 [diff vs current]
  76. v1 [diff vs current]
  77. v1 [diff vs current]
  78. v1 [diff vs current]
  79. v1 [diff vs current]
  80. v1 [diff vs current]
  81. v2 [diff vs current]
  82. v1 [diff vs current]
  83. v1 [diff vs current]
  84. v1 [diff vs current]
  85. v1 [diff vs current]
  86. v1 [diff vs current]
  87. v1 [diff vs current]
  88. v1 [diff vs current]
  89. v1 [diff vs current]
  90. v1 current
  91. v2 [diff vs current]
  92. v1 [diff vs current]
  93. v1 [diff vs current]
  94. v1 [diff vs current]
  95. v1 [diff vs current]
  96. v1 [diff vs current]
  97. v1 [diff vs current]
  98. v1 [diff vs current]
  99. v1 [diff vs current]
  100. v1 [diff vs current]
  101. v1 [diff vs current]
  102. v1 [diff vs current]
  103. v1 [diff vs current]
  104. v1 [diff vs current]
  105. v1 [diff vs current]
  106. v1 [diff vs current]
  107. v1 [diff vs current]
  108. v1 [diff vs current]
  109. v1 [diff vs current]
  110. v1 [diff vs current]
  111. v2 [diff vs current]
  112. v1 [diff vs current]
  113. v1 [diff vs current]
  114. v1 [diff vs current]
  115. v2 [diff vs current]
  116. v1 [diff vs current]
  117. v1 [diff vs current]
  118. v1 [diff vs current]
  119. v1 [diff vs current]
  120. v1 [diff vs current]
  121. v2 [diff vs current]
  122. v3 [diff vs current]
  123. v1 [diff vs current]
  124. v1 [diff vs current]
  125. v1 [diff vs current]
  126. v2 [diff vs current]
  127. v1 [diff vs current]
  128. v1 [diff vs current]
  129. v1 [diff vs current]
  130. v1 [diff vs current]
  131. v2 [diff vs current]
  132. v1 [diff vs current]
  133. v2 [diff vs current]
  134. v1 [diff vs current]
  135. v1 [diff vs current]
  136. v1 [diff vs current]
  137. v1 [diff vs current]
  138. v2 [diff vs current]
  139. v1 [diff vs current]

[PATCH nf 5/5] netfilter: nf_tables: fix memory leak during stateful obj update

From: Pablo Neira Ayuso <pablo@netfilter.org>
Date: 2022-02-21 16:18:25
Also in: netfilter-devel
Subsystem: netfilter, networking [general], the rest · Maintainers: Pablo Neira Ayuso, Florian Westphal, "David S. Miller", Eric Dumazet, Jakub Kicinski, Paolo Abeni, Linus Torvalds 

From: Florian Westphal <fw@strlen.de>

stateful objects can be updated from the control plane.
The transaction logic allocates a temporary object for this purpose.

The ->init function was called for this object, so plain kfree() leaks
resources. We must call ->destroy function of the object.

nft_obj_destroy does this, but it also decrements the module refcount,
but the update path doesn't increment it.

To avoid special-casing the update object release, do module_get for
the update case too and release it via nft_obj_destroy().

Fixes: d62d0ba97b58 ("netfilter: nf_tables: Introduce stateful object update operation")
Cc: Fernando Fernandez Mancera <redacted>
Signed-off-by: Florian Westphal <fw@strlen.de>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 net/netfilter/nf_tables_api.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
index 3081c4399f10..49060f281342 100644
--- a/net/netfilter/nf_tables_api.c
+++ b/net/netfilter/nf_tables_api.c
@@ -6553,10 +6553,13 @@ static int nf_tables_updobj(const struct nft_ctx *ctx,
 	struct nft_trans *trans;
 	int err;
 
+	if (!try_module_get(type->owner))
+		return -ENOENT;
+
 	trans = nft_trans_alloc(ctx, NFT_MSG_NEWOBJ,
 				sizeof(struct nft_trans_obj));
 	if (!trans)
-		return -ENOMEM;
+		goto err_trans;
 
 	newobj = nft_obj_init(ctx, type, attr);
 	if (IS_ERR(newobj)) {
@@ -6573,6 +6576,8 @@ static int nf_tables_updobj(const struct nft_ctx *ctx,
 
 err_free_trans:
 	kfree(trans);
+err_trans:
+	module_put(type->owner);
 	return err;
 }
 
@@ -8185,7 +8190,7 @@ static void nft_obj_commit_update(struct nft_trans *trans)
 	if (obj->ops->update)
 		obj->ops->update(obj, newobj);
 
-	kfree(newobj);
+	nft_obj_destroy(&trans->ctx, newobj);
 }
 
 static void nft_commit_release(struct nft_trans *trans)
@@ -8976,7 +8981,7 @@ static int __nf_tables_abort(struct net *net, enum nfnl_abort_action action)
 			break;
 		case NFT_MSG_NEWOBJ:
 			if (nft_trans_obj_update(trans)) {
-				kfree(nft_trans_obj_newobj(trans));
+				nft_obj_destroy(&trans->ctx, nft_trans_obj_newobj(trans));
 				nft_trans_destroy(trans);
 			} else {
 				trans->ctx.table->use--;
-- 
2.30.2
Other recent patches touching these files
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help