* [PATCH bpf, sockmap 0/2] potential memleak and proc stats fix
@ 2021-06-30 21:53 John Fastabend
2021-06-30 21:53 ` [PATCH bpf, sockmap 1/2] bpf, sockmap: fix potential memory leak on unlikely error case John Fastabend
2021-06-30 21:53 ` [PATCH bpf, sockmap 2/2] bpf, sockmap: sk_prot needs inuse_idx set for proc stats John Fastabend
0 siblings, 2 replies; 7+ messages in thread
From: John Fastabend @ 2021-06-30 21:53 UTC (permalink / raw)
To: ast, daniel, andriin; +Cc: john.fastabend, bpf, netdev
While investigating a memleak in sockmap I found these two issues. Patch
1 found doing code review, I wasn't able to get KASAN to trigger a
memleak here, but should be necessary. Patch 2 fixes proc stats so when
we use sockstats for debugging we get correct values.
The fix for observered memleak will come after these, but requires some
more discussion and potentially patch revert so I'll try to get the set
here going now.
John Fastabend (2):
bpf, sockmap: fix potential memory leak on unlikely error case
bpf, sockmap: sk_prot needs inuse_idx set for proc stats
net/core/skmsg.c | 4 +++-
net/core/sock_map.c | 4 +++-
2 files changed, 6 insertions(+), 2 deletions(-)
--
2.25.1
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH bpf, sockmap 1/2] bpf, sockmap: fix potential memory leak on unlikely error case
2021-06-30 21:53 [PATCH bpf, sockmap 0/2] potential memleak and proc stats fix John Fastabend
@ 2021-06-30 21:53 ` John Fastabend
2021-06-30 21:53 ` [PATCH bpf, sockmap 2/2] bpf, sockmap: sk_prot needs inuse_idx set for proc stats John Fastabend
1 sibling, 0 replies; 7+ messages in thread
From: John Fastabend @ 2021-06-30 21:53 UTC (permalink / raw)
To: ast, daniel, andriin; +Cc: john.fastabend, bpf, netdev
If skb_linearize is needed and fails we could leak a msg on the error
handling. To fix ensure we kfree the msg block before returning error.
Found during code review.
Fixes: 4363023d2668e ("bpf, sockmap: Avoid failures from skb_to_sgvec when skb has frag_list")
Signed-off-by: John Fastabend <john.fastabend@gmail.com>
---
net/core/skmsg.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/net/core/skmsg.c b/net/core/skmsg.c
index 9b6160a191f8..22603289c2b2 100644
--- a/net/core/skmsg.c
+++ b/net/core/skmsg.c
@@ -505,8 +505,10 @@ static int sk_psock_skb_ingress_enqueue(struct sk_buff *skb,
* drop the skb. We need to linearize the skb so that the mapping
* in skb_to_sgvec can not error.
*/
- if (skb_linearize(skb))
+ if (skb_linearize(skb)) {
+ kfree(msg);
return -EAGAIN;
+ }
num_sge = skb_to_sgvec(skb, msg->sg.data, 0, skb->len);
if (unlikely(num_sge < 0)) {
kfree(msg);
--
2.25.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH bpf, sockmap 2/2] bpf, sockmap: sk_prot needs inuse_idx set for proc stats
2021-06-30 21:53 [PATCH bpf, sockmap 0/2] potential memleak and proc stats fix John Fastabend
2021-06-30 21:53 ` [PATCH bpf, sockmap 1/2] bpf, sockmap: fix potential memory leak on unlikely error case John Fastabend
@ 2021-06-30 21:53 ` John Fastabend
2021-07-01 6:58 ` kernel test robot
2021-07-01 8:21 ` kernel test robot
1 sibling, 2 replies; 7+ messages in thread
From: John Fastabend @ 2021-06-30 21:53 UTC (permalink / raw)
To: ast, daniel, andriin; +Cc: john.fastabend, bpf, netdev
Proc socket stats use sk_prot->inuse_idx value to record inuse sock stats.
We currently do not set this correctly from sockmap side. The result is
reading sock stats '/proc/net/sockstat' gives incorrect values. The
socket counter is incremented correctly, but because we don't set the
counter correctly when we replace sk_prot we may omit the decrement.
Fixes: 604326b41a6fb ("bpf, sockmap: convert to generic sk_msg interface")
Signed-off-by: John Fastabend <john.fastabend@gmail.com>
---
net/core/sock_map.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/net/core/sock_map.c b/net/core/sock_map.c
index 60decd6420ca..29e7bae65db5 100644
--- a/net/core/sock_map.c
+++ b/net/core/sock_map.c
@@ -221,7 +221,7 @@ static int sock_map_link(struct bpf_map *map, struct sock *sk)
struct bpf_prog *skb_verdict = NULL;
struct bpf_prog *msg_parser = NULL;
struct sk_psock *psock;
- int ret;
+ int ret, idx;
/* Only sockets we can redirect into/from in BPF need to hold
* refs to parser/verdict progs and have their sk_data_ready
@@ -293,9 +293,11 @@ static int sock_map_link(struct bpf_map *map, struct sock *sk)
if (msg_parser)
psock_set_prog(&psock->progs.msg_parser, msg_parser);
+ idx = sk->sk_prot->inuse_idx;
ret = sock_map_init_proto(sk, psock);
if (ret < 0)
goto out_drop;
+ sk->sk_prot->inuse_idx = idx;
write_lock_bh(&sk->sk_callback_lock);
if (stream_parser && stream_verdict && !psock->saved_data_ready) {
--
2.25.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH bpf, sockmap 2/2] bpf, sockmap: sk_prot needs inuse_idx set for proc stats
2021-06-30 21:53 ` [PATCH bpf, sockmap 2/2] bpf, sockmap: sk_prot needs inuse_idx set for proc stats John Fastabend
@ 2021-07-01 6:58 ` kernel test robot
2021-07-01 8:21 ` kernel test robot
1 sibling, 0 replies; 7+ messages in thread
From: kernel test robot @ 2021-07-01 6:58 UTC (permalink / raw)
To: John Fastabend, ast, daniel, andriin
Cc: kbuild-all, john.fastabend, bpf, netdev
[-- Attachment #1: Type: text/plain, Size: 6355 bytes --]
Hi John,
I love your patch! Yet something to improve:
[auto build test ERROR on linus/master]
[also build test ERROR on v5.13 next-20210630]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/John-Fastabend/potential-memleak-and-proc-stats-fix/20210701-055546
base: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 440462198d9c45e48f2d8d9b18c5702d92282f46
config: sparc-buildonly-randconfig-r006-20210701 (attached as .config)
compiler: sparc64-linux-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/0day-ci/linux/commit/82ee893f50c6899cb557f22d0ae9a657b14d183f
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review John-Fastabend/potential-memleak-and-proc-stats-fix/20210701-055546
git checkout 82ee893f50c6899cb557f22d0ae9a657b14d183f
# save the attached .config to linux build tree
mkdir build_dir
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross O=build_dir ARCH=sparc SHELL=/bin/bash net/core/
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All errors (new ones prefixed by >>):
net/core/sock_map.c: In function 'sock_map_link':
>> net/core/sock_map.c:296:19: error: 'struct proto' has no member named 'inuse_idx'
296 | idx = sk->sk_prot->inuse_idx;
| ^~
net/core/sock_map.c:300:13: error: 'struct proto' has no member named 'inuse_idx'
300 | sk->sk_prot->inuse_idx = idx;
| ^~
vim +296 net/core/sock_map.c
215
216 static int sock_map_link(struct bpf_map *map, struct sock *sk)
217 {
218 struct sk_psock_progs *progs = sock_map_progs(map);
219 struct bpf_prog *stream_verdict = NULL;
220 struct bpf_prog *stream_parser = NULL;
221 struct bpf_prog *skb_verdict = NULL;
222 struct bpf_prog *msg_parser = NULL;
223 struct sk_psock *psock;
224 int ret, idx;
225
226 /* Only sockets we can redirect into/from in BPF need to hold
227 * refs to parser/verdict progs and have their sk_data_ready
228 * and sk_write_space callbacks overridden.
229 */
230 if (!sock_map_redirect_allowed(sk))
231 goto no_progs;
232
233 stream_verdict = READ_ONCE(progs->stream_verdict);
234 if (stream_verdict) {
235 stream_verdict = bpf_prog_inc_not_zero(stream_verdict);
236 if (IS_ERR(stream_verdict))
237 return PTR_ERR(stream_verdict);
238 }
239
240 stream_parser = READ_ONCE(progs->stream_parser);
241 if (stream_parser) {
242 stream_parser = bpf_prog_inc_not_zero(stream_parser);
243 if (IS_ERR(stream_parser)) {
244 ret = PTR_ERR(stream_parser);
245 goto out_put_stream_verdict;
246 }
247 }
248
249 msg_parser = READ_ONCE(progs->msg_parser);
250 if (msg_parser) {
251 msg_parser = bpf_prog_inc_not_zero(msg_parser);
252 if (IS_ERR(msg_parser)) {
253 ret = PTR_ERR(msg_parser);
254 goto out_put_stream_parser;
255 }
256 }
257
258 skb_verdict = READ_ONCE(progs->skb_verdict);
259 if (skb_verdict) {
260 skb_verdict = bpf_prog_inc_not_zero(skb_verdict);
261 if (IS_ERR(skb_verdict)) {
262 ret = PTR_ERR(skb_verdict);
263 goto out_put_msg_parser;
264 }
265 }
266
267 no_progs:
268 psock = sock_map_psock_get_checked(sk);
269 if (IS_ERR(psock)) {
270 ret = PTR_ERR(psock);
271 goto out_progs;
272 }
273
274 if (psock) {
275 if ((msg_parser && READ_ONCE(psock->progs.msg_parser)) ||
276 (stream_parser && READ_ONCE(psock->progs.stream_parser)) ||
277 (skb_verdict && READ_ONCE(psock->progs.skb_verdict)) ||
278 (skb_verdict && READ_ONCE(psock->progs.stream_verdict)) ||
279 (stream_verdict && READ_ONCE(psock->progs.skb_verdict)) ||
280 (stream_verdict && READ_ONCE(psock->progs.stream_verdict))) {
281 sk_psock_put(sk, psock);
282 ret = -EBUSY;
283 goto out_progs;
284 }
285 } else {
286 psock = sk_psock_init(sk, map->numa_node);
287 if (IS_ERR(psock)) {
288 ret = PTR_ERR(psock);
289 goto out_progs;
290 }
291 }
292
293 if (msg_parser)
294 psock_set_prog(&psock->progs.msg_parser, msg_parser);
295
> 296 idx = sk->sk_prot->inuse_idx;
297 ret = sock_map_init_proto(sk, psock);
298 if (ret < 0)
299 goto out_drop;
300 sk->sk_prot->inuse_idx = idx;
301
302 write_lock_bh(&sk->sk_callback_lock);
303 if (stream_parser && stream_verdict && !psock->saved_data_ready) {
304 ret = sk_psock_init_strp(sk, psock);
305 if (ret)
306 goto out_unlock_drop;
307 psock_set_prog(&psock->progs.stream_verdict, stream_verdict);
308 psock_set_prog(&psock->progs.stream_parser, stream_parser);
309 sk_psock_start_strp(sk, psock);
310 } else if (!stream_parser && stream_verdict && !psock->saved_data_ready) {
311 psock_set_prog(&psock->progs.stream_verdict, stream_verdict);
312 sk_psock_start_verdict(sk,psock);
313 } else if (!stream_verdict && skb_verdict && !psock->saved_data_ready) {
314 psock_set_prog(&psock->progs.skb_verdict, skb_verdict);
315 sk_psock_start_verdict(sk, psock);
316 }
317 write_unlock_bh(&sk->sk_callback_lock);
318 return 0;
319 out_unlock_drop:
320 write_unlock_bh(&sk->sk_callback_lock);
321 out_drop:
322 sk_psock_put(sk, psock);
323 out_progs:
324 if (skb_verdict)
325 bpf_prog_put(skb_verdict);
326 out_put_msg_parser:
327 if (msg_parser)
328 bpf_prog_put(msg_parser);
329 out_put_stream_parser:
330 if (stream_parser)
331 bpf_prog_put(stream_parser);
332 out_put_stream_verdict:
333 if (stream_verdict)
334 bpf_prog_put(stream_verdict);
335 return ret;
336 }
337
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 39363 bytes --]
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH bpf, sockmap 2/2] bpf, sockmap: sk_prot needs inuse_idx set for proc stats
@ 2021-07-01 6:58 ` kernel test robot
0 siblings, 0 replies; 7+ messages in thread
From: kernel test robot @ 2021-07-01 6:58 UTC (permalink / raw)
To: kbuild-all
[-- Attachment #1: Type: text/plain, Size: 6525 bytes --]
Hi John,
I love your patch! Yet something to improve:
[auto build test ERROR on linus/master]
[also build test ERROR on v5.13 next-20210630]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/John-Fastabend/potential-memleak-and-proc-stats-fix/20210701-055546
base: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 440462198d9c45e48f2d8d9b18c5702d92282f46
config: sparc-buildonly-randconfig-r006-20210701 (attached as .config)
compiler: sparc64-linux-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/0day-ci/linux/commit/82ee893f50c6899cb557f22d0ae9a657b14d183f
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review John-Fastabend/potential-memleak-and-proc-stats-fix/20210701-055546
git checkout 82ee893f50c6899cb557f22d0ae9a657b14d183f
# save the attached .config to linux build tree
mkdir build_dir
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross O=build_dir ARCH=sparc SHELL=/bin/bash net/core/
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All errors (new ones prefixed by >>):
net/core/sock_map.c: In function 'sock_map_link':
>> net/core/sock_map.c:296:19: error: 'struct proto' has no member named 'inuse_idx'
296 | idx = sk->sk_prot->inuse_idx;
| ^~
net/core/sock_map.c:300:13: error: 'struct proto' has no member named 'inuse_idx'
300 | sk->sk_prot->inuse_idx = idx;
| ^~
vim +296 net/core/sock_map.c
215
216 static int sock_map_link(struct bpf_map *map, struct sock *sk)
217 {
218 struct sk_psock_progs *progs = sock_map_progs(map);
219 struct bpf_prog *stream_verdict = NULL;
220 struct bpf_prog *stream_parser = NULL;
221 struct bpf_prog *skb_verdict = NULL;
222 struct bpf_prog *msg_parser = NULL;
223 struct sk_psock *psock;
224 int ret, idx;
225
226 /* Only sockets we can redirect into/from in BPF need to hold
227 * refs to parser/verdict progs and have their sk_data_ready
228 * and sk_write_space callbacks overridden.
229 */
230 if (!sock_map_redirect_allowed(sk))
231 goto no_progs;
232
233 stream_verdict = READ_ONCE(progs->stream_verdict);
234 if (stream_verdict) {
235 stream_verdict = bpf_prog_inc_not_zero(stream_verdict);
236 if (IS_ERR(stream_verdict))
237 return PTR_ERR(stream_verdict);
238 }
239
240 stream_parser = READ_ONCE(progs->stream_parser);
241 if (stream_parser) {
242 stream_parser = bpf_prog_inc_not_zero(stream_parser);
243 if (IS_ERR(stream_parser)) {
244 ret = PTR_ERR(stream_parser);
245 goto out_put_stream_verdict;
246 }
247 }
248
249 msg_parser = READ_ONCE(progs->msg_parser);
250 if (msg_parser) {
251 msg_parser = bpf_prog_inc_not_zero(msg_parser);
252 if (IS_ERR(msg_parser)) {
253 ret = PTR_ERR(msg_parser);
254 goto out_put_stream_parser;
255 }
256 }
257
258 skb_verdict = READ_ONCE(progs->skb_verdict);
259 if (skb_verdict) {
260 skb_verdict = bpf_prog_inc_not_zero(skb_verdict);
261 if (IS_ERR(skb_verdict)) {
262 ret = PTR_ERR(skb_verdict);
263 goto out_put_msg_parser;
264 }
265 }
266
267 no_progs:
268 psock = sock_map_psock_get_checked(sk);
269 if (IS_ERR(psock)) {
270 ret = PTR_ERR(psock);
271 goto out_progs;
272 }
273
274 if (psock) {
275 if ((msg_parser && READ_ONCE(psock->progs.msg_parser)) ||
276 (stream_parser && READ_ONCE(psock->progs.stream_parser)) ||
277 (skb_verdict && READ_ONCE(psock->progs.skb_verdict)) ||
278 (skb_verdict && READ_ONCE(psock->progs.stream_verdict)) ||
279 (stream_verdict && READ_ONCE(psock->progs.skb_verdict)) ||
280 (stream_verdict && READ_ONCE(psock->progs.stream_verdict))) {
281 sk_psock_put(sk, psock);
282 ret = -EBUSY;
283 goto out_progs;
284 }
285 } else {
286 psock = sk_psock_init(sk, map->numa_node);
287 if (IS_ERR(psock)) {
288 ret = PTR_ERR(psock);
289 goto out_progs;
290 }
291 }
292
293 if (msg_parser)
294 psock_set_prog(&psock->progs.msg_parser, msg_parser);
295
> 296 idx = sk->sk_prot->inuse_idx;
297 ret = sock_map_init_proto(sk, psock);
298 if (ret < 0)
299 goto out_drop;
300 sk->sk_prot->inuse_idx = idx;
301
302 write_lock_bh(&sk->sk_callback_lock);
303 if (stream_parser && stream_verdict && !psock->saved_data_ready) {
304 ret = sk_psock_init_strp(sk, psock);
305 if (ret)
306 goto out_unlock_drop;
307 psock_set_prog(&psock->progs.stream_verdict, stream_verdict);
308 psock_set_prog(&psock->progs.stream_parser, stream_parser);
309 sk_psock_start_strp(sk, psock);
310 } else if (!stream_parser && stream_verdict && !psock->saved_data_ready) {
311 psock_set_prog(&psock->progs.stream_verdict, stream_verdict);
312 sk_psock_start_verdict(sk,psock);
313 } else if (!stream_verdict && skb_verdict && !psock->saved_data_ready) {
314 psock_set_prog(&psock->progs.skb_verdict, skb_verdict);
315 sk_psock_start_verdict(sk, psock);
316 }
317 write_unlock_bh(&sk->sk_callback_lock);
318 return 0;
319 out_unlock_drop:
320 write_unlock_bh(&sk->sk_callback_lock);
321 out_drop:
322 sk_psock_put(sk, psock);
323 out_progs:
324 if (skb_verdict)
325 bpf_prog_put(skb_verdict);
326 out_put_msg_parser:
327 if (msg_parser)
328 bpf_prog_put(msg_parser);
329 out_put_stream_parser:
330 if (stream_parser)
331 bpf_prog_put(stream_parser);
332 out_put_stream_verdict:
333 if (stream_verdict)
334 bpf_prog_put(stream_verdict);
335 return ret;
336 }
337
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org
[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 39363 bytes --]
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH bpf, sockmap 2/2] bpf, sockmap: sk_prot needs inuse_idx set for proc stats
2021-06-30 21:53 ` [PATCH bpf, sockmap 2/2] bpf, sockmap: sk_prot needs inuse_idx set for proc stats John Fastabend
@ 2021-07-01 8:21 ` kernel test robot
2021-07-01 8:21 ` kernel test robot
1 sibling, 0 replies; 7+ messages in thread
From: kernel test robot @ 2021-07-01 8:21 UTC (permalink / raw)
To: John Fastabend, ast, daniel, andriin
Cc: clang-built-linux, kbuild-all, john.fastabend, bpf, netdev
[-- Attachment #1: Type: text/plain, Size: 6499 bytes --]
Hi John,
I love your patch! Yet something to improve:
[auto build test ERROR on linus/master]
[also build test ERROR on v5.13 next-20210630]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/John-Fastabend/potential-memleak-and-proc-stats-fix/20210701-055546
base: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 440462198d9c45e48f2d8d9b18c5702d92282f46
config: arm64-randconfig-r026-20210630 (attached as .config)
compiler: clang version 13.0.0 (https://github.com/llvm/llvm-project e7e71e9454ed76c1b3d8140170b5333c28bef1be)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# install arm64 cross compiling tool for clang build
# apt-get install binutils-aarch64-linux-gnu
# https://github.com/0day-ci/linux/commit/82ee893f50c6899cb557f22d0ae9a657b14d183f
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review John-Fastabend/potential-memleak-and-proc-stats-fix/20210701-055546
git checkout 82ee893f50c6899cb557f22d0ae9a657b14d183f
# save the attached .config to linux build tree
mkdir build_dir
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross O=build_dir ARCH=arm64 SHELL=/bin/bash arch/arm64/ net/
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All errors (new ones prefixed by >>):
>> net/core/sock_map.c:296:21: error: no member named 'inuse_idx' in 'struct proto'
idx = sk->sk_prot->inuse_idx;
~~~~~~~~~~~ ^
net/core/sock_map.c:300:15: error: no member named 'inuse_idx' in 'struct proto'
sk->sk_prot->inuse_idx = idx;
~~~~~~~~~~~ ^
2 errors generated.
vim +296 net/core/sock_map.c
215
216 static int sock_map_link(struct bpf_map *map, struct sock *sk)
217 {
218 struct sk_psock_progs *progs = sock_map_progs(map);
219 struct bpf_prog *stream_verdict = NULL;
220 struct bpf_prog *stream_parser = NULL;
221 struct bpf_prog *skb_verdict = NULL;
222 struct bpf_prog *msg_parser = NULL;
223 struct sk_psock *psock;
224 int ret, idx;
225
226 /* Only sockets we can redirect into/from in BPF need to hold
227 * refs to parser/verdict progs and have their sk_data_ready
228 * and sk_write_space callbacks overridden.
229 */
230 if (!sock_map_redirect_allowed(sk))
231 goto no_progs;
232
233 stream_verdict = READ_ONCE(progs->stream_verdict);
234 if (stream_verdict) {
235 stream_verdict = bpf_prog_inc_not_zero(stream_verdict);
236 if (IS_ERR(stream_verdict))
237 return PTR_ERR(stream_verdict);
238 }
239
240 stream_parser = READ_ONCE(progs->stream_parser);
241 if (stream_parser) {
242 stream_parser = bpf_prog_inc_not_zero(stream_parser);
243 if (IS_ERR(stream_parser)) {
244 ret = PTR_ERR(stream_parser);
245 goto out_put_stream_verdict;
246 }
247 }
248
249 msg_parser = READ_ONCE(progs->msg_parser);
250 if (msg_parser) {
251 msg_parser = bpf_prog_inc_not_zero(msg_parser);
252 if (IS_ERR(msg_parser)) {
253 ret = PTR_ERR(msg_parser);
254 goto out_put_stream_parser;
255 }
256 }
257
258 skb_verdict = READ_ONCE(progs->skb_verdict);
259 if (skb_verdict) {
260 skb_verdict = bpf_prog_inc_not_zero(skb_verdict);
261 if (IS_ERR(skb_verdict)) {
262 ret = PTR_ERR(skb_verdict);
263 goto out_put_msg_parser;
264 }
265 }
266
267 no_progs:
268 psock = sock_map_psock_get_checked(sk);
269 if (IS_ERR(psock)) {
270 ret = PTR_ERR(psock);
271 goto out_progs;
272 }
273
274 if (psock) {
275 if ((msg_parser && READ_ONCE(psock->progs.msg_parser)) ||
276 (stream_parser && READ_ONCE(psock->progs.stream_parser)) ||
277 (skb_verdict && READ_ONCE(psock->progs.skb_verdict)) ||
278 (skb_verdict && READ_ONCE(psock->progs.stream_verdict)) ||
279 (stream_verdict && READ_ONCE(psock->progs.skb_verdict)) ||
280 (stream_verdict && READ_ONCE(psock->progs.stream_verdict))) {
281 sk_psock_put(sk, psock);
282 ret = -EBUSY;
283 goto out_progs;
284 }
285 } else {
286 psock = sk_psock_init(sk, map->numa_node);
287 if (IS_ERR(psock)) {
288 ret = PTR_ERR(psock);
289 goto out_progs;
290 }
291 }
292
293 if (msg_parser)
294 psock_set_prog(&psock->progs.msg_parser, msg_parser);
295
> 296 idx = sk->sk_prot->inuse_idx;
297 ret = sock_map_init_proto(sk, psock);
298 if (ret < 0)
299 goto out_drop;
300 sk->sk_prot->inuse_idx = idx;
301
302 write_lock_bh(&sk->sk_callback_lock);
303 if (stream_parser && stream_verdict && !psock->saved_data_ready) {
304 ret = sk_psock_init_strp(sk, psock);
305 if (ret)
306 goto out_unlock_drop;
307 psock_set_prog(&psock->progs.stream_verdict, stream_verdict);
308 psock_set_prog(&psock->progs.stream_parser, stream_parser);
309 sk_psock_start_strp(sk, psock);
310 } else if (!stream_parser && stream_verdict && !psock->saved_data_ready) {
311 psock_set_prog(&psock->progs.stream_verdict, stream_verdict);
312 sk_psock_start_verdict(sk,psock);
313 } else if (!stream_verdict && skb_verdict && !psock->saved_data_ready) {
314 psock_set_prog(&psock->progs.skb_verdict, skb_verdict);
315 sk_psock_start_verdict(sk, psock);
316 }
317 write_unlock_bh(&sk->sk_callback_lock);
318 return 0;
319 out_unlock_drop:
320 write_unlock_bh(&sk->sk_callback_lock);
321 out_drop:
322 sk_psock_put(sk, psock);
323 out_progs:
324 if (skb_verdict)
325 bpf_prog_put(skb_verdict);
326 out_put_msg_parser:
327 if (msg_parser)
328 bpf_prog_put(msg_parser);
329 out_put_stream_parser:
330 if (stream_parser)
331 bpf_prog_put(stream_parser);
332 out_put_stream_verdict:
333 if (stream_verdict)
334 bpf_prog_put(stream_verdict);
335 return ret;
336 }
337
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 46463 bytes --]
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH bpf, sockmap 2/2] bpf, sockmap: sk_prot needs inuse_idx set for proc stats
@ 2021-07-01 8:21 ` kernel test robot
0 siblings, 0 replies; 7+ messages in thread
From: kernel test robot @ 2021-07-01 8:21 UTC (permalink / raw)
To: kbuild-all
[-- Attachment #1: Type: text/plain, Size: 6671 bytes --]
Hi John,
I love your patch! Yet something to improve:
[auto build test ERROR on linus/master]
[also build test ERROR on v5.13 next-20210630]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/John-Fastabend/potential-memleak-and-proc-stats-fix/20210701-055546
base: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 440462198d9c45e48f2d8d9b18c5702d92282f46
config: arm64-randconfig-r026-20210630 (attached as .config)
compiler: clang version 13.0.0 (https://github.com/llvm/llvm-project e7e71e9454ed76c1b3d8140170b5333c28bef1be)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# install arm64 cross compiling tool for clang build
# apt-get install binutils-aarch64-linux-gnu
# https://github.com/0day-ci/linux/commit/82ee893f50c6899cb557f22d0ae9a657b14d183f
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review John-Fastabend/potential-memleak-and-proc-stats-fix/20210701-055546
git checkout 82ee893f50c6899cb557f22d0ae9a657b14d183f
# save the attached .config to linux build tree
mkdir build_dir
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross O=build_dir ARCH=arm64 SHELL=/bin/bash arch/arm64/ net/
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All errors (new ones prefixed by >>):
>> net/core/sock_map.c:296:21: error: no member named 'inuse_idx' in 'struct proto'
idx = sk->sk_prot->inuse_idx;
~~~~~~~~~~~ ^
net/core/sock_map.c:300:15: error: no member named 'inuse_idx' in 'struct proto'
sk->sk_prot->inuse_idx = idx;
~~~~~~~~~~~ ^
2 errors generated.
vim +296 net/core/sock_map.c
215
216 static int sock_map_link(struct bpf_map *map, struct sock *sk)
217 {
218 struct sk_psock_progs *progs = sock_map_progs(map);
219 struct bpf_prog *stream_verdict = NULL;
220 struct bpf_prog *stream_parser = NULL;
221 struct bpf_prog *skb_verdict = NULL;
222 struct bpf_prog *msg_parser = NULL;
223 struct sk_psock *psock;
224 int ret, idx;
225
226 /* Only sockets we can redirect into/from in BPF need to hold
227 * refs to parser/verdict progs and have their sk_data_ready
228 * and sk_write_space callbacks overridden.
229 */
230 if (!sock_map_redirect_allowed(sk))
231 goto no_progs;
232
233 stream_verdict = READ_ONCE(progs->stream_verdict);
234 if (stream_verdict) {
235 stream_verdict = bpf_prog_inc_not_zero(stream_verdict);
236 if (IS_ERR(stream_verdict))
237 return PTR_ERR(stream_verdict);
238 }
239
240 stream_parser = READ_ONCE(progs->stream_parser);
241 if (stream_parser) {
242 stream_parser = bpf_prog_inc_not_zero(stream_parser);
243 if (IS_ERR(stream_parser)) {
244 ret = PTR_ERR(stream_parser);
245 goto out_put_stream_verdict;
246 }
247 }
248
249 msg_parser = READ_ONCE(progs->msg_parser);
250 if (msg_parser) {
251 msg_parser = bpf_prog_inc_not_zero(msg_parser);
252 if (IS_ERR(msg_parser)) {
253 ret = PTR_ERR(msg_parser);
254 goto out_put_stream_parser;
255 }
256 }
257
258 skb_verdict = READ_ONCE(progs->skb_verdict);
259 if (skb_verdict) {
260 skb_verdict = bpf_prog_inc_not_zero(skb_verdict);
261 if (IS_ERR(skb_verdict)) {
262 ret = PTR_ERR(skb_verdict);
263 goto out_put_msg_parser;
264 }
265 }
266
267 no_progs:
268 psock = sock_map_psock_get_checked(sk);
269 if (IS_ERR(psock)) {
270 ret = PTR_ERR(psock);
271 goto out_progs;
272 }
273
274 if (psock) {
275 if ((msg_parser && READ_ONCE(psock->progs.msg_parser)) ||
276 (stream_parser && READ_ONCE(psock->progs.stream_parser)) ||
277 (skb_verdict && READ_ONCE(psock->progs.skb_verdict)) ||
278 (skb_verdict && READ_ONCE(psock->progs.stream_verdict)) ||
279 (stream_verdict && READ_ONCE(psock->progs.skb_verdict)) ||
280 (stream_verdict && READ_ONCE(psock->progs.stream_verdict))) {
281 sk_psock_put(sk, psock);
282 ret = -EBUSY;
283 goto out_progs;
284 }
285 } else {
286 psock = sk_psock_init(sk, map->numa_node);
287 if (IS_ERR(psock)) {
288 ret = PTR_ERR(psock);
289 goto out_progs;
290 }
291 }
292
293 if (msg_parser)
294 psock_set_prog(&psock->progs.msg_parser, msg_parser);
295
> 296 idx = sk->sk_prot->inuse_idx;
297 ret = sock_map_init_proto(sk, psock);
298 if (ret < 0)
299 goto out_drop;
300 sk->sk_prot->inuse_idx = idx;
301
302 write_lock_bh(&sk->sk_callback_lock);
303 if (stream_parser && stream_verdict && !psock->saved_data_ready) {
304 ret = sk_psock_init_strp(sk, psock);
305 if (ret)
306 goto out_unlock_drop;
307 psock_set_prog(&psock->progs.stream_verdict, stream_verdict);
308 psock_set_prog(&psock->progs.stream_parser, stream_parser);
309 sk_psock_start_strp(sk, psock);
310 } else if (!stream_parser && stream_verdict && !psock->saved_data_ready) {
311 psock_set_prog(&psock->progs.stream_verdict, stream_verdict);
312 sk_psock_start_verdict(sk,psock);
313 } else if (!stream_verdict && skb_verdict && !psock->saved_data_ready) {
314 psock_set_prog(&psock->progs.skb_verdict, skb_verdict);
315 sk_psock_start_verdict(sk, psock);
316 }
317 write_unlock_bh(&sk->sk_callback_lock);
318 return 0;
319 out_unlock_drop:
320 write_unlock_bh(&sk->sk_callback_lock);
321 out_drop:
322 sk_psock_put(sk, psock);
323 out_progs:
324 if (skb_verdict)
325 bpf_prog_put(skb_verdict);
326 out_put_msg_parser:
327 if (msg_parser)
328 bpf_prog_put(msg_parser);
329 out_put_stream_parser:
330 if (stream_parser)
331 bpf_prog_put(stream_parser);
332 out_put_stream_verdict:
333 if (stream_verdict)
334 bpf_prog_put(stream_verdict);
335 return ret;
336 }
337
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org
[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 46463 bytes --]
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2021-07-01 8:21 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-06-30 21:53 [PATCH bpf, sockmap 0/2] potential memleak and proc stats fix John Fastabend
2021-06-30 21:53 ` [PATCH bpf, sockmap 1/2] bpf, sockmap: fix potential memory leak on unlikely error case John Fastabend
2021-06-30 21:53 ` [PATCH bpf, sockmap 2/2] bpf, sockmap: sk_prot needs inuse_idx set for proc stats John Fastabend
2021-07-01 6:58 ` kernel test robot
2021-07-01 6:58 ` kernel test robot
2021-07-01 8:21 ` kernel test robot
2021-07-01 8:21 ` kernel test robot
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.