seccomp / libseccomp

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

BUG: seccomp_rule_add.3: add -EACCES return value #339

Closed kolyshkin closed 2 years ago

kolyshkin commented 2 years ago

The -EACCES return value from seccomp_rule_add and friends was added by commit 83989be02 (included into 2.5.0), which tells that this is "part of our ... API promise", so it needs to be documented accordingly. Add it.

The discussion leading to this PR is in https://github.com/seccomp/libseccomp-golang/pull/74, however we did not came to the agreement whether -EACCES is the best choice.

Fixes: 83989be02

coveralls commented 2 years ago

Coverage Status

Coverage remained the same at 89.53% when pulling fba2a669afc51ce76ddc7a7c6a88895ff8cc336c on kolyshkin:eacces-doc into 2457dec1a90101d720e89e8027376742e2f3c327 on seccomp:main.

pcmoore commented 2 years ago

Thanks @kolyshkin.

I'm still wondering if we are better off returning -EACCES in this case or changing it to -EEXIST, but I guess sticking with -EACCES is probably for the best as that is what we have done starting with v2.5.x and the beginning of our "API promise".

My next question is if we want to be a specific as this patch when explaining the reason for the error code, "The action argument equals the default action of the filter" is pretty specific. Perhaps consider changing it to: "The rule conflicts with the default filter settings"?

Thoughts?

kolyshkin commented 2 years ago

My next question is if we want to be a specific as this patch when explaining the reason for the error code, "The action argument equals the default action of the filter" is pretty specific. Perhaps consider changing it to: "The rule conflicts with the default filter settings"?

Thoughts?

What about something like "The rule conflicts with the filter (for example, the rule action equals the default action of the filter)"? This way we are specific (and clear) but still leave room for other similar errors.

drakenclimber commented 2 years ago

My next question is if we want to be a specific as this patch when explaining the reason for the error code, "The action argument equals the default action of the filter" is pretty specific. Perhaps consider changing it to: "The rule conflicts with the default filter settings"? Thoughts?

What about something like "The rule conflicts with the filter (for example, the rule action equals the default action of the filter)"? This way we are specific (and clear) but still leave room for other similar errors.

I'm fine with either of these.

kolyshkin commented 2 years ago

Updated to

       -EACCCES
              The rule conflicts with the filter (for example, the rule action
              equals the default action of the filter).

and fix a typesetting error I made earlier (missing .TP).

pcmoore commented 2 years ago

Since @drakenclimber is okay with either approach I went ahead and merged this via 50da6c1c61c1237cc3af2240b294af66de505018, thanks @kolyshkin!

pcmoore commented 2 years ago

Ported to the release-2.5 branch via 5535f144901267ae768a17969b989e2edee7b0a2.