Ring buffer size as module parameter #1671

nvanheuverzwijn · 2020-07-29T13:51:52Z

This PR makes the ring_buf_size variable as a module parameter, configurable via /sys/module/MODULE_NAME/parameters/ring_buf_size.

This should help users to prevent syscall drop error message by making the ring_buf_size bigger.

Related issues
falcosecurity/falco#813

CMakeLists.txt

@@ -92,6 +92,7 @@ if(NOT WIN32)
 		if(NOT DEFINED PROBE_NAME)
 			set(PROBE_NAME "sysdig-probe")
 		endif()
+		string(REPLACE "-" "_" PROBE_NAME ${PROBE_NAME})


userspace/libscap/scap.c

@@ -323,7 +323,16 @@ scap_t* scap_open_live_int(char *error, int32_t *rc,
 		//
 		// Allocate the device descriptors.
 		//
-		len = RING_BUF_SIZE * 2;
+
+		FILE * fp = fopen("/sys/module/" PROBE_NAME "/parameters/ring_buf_size", "r");


antoinedeschenes · 2020-07-31T11:57:57Z

userspace/libscap/scap.c

+
+		FILE * fp = fopen("/sys/module/" PROBE_NAME "/parameters/ring_buf_size", "r");
+		if (fp == NULL){
+			snprintf(error, SCAP_LASTERR_SIZE, "Could not read module parameter ring_bug_size at '/sys/module/" PROBE_NAME "/parameters/ring_buf_size'");


Would it be worth adding a fallback to the current default value? (In case libscap doesn't match the kernel module value)

fntlnz · 2020-08-14T13:45:44Z

Thanks for opening @nvanheuverzwijn - This is very needed for Falco!

I'm taking a look.

nvanheuverzwijn · 2020-08-31T19:46:17Z

@fntlnz If that helps, we run with these patches in our production kubernetes cluster without problems so far!

leogr · 2020-09-14T10:24:45Z

Any updates on this?

jannis-a · 2020-09-14T22:43:11Z

+1

@nvanheuverzwijn do you run this with Falco o sysdig secure?

antoinedeschenes · 2020-09-15T11:55:27Z

@jannis-a we're running this with a patched Falco 0.24.0 version

nvanheuverzwijn · 2020-11-18T19:44:37Z

@fntlnz Hi, is this going to be merged anytime soon ?

…/module/. Linux kernel replaces dash (`-`) by underscore (`_`) when installing a module with a dash in it's name. sysdig-CLA-1.0-signed-off-by: Nicolas Vanheuverzwijn <[email protected]>

sysdig-CLA-1.0-signed-off-by: Nicolas Vanheuverzwijn <[email protected]>

instead of "Ring buffer size too small (4096 bytes, must be at least 4096 bytes", message is now "Ring buffer size too small (4096 bytes, must be at least 8192 bytes)" sysdig-CLA-1.0-signed-off-by: Nicolas Vanheuverzwijn <[email protected]>

sysdig-CLA-1.0-signed-off-by: Nicolas Vanheuverzwijn <[email protected]>

…umers parameter sysdig-CLA-1.0-signed-off-by: Nicolas Vanheuverzwijn <[email protected]>

fntlnz · 2020-11-27T14:45:41Z

@nvanheuverzwijn I'm reviewing and I rebased the branch with the current dev.

fntlnz

Hi !

If I understand correctly, what you want to do here is to have a sysfs structure to place a configurable ring_buf_size and adjust the ring buffer based on your needs.

To justify this change my question is: Do you have any benchmarks to share with us on how did you tune the number?

Second, updating the value of ring_buf_size at runtime leads to a page fault. Logs are following

fntlnz · 2020-11-27T14:39:35Z

userspace/libscap/scap.c

@@ -114,7 +114,7 @@ scap_t* scap_open_udig_int(char *error, int32_t *rc,
 static uint32_t get_max_consumers()
 {
 	uint32_t max;
-	FILE *pfile = fopen("/sys/module/" PROBE_DEVICE_NAME "_probe/parameters/max_consumers", "r");
+	FILE *pfile = fopen("/sys/module/" SYSFS_NAME "/parameters/max_consumers", "r");


a reminder for ourselves: if this gets merged the libscap patch that Falco uses needs to be slimmed down https://github.com/falcosecurity/falco/blob/master/cmake/modules/sysdig-repo/patch/libscap.patch#L19

fntlnz · 2020-11-27T15:37:47Z

Here are the logs for the page fault.

click for logs

Nov 27 16:21:30.040508 gallifrey kernel: BUG: unable to handle page fault for address: ffffb18600c61000
Nov 27 16:21:30.040953 gallifrey kernel: #PF: supervisor write access in kernel mode
Nov 27 16:21:30.053242 gallifrey kernel: #PF: error_code(0x0002) - not-present page
Nov 27 16:21:30.061407 gallifrey kernel: PGD 43f800067 P4D 43f800067 PUD 43f99b067 PMD 3f9dcc067 PTE 0
Nov 27 16:21:30.069413 gallifrey kernel: Oops: 0002 [#1] PREEMPT SMP PTI
Nov 27 16:21:30.069476 gallifrey kernel: CPU: 7 PID: 1523 Comm: gnome-terminal- Tainted: G           OE     5.9.8-arch1-1 #1
Nov 27 16:21:30.069509 gallifrey kernel: Hardware name: LENOVO 20KH002FUS/20KH002FUS, BIOS N23ET74W (1.49 ) 06/03/2020
Nov 27 16:21:30.070733 gallifrey kernel: RIP: 0010:record_event_consumer+0x191/0x7a0 [sysdig_probe]
Nov 27 16:21:30.070782 gallifrey kernel: Code: 24 68 01 c0 89 44 24 6c 48 83 c0 1a 48 39 c2 0f 82 fc 04 00 00 48 8b 53 10 89 c8 65 48 8b 34 25 c0 7b 01 00 48 01 c2 45 39 d1 <4c> 89 2a 48 63 be 08 05 00 00 66 44 89 62 14 48 89 7a 08 8b 7c 24
Nov 27 16:21:30.070815 gallifrey kernel: RSP: 0018:ffffb186010efd78 EFLAGS: 00010293
Nov 27 16:21:30.070844 gallifrey kernel: RAX: 0000000000004000 RBX: ffffd185ffdce2d0 RCX: 0000000000004000
Nov 27 16:21:30.070874 gallifrey kernel: RDX: ffffb18600c61000 RSI: ffff969674383e00 RDI: ffffffffb2d8c52e
Nov 27 16:21:30.070903 gallifrey kernel: RBP: ffffb186010efeb8 R08: ffffb18600769000 R09: 000000000025703c
Nov 27 16:21:30.070941 gallifrey kernel: R10: 0000000000257fff R11: 0000000000000000 R12: 0000000000000006
Nov 27 16:21:30.070979 gallifrey kernel: R13: 164b6696d1c7967f R14: ffffb18600721000 R15: 0000000000000006
Nov 27 16:21:30.071018 gallifrey kernel: FS:  00007f25d841d180(0000) GS:ffff9696817c0000(0000) knlGS:0000000000000000
Nov 27 16:21:30.071292 gallifrey kernel: CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
Nov 27 16:21:30.071340 gallifrey kernel: CR2: ffffb18600c61000 CR3: 00000004343ec001 CR4: 00000000003706e0
Nov 27 16:21:30.071372 gallifrey kernel: DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
Nov 27 16:21:30.071410 gallifrey kernel: DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
Nov 27 16:21:30.071443 gallifrey kernel: Call Trace:
Nov 27 16:21:30.071621 gallifrey kernel:  ? record_event_consumer+0x323/0x7a0 [sysdig_probe]
Nov 27 16:21:30.071658 gallifrey kernel:  syscall_enter_probe+0x1d1/0x1f0 [sysdig_probe]
Nov 27 16:21:30.071687 gallifrey kernel:  syscall_trace_enter.constprop.0+0x1b6/0x1e0
Nov 27 16:21:30.071752 gallifrey kernel:  do_syscall_64+0xf/0x40
Nov 27 16:21:30.071782 gallifrey kernel:  entry_SYSCALL_64_after_hwframe+0x44/0xa9
Nov 27 16:21:30.071822 gallifrey kernel: RIP: 0033:0x7f25dbaab00c
Nov 27 16:21:30.071851 gallifrey kernel: Code: ec 28 48 89 54 24 18 48 89 74 24 10 89 7c 24 08 e8 19 fc ff ff 48 8b 54 24 18 48 8b 74 24 10 41 89 c0 8b 7c 24 08 31 c0 0f 05 <48> 3d 00 f0 ff ff 77 34 44 89 c7 48 89 44 24 08 e8 6f fc ff ff 48
N

Honestly, we have two options. Do this read only and only allow to modify the value via module insertion like it's done for max_consumers or make the handler reload the module and the consumer to apply the changes.

The second option is a bit harder because it requires the module to have a way to tell the consumers to remap the memory in userspace after resizing the buffer. This could also have security implications that are not coming in mind right now.

leodido

I've concerns about letting users change the ring buffer size at runtime.

What are the use cases to change the ring buffer size during run time?
I see more cons than benefits because doing it has a lot of implications and opens the path to a lot of unknowns that should be handled (and are not, at the moment).

My vote goes to make it configurable at insertion time, just like max_consumers.

leodido · 2020-11-27T16:03:30Z

userspace/libscap/scap.c

@@ -114,7 +114,7 @@ scap_t* scap_open_udig_int(char *error, int32_t *rc,
 static uint32_t get_max_consumers()
 {
 	uint32_t max;
-	FILE *pfile = fopen("/sys/module/" PROBE_DEVICE_NAME "_probe/parameters/max_consumers", "r");
+	FILE *pfile = fopen("/sys/module/" SYSFS_NAME "/parameters/max_consumers", "r");


a reminder for ourselves: if this gets merged the libscap patch that Falco uses needs to be slimmed down https://github.com/falcosecurity/falco/blob/master/cmake/modules/sysdig-repo/patch/libscap.patch#L19

fntlnz · 2020-11-27T17:18:05Z

I agree with @leodido - this is the option we want:

My vote goes to make it configurable at insertion time, just like max_consumers.

antoinedeschenes approved these changes Jul 31, 2020

View reviewed changes

nvanheuverzwijn force-pushed the ring-buffer-parameter branch 2 times, most recently from 62cf5a4 to 43905bd Compare August 5, 2020 15:57

This was referenced Aug 14, 2020

fix(userspace/libscap): use proper module name in sysfs paths #1674

Closed

Set module arguments via falco-driver-loader falcosecurity/falco#1342

Closed

nvanheuverzwijn added 5 commits November 27, 2020 15:44

driver: add cmake SYSFS_NAME which is the module name as seen in /sys…

00353b1

…/module/. Linux kernel replaces dash (`-`) by underscore (`_`) when installing a module with a dash in it's name. sysdig-CLA-1.0-signed-off-by: Nicolas Vanheuverzwijn <[email protected]>

driver: rename 'RING_BUF_SIZE' to 'ring_buf_size' and removed 'const'

6a18d6b

sysdig-CLA-1.0-signed-off-by: Nicolas Vanheuverzwijn <[email protected]>

driver: ring_buf_size is now a module parameter

71e5f67

sysdig-CLA-1.0-signed-off-by: Nicolas Vanheuverzwijn <[email protected]>

libscap: use SYSFS_NAME instead of PROBE_DEVICE_NAME to read max_cons…

73569dd

…umers parameter sysdig-CLA-1.0-signed-off-by: Nicolas Vanheuverzwijn <[email protected]>

fntlnz force-pushed the ring-buffer-parameter branch from 537e0cb to 73569dd Compare November 27, 2020 14:45

fntlnz reviewed Nov 27, 2020

View reviewed changes

leodido reviewed Nov 27, 2020

View reviewed changes

leogr added the falcosecurity-contrib label Feb 19, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Ring buffer size as module parameter #1671

Ring buffer size as module parameter #1671

nvanheuverzwijn commented Jul 29, 2020

This comment was marked as outdated.

This comment was marked as outdated.

antoinedeschenes Jul 31, 2020

This comment was marked as outdated.

fntlnz commented Aug 14, 2020

nvanheuverzwijn commented Aug 31, 2020

leogr commented Sep 14, 2020

jannis-a commented Sep 14, 2020

antoinedeschenes commented Sep 15, 2020

nvanheuverzwijn commented Nov 18, 2020

fntlnz commented Nov 27, 2020

fntlnz left a comment

fntlnz Nov 27, 2020

leodido Nov 27, 2020

fntlnz commented Nov 27, 2020

leodido left a comment

leodido Nov 27, 2020

fntlnz commented Nov 27, 2020

Ring buffer size as module parameter #1671

Are you sure you want to change the base?

Ring buffer size as module parameter #1671

Conversation

nvanheuverzwijn commented Jul 29, 2020

This comment was marked as outdated.

This comment was marked as outdated.

antoinedeschenes Jul 31, 2020

Choose a reason for hiding this comment

This comment was marked as outdated.

fntlnz commented Aug 14, 2020

nvanheuverzwijn commented Aug 31, 2020

leogr commented Sep 14, 2020

jannis-a commented Sep 14, 2020

antoinedeschenes commented Sep 15, 2020

nvanheuverzwijn commented Nov 18, 2020

fntlnz commented Nov 27, 2020

fntlnz left a comment

Choose a reason for hiding this comment

fntlnz Nov 27, 2020

Choose a reason for hiding this comment

leodido Nov 27, 2020

Choose a reason for hiding this comment

fntlnz commented Nov 27, 2020

leodido left a comment

Choose a reason for hiding this comment

leodido Nov 27, 2020

Choose a reason for hiding this comment

fntlnz commented Nov 27, 2020