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

sk-inet: Add support for checkpoint/restore of ICMP sockets #2558

Open
wants to merge 6 commits into
base: criu-dev
Choose a base branch
from

Conversation

ss141309
Copy link

Currently there is no option to checkpoint/restore programs that use ICMP sockets, such as ping. This patch adds support for the same.

Fixes #2557

Currently there is no option to checkpoint/restore programs that use
ICMP sockets, such as `ping`. This patch adds support for the same.

Fixes checkpoint-restore#2557

Signed-off-by: समीर सिंह Sameer Singh <[email protected]>
@rst0git
Copy link
Member

rst0git commented Dec 27, 2024

@ss141309 Thank you for opening this pull request! Would you be able to add a ZDTM test for this functionality?

Example:

Add a ZDTM static test for the ICMP socket feature.

Signed-off-by: समीर सिंह Sameer Singh <[email protected]>
@ss141309
Copy link
Author

@rst0git oops, it looks like I forgot to add an IP6 version of the test, do I need to create it?

test_init(argc, argv);

test_daemon();
test_waitsig();
Copy link
Member

Choose a reason for hiding this comment

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

@ss141309 test_daemon()/test_waitsig() should be called when the socket has been created.
The following page provides more information about the ZDTM API: https://criu.org/ZDTM_API

Copy link
Member

Choose a reason for hiding this comment

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

Ideally, the test should fail if the patch that adds support for ICMP sockets has not been applied.

Copy link
Author

Choose a reason for hiding this comment

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

@rst0git do you have an idea why I am not able to create an ICMP socket in the zdtm test suite?

#include "zdtmtst.h"
#include <unistd.h>
#include <sys/socket.h>
#include <netinet/in.h>
#include <netinet/ip_icmp.h>
#include <arpa/inet.h>
#include <sys/time.h>
#include <netdb.h>

int main(int argc, char **argv)
{
  int sock;
  test_init(argc, argv);

  sock = socket(PF_INET, SOCK_DGRAM, IPPROTO_ICMP);
	if (sock < 0) {
		pr_perror("Can't create socket");
		return 1;
	}

	test_daemon();
	test_waitsig();

        close(sock);
	pass();
	return 0;
}

I run the test like this and get the output:

ss141309@fedora:~/criu/test$ sudo ./zdtm.py run --ignore-taint -a
[sudo] password for ss141309: 
userns is supported
The kernel is tainted: '12289'
Skipping zdtm/lib/groups (manual run only)
Skipping zdtm/static/conntracks (manual run only)
=== Run 3/3 ================ zdtm/static/socket_icmp
====================== Run zdtm/static/socket_icmp in ns =======================
 DEP       socket_icmp.d
 CC        socket_icmp.o
 LINK      socket_icmp
Start test
./socket_icmp --pidfile=socket_icmp.pid --outfile=socket_icmp.out
The test failed (0, 1)
make: *** [Makefile:504: socket_icmp.pid] Error 1
 Test zdtm/static/socket_icmp FAIL at ['make', '--no-print-directory', '-C', 'zdtm/static', 'socket_icmp.pid'] 
Test output: ================================
10:23:46.792:     4: ERR: socket_icmp.c:25: Can't create socket (errno = 13 (Permission denied))
10:23:46.792:     3: ERR: test.c:320: Test exited unexpectedly with code 1

 <<< ================================
##################################### FAIL #####################################

Copy link
Member

Choose a reason for hiding this comment

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

ICMP sockets probably require CAP_NET_RAW. You need to add the suid flag for this test. Here is an example: https://github.com/checkpoint-restore/criu/blob/criu-dev/test/zdtm/static/socket-raw.desc

Copy link
Author

Choose a reason for hiding this comment

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

@avagin using ICMP protocol with SOCK_DGRAM allows it to be run rootless and without requiring CAP_NET_RAW, also even if I add an suid flag in the desc file, the test fails.

Is this a problem with my net.ipv4.ping_group_range variable? It is set to '0 2147483647', therefore the program should have no problem running unprivileged ICMP sockets, this is evident when I run the test manually

make cleanout
sudo make socket_icmp
make socket_icmp.pid
// Then C-R
make socket_icmp.out

The test passes with no problem

Copy link
Member

@rst0git rst0git Dec 30, 2024

Choose a reason for hiding this comment

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

@avagin I was also able to reproduce the error locally.

socket(AF_INET, SOCK_DGRAM, IPPROTO_ICMP) can be used for non-privileged ICMP. This works when used outside ZDTM. However, the ZDTM environment causes socket() to fail with Permission denied even when suid flag is set.

Is this a problem with my net.ipv4.ping_group_range variable?

The test still fails when net.ipv4.ping_group_range is set to 0 2147483647

Copy link
Member

Choose a reason for hiding this comment

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

All zdtm tests are running in separate (new) network namespaces. We have two places where test namespaces are created:

  • test/zdtm_ct.c
  • test/zdtm/lib/ns.c

In both cases, we need to set net.ipv4.ping_group_range. BTW, the net.ipv4.ping_group_range sysctl value should be C/R-ed.

@rst0git
Copy link
Member

rst0git commented Dec 28, 2024

I forgot to add an IP6 version of the test, do I need to create it?

It would be good to have test for this. CRIU is used in some production environments where only IPv6 addresses are being used.

@avagin
Copy link
Member

avagin commented Dec 30, 2024

As far as I remember, ICMP sockets can have attached filters and we need to dump them. Pls take a look at c2cbcaf, maybe some code can be reused.

ZDTM test suite creates separate network namespaces to run
tests. These namespaces do not preserve the value of the sysctl
variable `net.ipv4.ping_group_range` which allows the creation of
unprivileged ICMP sockets. This commit modifies the variable after
the namespaces have been created to allow every GID to create
unprivileged ICMP sockets.

Signed-off-by: समीर सिंह Sameer Singh <[email protected]>
Dump and restore the net.ipv4.ping_group_range variable to allow the
creation of unprivileged ICMP sockets.

Signed-off-by: समीर सिंह Sameer Singh <[email protected]>
Set the sysctl variable net.ipv4.ping_group_range to 40000-50000 to
allow other tests to pass.

Signed-off-by: समीर सिंह Sameer Singh <[email protected]>
modify the IP4/ICMP test so that the socket is created before the call
to `test_daemon()` and `test_waitsig()`. Also add a test IP6/ICMP.

Signed-off-by: समीर सिंह Sameer Singh <[email protected]>
@ss141309
Copy link
Author

ss141309 commented Jan 1, 2025

it seems that the tests are failing because of the GIDs being set in the ping_group_range variable. What should I set them to in the test/zdtm_ct.c and test/zdtm/lib/ns.c files? setting them to 0 2147483647 causes some of the other tests like socket_udp to fail. Also in which flavours should I run the tests?

@ss141309 ss141309 requested review from rst0git and avagin January 2, 2025 09:57
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.

Checkpoint/restore of ICMP sockets is not supported
3 participants