RFE: add new API to enable addfd support for seccomp notifier #454

realsdx · 2025-02-03T14:42:56Z

This PR adds a new API to enable addfd support for seccomp notifiers. It introduces changes to the seccomp_notif and seccomp_notif_resp structures to handle the addfd operation, allowing file descriptors to be passed through a seccomp notifier.

It also adds supporting man page entry and unit regression tests for both C and Python APIs.

pcmoore · 2025-02-03T21:40:54Z

Hi @realsdx, thanks for the PR :)

Unfortunately, it looks like the the CI test are failing because of a missing seccomp_notif_addfd struct declaration due to an older userspace/headers on the build/CI system. Look at how we handle seccomp_notif and seccomp_notif_resp in the "include/seccomp.h.in" file for an example on how to fix this:

libseccomp/include/seccomp.h.in

Lines 391 to 408 in 7db46d7

    
           /* SECCOMP_RET_USER_NOTIF was added in kernel v5.0. */ 
        
           #ifndef SECCOMP_RET_USER_NOTIF 
        
           #define SECCOMP_RET_USER_NOTIF	0x7fc00000U 
        
           struct seccomp_notif { 
        
           	__u64 id; 
        
           	__u32 pid; 
        
           	__u32 flags; 
        
           	struct seccomp_data data; 
        
           }; 
        
           struct seccomp_notif_resp { 
        
           	__u64 id; 
        
           	__s64 val; 
        
           	__s32 error; 
        
           	__u32 flags; 
        
           }; 
        
           #endif

pcmoore · 2025-02-04T20:58:55Z

FWIW, if I made the following changes on top of your PR I was able to get everything building:

diff --git a/include/seccomp.h.in b/include/seccomp.h.in
index 024aca537f64..568eabaad949 100644
--- a/include/seccomp.h.in
+++ b/include/seccomp.h.in
@@ -406,6 +406,11 @@ struct seccomp_notif_resp {
        __u32 flags;
 };
 
+#endif
+
+/* seccomp_notif_addfd and ADDFD_FLAG_SETFD was added in kernel v5.10 */
+#ifndef SECCOMP_ADDFD_FLAG_SETFD
+#define SECCOMP_ADDFD_FLAG_SETFD       (1UL << 0)
 struct seccomp_notif_addfd {
        __u64 id;
        __u32 flags;
diff --git a/src/system.h b/src/system.h
index 495050fb2e75..87bf8a6e832d 100644
--- a/src/system.h
+++ b/src/system.h
@@ -180,14 +180,6 @@ struct seccomp_notif_resp {
        __u32 flags;
 };
 
-struct seccomp_notif_addfd {
-       __u64 id;
-       __u32 flags;
-       __u32 srcfd;
-       __u32 newfd;
-       __u32 newfd_flags;
-};
-
 #define SECCOMP_IOC_MAGIC               '!'
 #define SECCOMP_IO(nr)                  _IO(SECCOMP_IOC_MAGIC, nr)
 #define SECCOMP_IOR(nr, type)           _IOR(SECCOMP_IOC_MAGIC, nr, type)
@@ -201,6 +193,24 @@ struct seccomp_notif_addfd {
 #define SECCOMP_IOCTL_NOTIF_ID_VALID    SECCOMP_IOW(2, __u64)
 #endif /* SECCOMP_RET_USER_NOTIF */
 
+/* seccomp_notif_addfd and ADDFD_FLAG_SETFD was added in kernel v5.10 */
+#ifndef SECCOMP_ADDFD_FLAG_SETFD
+#define SECCOMP_ADDFD_FLAG_SETFD       (1UL << 0)
+struct seccomp_notif_addfd {
+       __u64 id;
+       __u32 flags;
+       __u32 srcfd;
+       __u32 newfd;
+       __u32 newfd_flags;
+};
+#endif
+
+/* SECCOMP_IOCTL_NOTIF_ADDFD was added in kernel v5.10 */
+#ifndef SECCOMP_IOCTL_NOTIF_ADDFD
+#define SECCOMP_IOCTL_NOTIF_ADDFD      SECCOMP_IOW(3, \
+                                                   struct seccomp_notif_addfd)
+#endif
+
 /* non-public ioctl number for backwards compat (see system.c) */
 #define SECCOMP_IOCTL_NOTIF_ID_VALID_WRONG_DIR SECCOMP_IOR(2, __u64)

pcmoore · 2025-02-04T21:00:23Z

Another quick comment, we'll likely want to add a new API level to seccomp_api_get() to indicate ADDFD support.

realsdx · 2025-02-05T13:32:47Z

Another quick comment, we'll likely want to add a new API level to seccomp_api_get() to indicate ADDFD support.

The current API levels are based on different Seccomp actions and filter flags. If we add a new level for addfd support, it would be based on IOCTL support.

What would be the best way to determine the addfd support at runtime? One issue with probing the IOCTL with notifyfd is that we need a valid notify fd first. Since we cannot uninstall the seccomp filter once it's loaded, is it a good idea to probe for ADDFD IOCTL?

coveralls · 2025-02-09T22:11:35Z

coverage: 88.845% (-0.2%) from 89.046%
when pulling 3de75bb on realsdx:dev/addfd
into 2bc7189 on seccomp:main.

pcmoore · 2025-03-18T00:54:05Z

What would be the best way to determine the addfd support at runtime? One issue with probing the IOCTL with notifyfd is that we need a valid notify fd first. Since we cannot uninstall the seccomp filter once it's loaded, is it a good idea to probe for ADDFD IOCTL?

My apologies, I suspect I was stuck in the thinking that we were doing this through the seccomp() syscall somehow ... but yes, I see what you mean about the ioctl issue. I'm looking at the associated kernel code to see if there something in there for us to use to query for the functionality without actually doing anything, and nothing is jumping out at me. The closest I can think of would be to pass a bogus file descriptor and see if -EBADF is returned; if the kernel support is missing we should see -EINVAL. However, I'm not sure we want to risk doing that in a library, that seems like a Bad Thing.

@drakenclimber do you have any thoughts on this?

pcmoore · 2025-04-08T13:15:10Z

Okay, I'm not seeing any good ideas around the runtime detection so let's just go ahead and skip that. Let me take a closer look at the rest of the PR ...

pcmoore

Added some thoughts on the PR, overall it looks good, just a few minor things to polish up :)

doc/man/man3/seccomp_notify_alloc.3

include/seccomp.h.in

src/api.c

src/python/seccomp.pyx

src/system.c

tests/63-live-notify_addfd.c

pcmoore · 2025-04-08T14:48:10Z

@realsdx mentioned SECCOMP_ADDFD_FLAG_SEND in a separate chat, and I think it might be a nice thing to add both SECCOMP_ADDFD_FLAG_SETFD and SECCOMP_ADDFD_FLAG_SEND to this PR, thoughts?

For reference, SECCOMP_ADDFD_FLAG_SEND injects a file descriptor into the target's file descriptor table while also atomically clearing the target's errno and setting the syscall return value equal to the value of the newly injected file descriptor. More information can be found in the manpage.

timscharfenort8

Good work

pcmoore · 2025-05-09T20:45:12Z

This is looking good @realsdx, did you want to do another push to this PR branch with all of the changes discussed and remove the draft flag on this PR?

pcmoore · 2025-08-18T22:36:12Z

src/system.h

+	__u32 newfd;
+	__u32 newfd_flags;
+};
+#endif


Since we're defining this in include/seccomp.h we shouldn't need it here, unless I'm misunderstanding something?

pcmoore · 2025-08-18T22:36:50Z

src/system.h

+/* Addfd and return it, atomically. ADDFD_FLAG_SEND was added in kernel 5.14 */
+#ifndef SECCOMP_ADDFD_FLAG_SEND
+#define SECCOMP_ADDFD_FLAG_SEND        (1UL << 1)
+#endif


Same thing here, we already define this in include/seccomp.h if the system headers don't define it for us.

This part I am not quite sure yet, as this whole thing only gets loaded when the kernel's seccomp headers are not available. But why would we need to keep the SECCOMP_IOCTL_NOTIF_ADDFD then?

In the early days of libseccomp it was not uncommon to build the library on a system that was running an older kernel without libseccomp support and the necessary kernel header files. Including the relevant kernel header bits (macros, structure definitions) allows us to build the libseccomp library on a system without libseccomp support, but still have it run as one would expect on a modern system with a Linux kernel that fully supports seccomp.

Does that help?

Okay makes sense. I will remove the duplicate flags and structs. Thanks for clarifying.

pcmoore · 2025-08-18T22:38:43Z

@drakenclimber aside from the two things mentioned above, what do you think of this?

drakenclimber · 2025-08-19T17:51:56Z

@drakenclimber aside from the two things mentioned above, what do you think of this?

I'll look into it this week. Thanks for reminding me :)

drakenclimber

I think this is a really good first cut. I had a few comments that will require some minor rework, but overall it's solid.

Thanks for the contribution 👍

drakenclimber · 2025-08-19T19:17:21Z

include/seccomp.h.in

+/* seccomp_notif_addfd and ADDFD_FLAG_SETFD was added in kernel v5.10 */
+#ifndef SECCOMP_ADDFD_FLAG_SETFD
+#define SECCOMP_ADDFD_FLAG_SETFD        (1UL << 0)
+struct seccomp_notif_addfd {


Why is the struct seccomp_notif_addfd() definition inside of the #ifndef SECCOMP_ADDFD_FLAG_SETFD? Seems strange to me.

That's a fair question. The seccomp_notif_addfd struct is only used when SECCOMP_ADDFD_FLAG_SETFD is present and supported, so using the presence of SECCOMP_ADDFD_FLAG_SETFD to serve as a proxy for the definition of seccomp_notif_addfd seems like a reasonable thing to do.

I'm definitely open to other ideas and I'm sure @realsdx would be as well.

https://man7.org/linux/man-pages/man2/seccomp_unotify.2.html

src/system.c

drakenclimber · 2025-08-19T19:23:34Z

src/system.c

+	if (state.sup_user_notif <= 0)
+		return -EOPNOTSUPP;
+
+	int rc = ioctl(fd, SECCOMP_IOCTL_NOTIF_ADDFD, addfd);


As a convention, we define variables at the start of the function in libseccomp. (Aside - this is a relic of the C89 standard, but it's generally good practice today even though it's no longer required.)

tests/63-live-notify_addfd.c

drakenclimber · 2025-08-19T19:35:27Z

tests/63-live-notify_addfd.c

+		close(sock_pair[1]);
+		exit(bytes_read); // bytes_read should be 0, as it's reading /dev/null
+	} else {
+		close(sock_pair[1]); // close the child's end


I would recommend moving the parent code to its own function. This will improve readability and better scope the variables that are only needed by the parent process.

And, as outlined above, it will fix the erroneous goto logic in the child, as you'll have to handle that properly in its own function

tests/63-live-notify_addfd.py

src/python/libseccomp.pxd

drakenclimber · 2025-08-19T19:46:06Z

tests/63-live-notify_addfd.c

+			goto out;
+		}
+
+		new_fd = openat(AT_FDCWD, "/dev/null", O_RDONLY);


Does reading /dev/null provide a good test (since it always results in 0 bytes read)? Or would reading /dev/zero be a better test? Reading it could provide a non-zero length.

I don't have a strong preference here - just wondering

The intent is to simulate a behavior, where the supervisee wants to access a 'restricted' file but gets nothing. Let me know if that makes sense.

drakenclimber · 2025-08-19T20:01:28Z

What would be the best way to determine the addfd support at runtime? One issue with probing the IOCTL with notifyfd is that we need a valid notify fd first. Since we cannot uninstall the seccomp filter once it's loaded, is it a good idea to probe for ADDFD IOCTL?

My apologies, I suspect I was stuck in the thinking that we were doing this through the seccomp() syscall somehow ... but yes, I see what you mean about the ioctl issue. I'm looking at the associated kernel code to see if there something in there for us to use to query for the functionality without actually doing anything, and nothing is jumping out at me. The closest I can think of would be to pass a bogus file descriptor and see if -EBADF is returned; if the kernel support is missing we should see -EINVAL. However, I'm not sure we want to risk doing that in a library, that seems like a Bad Thing.

@drakenclimber do you have any thoughts on this?

I don't have any better ideas here either. Bummer.

realsdx · 2025-10-04T00:34:17Z

First of all, apologies for the delay. I was on a long vacation, and then some work piled up. I should’ve replied much sooner!
Thanks a lot for the thorough review. I definitely made a few silly mistakes and some poor choices, really appreciate you pointing those out.

I’ve made several updates to the C test code (refactored the parent and child functions) and cleaned up some of the debug code.
A couple of things are still pending:

Refactoring the changes into 3 commits, as suggested by @drakenclimber
Fixing the duplicate macros in the system.h file

I still have a few doubts regarding the duplicate macros @pcmoore

pcmoore · 2025-10-07T21:02:20Z

I still have a few doubts regarding the duplicate macros @pcmoore

I think I just answered those in the code review/comment section, but please let me know if that still doesn't make sense.

realsdx · 2025-10-08T20:31:51Z

Thank you, @pcmoore, for the explanation. It's much clearer to me now. I'll update the code with the remaining changes.

Add support for the seccomp notify addfd feature in the core API. It allows a caller to install fd in target's fd table. Signed-off-by: Sudipta Pandit <sudpandit@microsoft.com>

Add seccomp notify addfd support to the Python API and update the receive_notify and respond_notify methods to accept user-provided notify fd's as an optional argument. Signed-off-by: Sudipta Pandit <sudpandit@microsoft.com>

Add live test for seccomp_notify_addfd() that verifies fd injection capabilities by intercepting openat() syscalls and replacing the target's file descriptor for it's C and Python API. Signed-off-by: Sudipta Pandit <sudpandit@microsoft.com>

tests/63-live-notify_addfd.py

src/python/seccomp.pyx

Signed-off-by: Sudipta Pandit <sudpandit@microsoft.com>

pcmoore · 2025-11-25T01:12:01Z

What do you think @drakenclimber ?

realsdx force-pushed the dev/addfd branch from c1ab815 to cb91aac Compare February 3, 2025 14:49

pcmoore added the enhancement label Feb 3, 2025

realsdx force-pushed the dev/addfd branch from cb91aac to b52d26d Compare February 4, 2025 10:38

realsdx force-pushed the dev/addfd branch from b52d26d to 3610e7c Compare February 9, 2025 20:44

pcmoore requested changes Apr 8, 2025

View reviewed changes

timscharfenort8 reviewed Apr 27, 2025

View reviewed changes

realsdx force-pushed the dev/addfd branch from 3610e7c to af71526 Compare May 15, 2025 21:16

realsdx marked this pull request as ready for review May 15, 2025 21:17

realsdx requested a review from pcmoore May 15, 2025 21:17

pcmoore requested changes Aug 18, 2025

View reviewed changes

pcmoore requested a review from drakenclimber August 18, 2025 22:38

pcmoore added the pending/revision label Aug 18, 2025

drakenclimber requested changes Aug 19, 2025

View reviewed changes

realsdx force-pushed the dev/addfd branch 3 times, most recently from 24c9882 to 805517e Compare October 4, 2025 00:29

realsdx added 3 commits October 9, 2025 20:49

api: add the seccomp notify addfd support

593b8a6

Add support for the seccomp notify addfd feature in the core API. It allows a caller to install fd in target's fd table. Signed-off-by: Sudipta Pandit <sudpandit@microsoft.com>

api: add python api for seccomp notify addfd

2808909

Add seccomp notify addfd support to the Python API and update the receive_notify and respond_notify methods to accept user-provided notify fd's as an optional argument. Signed-off-by: Sudipta Pandit <sudpandit@microsoft.com>

tests: add test 63-live-notify_addfd

4188036

Add live test for seccomp_notify_addfd() that verifies fd injection capabilities by intercepting openat() syscalls and replacing the target's file descriptor for it's C and Python API. Signed-off-by: Sudipta Pandit <sudpandit@microsoft.com>

realsdx force-pushed the dev/addfd branch from 805517e to 4188036 Compare October 9, 2025 21:13

realsdx requested review from drakenclimber and pcmoore October 9, 2025 21:13

pcmoore reviewed Nov 23, 2025

View reviewed changes

tests/63-live-notify_addfd.py Outdated Show resolved Hide resolved

pcmoore reviewed Nov 23, 2025

View reviewed changes

src/python/seccomp.pyx Outdated Show resolved Hide resolved

pcmoore reviewed Nov 23, 2025

View reviewed changes

src/python/seccomp.pyx Outdated Show resolved Hide resolved

realsdx added 2 commits November 23, 2025 16:14

api: rename newflags to newfd_flags in python api for addfd

5d22278

Signed-off-by: Sudipta Pandit <sudpandit@microsoft.com>

tests: fix comment in recv_fd test for clarity

3de75bb

Signed-off-by: Sudipta Pandit <sudpandit@microsoft.com>

RFE: add new API to enable addfd support for seccomp notifier #454

Are you sure you want to change the base?

RFE: add new API to enable addfd support for seccomp notifier #454

Uh oh!

Conversation

realsdx commented Feb 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pcmoore commented Feb 3, 2025

Uh oh!

pcmoore commented Feb 4, 2025

Uh oh!

pcmoore commented Feb 4, 2025

Uh oh!

realsdx commented Feb 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

coveralls commented Feb 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pcmoore commented Mar 18, 2025

Uh oh!

pcmoore commented Apr 8, 2025

Uh oh!

pcmoore left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

pcmoore commented Apr 8, 2025

Uh oh!

timscharfenort8 left a comment

Choose a reason for hiding this comment

Uh oh!

pcmoore commented May 9, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pcmoore commented Aug 18, 2025

Uh oh!

drakenclimber commented Aug 19, 2025

Uh oh!

drakenclimber left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

drakenclimber commented Aug 19, 2025

realsdx commented Feb 3, 2025 •

edited

Loading

realsdx commented Feb 5, 2025 •

edited

Loading

coveralls commented Feb 9, 2025 •

edited

Loading