Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

sockets: fix incorrect malloc size #1220

Merged
merged 2 commits into from Oct 6, 2020

Conversation

azhadchenko
Copy link
Contributor

Fix malloc size: use size * filter_size instead of typo size * size.

In case when there are filter with 4 commands this would corrupt malloc chunk (reproduce)

(1732.81300 Error (criu/cr-restore.c:1968): 38 killed by signal 6: Aborted
(1732.81304 Error (criu/cr-restore.c:3022): Restoring FAILED. 
backtrace:
Program received signal SIGABRT, Aborted.
__GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:50
50  ../sysdeps/unix/sysv/linux/raise.c: No such file or directory.
(gdb) bt
#0  __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:50
#1  0x00007f2eab371535 in __GI_abort () at abort.c:79
#2  0x00007f2eab3c8508 in __libc_message (action=action@entry=do_abort, 
    fmt=fmt@entry=0x7f2eab4d328d "%s\n") at ../sysdeps/posix/libc_fatal.c:181
#3  0x00007f2eab3cec1a in malloc_printerr (
    str=str@entry=0x7f2eab4d4bf8 "munmap_chunk(): invalid pointer") at malloc.c:5341
#4  0x00007f2eab3cf184 in munmap_chunk (p=<optimized out>) at malloc.c:2830
#5  0x00005567a718c3d9 in core_entry__free_unpacked (message=0x5567a7cc62b0, allocator=0x0)
    at images/core.pb-c.c:323
#6  0x00005567a71c2b16 in sigreturn_restore (pid=38, task_args=0x28000, alen=4096, core=0x5567a7cc62b0)
    at criu/cr-restore.c:4318
#7  0x00005567a71bac90 in restore_one_alive_task (pid=38, core=0x5567a7cc62b0)
    at criu/cr-restore.c:1291
#8  0x00005567a71bb889 in restore_one_task (pid=38, core=0x5567a7cc62b0) at criu/cr-restore.c:1657
#9  0x00005567a71bd9b7 in restore_task_with_children (_arg=0x7ffff01f2230) at criu/cr-restore.c:2368
#10 0x00007f2eab4484cf in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:95
(gdb)

@adrianreber
Copy link
Member

Can the reproducer you linked also be included in CRIU's test suite? Or is this something that the github version of CRIU cannot handle yet?

@azhadchenko
Copy link
Contributor Author

Can the reproducer you linked also be included in CRIU's test suite? Or is this something that the github version of CRIU cannot handle yet?

I have written reproduce in form of zdtm test. I don't think that the reproduce is valuable since it is basically a copy of socket_filter zdtm test with different filter (e.g. any filter with length 4).
Of course I can add it if you insist.

criu/sockets.c Outdated
if (!sfp.filter)
return -1;

decode_filter(soe->so_filter, sfp.filter, sfp.len);
ret = restore_opt(sk, SOL_SOCKET, SO_ATTACH_FILTER, &sfp);
if (ret)
pr_perror("Can't restore filter");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you should use pr_err in this case, because there is no guarantee that restore_opt returns a correct errno.

@avagin
Copy link
Member

avagin commented Oct 5, 2020

I have written reproduce in form of zdtm test. I don't think that the reproduce is valuable since it is basically a copy of socket_filter zdtm test with different filter (e.g. any filter with length 4).

You can modify the exiting test.

Signed-off-by: Andrey Zhadchenko <andrey.zhadchenko@virtuozzo.com>
A little rework of sock_filter test to be able to use it with different
filters

Signed-off-by: Andrey Zhadchenko <andrey.zhadchenko@virtuozzo.com>
@azhadchenko
Copy link
Contributor Author

Changed pr_perror to pr_err
Reworked sock_filter test:

  • sock_filter -> sock_filter00
  • added new define SOCK_FILTER01 that is responsible for choosing filter
  • added sock_filter01 symlink to sock_filter00. Makefile pass SOCK_FILTER01 define to this new test.

@avagin avagin merged commit 318fea8 into checkpoint-restore:criu-dev Oct 6, 2020
@avagin
Copy link
Member

avagin commented Oct 6, 2020

Applied. Thanks a lot!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants