search

seccomp / libseccomp

The main libseccomp repository
GNU Lesser General Public License v2.1
805 stars 171 forks source link

BUG: use old SECCOMP_IOCTL_NOTIF_ID_VALID number if necessary #306

Closed maxcrees closed 3 years ago

maxcrees commented 3 years ago

Kernel commit 47e33c05f9f07cac3de833e531bcac9ae052c7ca changed the public definition of SECCOMP_IOCTL_NOTIF_ID_VALID for correctness sake because it had the wrong direction (no current functional change). If libseccomp is built against kernel headers after this commit but is run on a kernel that was built prior to this commit, then the ioctl will always return EINVAL and thus seccomp_notify_id_valid will incorrectly return -ENOENT.

Copy the (now non-public) definition of the old ioctl number and try it if the ioctl with the number from the kernel headers fails with -1 EINVAL.

Signed-off-by: Max Rees maxcrees@me.com

Note: should I also update the value of SECCOMP_IOCTL_NOTIF_ID_VALID in the #ifndef SECCOMP_RET_USER_NOTIF in src/system.h while I'm at it?

maxcrees commented 3 years ago

Other notes: the reverse case is not true - compiling against old headers and running against a newer kernel will work fine because the newer kernel still supports the old ioctl number.

This behavior was discovered when running on Linux 5.4.5 after compiling against the 5.4.66 headers. This is because the commit was not backported until 5.4.59 (gregkh/linux@0f09c88f207c0b6a45454e770829a7aba814059e).

coveralls commented 3 years ago

Coverage Status

Coverage decreased (-0.06%) to 92.979% when pulling 0704f7d8628f4c5c4a44300fca0a221dec6445fd on sroracle:notif_id_valid_compat into 2ddf36ca5869dc886a93bb5ad8b7039b5156a03a on seccomp:master.

pcmoore commented 3 years ago

Thanks @sroracle, I've added this to the queue of things for v2.5.2 based on your description but I haven't looked too closely at it yet. We will get to it, but it's a busy time of year right now :)

drakenclimber commented 3 years ago

The change looks correct to me. As @pcmoore requested, using the standard 47e33c05f9f0 ("seccomp: Fix ioctl number for SECCOMP_IOCTL_NOTIF_ID_VALID") format in both the code comment and the git commit log would be a good cleanup. With those small changes, I think it's good to go.

Thanks for the patch, @sroracle.

Acked-by: Tom Hromatka <tom.hromatka@oracle.com>

maxcrees commented 3 years ago

V2: change comment and commit message wording.

maxcrees commented 3 years ago

V3: also update fallback definition of SECCOMP_IOCTL_NOTIF_ID_VALID to the new value.

pcmoore commented 3 years ago

Thanks @sroracle, I apologize for the delay. I've merged this via 83d7b022fa7ef8c24516cc668efc879e5398403f and b926e5af7295b04e8cf7ac4929731fed0f53a711.