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

Annotate libselinux functions #357

Open
wants to merge 9 commits into
base: master
Choose a base branch
from

Conversation

cgzones
Copy link
Contributor

@cgzones cgzones commented Jun 16, 2022

No description provided.

@cgzones
Copy link
Contributor Author

cgzones commented Jun 16, 2022

systemd build warning:

../src/shared/selinux-util.c:558:9: warning: ignoring return value of function declared with 'warn_unused_result' attribute [-Wunused-result]
        setfscreatecon_raw(NULL);
        ^~~~~~~~~~~~~~~~~~ ~~~~
../src/shared/selinux-util.c:585:9: warning: ignoring return value of function declared with 'warn_unused_result' attribute [-Wunused-result]
        setsockcreatecon_raw(NULL);
        ^~~~~~~~~~~~~~~~~~~~ ~~~~

libselinux is used widely, in object managers, like systemd or dbus, and
essential utilities, like coreutils or package managers.

Help compilers and static analyzers to find suspicious usages of
interfaces of libselinux by annotating them with function attributes.
This includes potentially passing NULL to non-NULL parameters, no error
handling by ignoring return values.

Function attributes are GNU extensions and supported by GCC[1] and
Clang[2].

[1]: https://gcc.gnu.org/onlinedocs/gcc/Common-Function-Attributes.html
[2]: https://clang.llvm.org/docs/AttributeReference.html#function-attributes

Signed-off-by: Christian Göttsche <[email protected]>
Annotate interfaces of the matchpathcon family and
security_compute_user(3) and security_compute_user_raw(3) as deprecated.

Signed-off-by: Christian Göttsche <[email protected]>
The first parameter of avc_open(3) is a read-only array of options.

Signed-off-by: Christian Göttsche <[email protected]>
    mcstrans.c: In function ‘new_context_str’:
    mcstrans.c:926:9: error: ignoring return value of ‘context_range_set’ declared with attribute ‘warn_unused_result’ [-Werror=unused-result]
      926 |         context_range_set(con, range);
          |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~

    mcscolor.c: In function ‘init_colors’:
    mcscolor.c:252:9: error: ignoring return value of ‘getcon’ declared with attribute ‘warn_unused_result’ [-Werror=unused-result]
      252 |         getcon(&my_context);
          |         ^~~~~~~~~~~~~~~~~~~

Signed-off-by: Christian Göttsche <[email protected]>
Tell GCC, see [1], to actually no issue warnings about explicitly
ignored return values.

Also explicitly ignored return values in cleanup handlers.

    togglesebool.c: In function ‘rollback’:
    togglesebool.c:18:17: error: ignoring return value of ‘security_set_boolean’ declared with attribute ‘warn_unused_result’ [-Werror=unused-result]
       18 |                 security_set_boolean(argv[i],
          |                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
       19 |                                      security_get_boolean_active(argv[i]));
          |                                      ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

    load_policy.c: In function ‘selinux_init_load_policy’:
    load_policy.c:329:17: error: ‘security_disable’ is deprecated: SELinux runtime disable is deprecated [-Werror=deprecated-declarations]
      329 |                 rc = security_disable();
          |                 ^~

    booleans.c: In function ‘rollback’:
    booleans.c:332:17: error: ignoring return value of ‘security_set_boolean’ declared with attribute ‘warn_unused_result’ [-Werror=unused-result]
      332 |                 security_set_boolean(boollist[i].name,
          |                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
      333 |                                      security_get_boolean_active(boollist[i].
          |                                      ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
      334 |                                                                  name));
          |                                                                  ~~~~~~

    checkAccess.c: In function ‘selinux_check_access’:
    checkAccess.c:42:16: error: ignoring return value of ‘selinux_status_updated’ declared with attribute ‘warn_unused_result’ [-Werror=unused-result]
       42 |         (void) selinux_status_updated();
          |                ^~~~~~~~~~~~~~~~~~~~~~~~

    avc.c: In function ‘avc_has_perm_noaudit’:
    avc.c:761:24: error: ignoring return value of ‘selinux_status_updated’ declared with attribute ‘warn_unused_result’ [-Werror=unused-result]
      761 |                 (void) selinux_status_updated();
          |                        ^~~~~~~~~~~~~~~~~~~~~~~~

[1]: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66425

Signed-off-by: Christian Göttsche <[email protected]>
Ignore internal use of deprecated interfaces within deprecated
interfaces.

    compute_user.c: In function ‘security_compute_user’:
    compute_user.c:93:9: error: ‘security_compute_user_raw’ is deprecated: Use get_ordered_context_list(3) family [-Werror=deprecated-declarations]
       93 |         ret = security_compute_user_raw(rscon, user, con);
          |         ^~~
    compute_user.c:13:5: note: declared here
       13 | int security_compute_user_raw(const char * scon,
          |     ^~~~~~~~~~~~~~~~~~~~~~~~~

Signed-off-by: Christian Göttsche <[email protected]>
    secon.c: In function ‘disp_con’:
    secon.c:634:9: error: ignoring return value of ‘selinux_raw_to_trans_context’ declared with attribute ‘warn_unused_result’ [-Werror=unused-result]
      634 |         selinux_raw_to_trans_context(scon_raw, &scon_trans);
          |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Signed-off-by: Christian Göttsche <[email protected]>
    watch.c: In function ‘watch_list_add’:
    watch.c:74:25: error: ignoring return value of ‘selinux_restorecon’ declared with attribute ‘warn_unused_result’ [-Werror=unused-result]
       74 |                         selinux_restorecon(globbuf.gl_pathv[i],
          |                         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
       75 |                                            r_opts.restorecon_flags);
          |                                            ~~~~~~~~~~~~~~~~~~~~~~~~
    watch.c: In function ‘watch_list_find’:
    watch.c:141:33: error: ignoring return value of ‘selinux_restorecon’ declared with attribute ‘warn_unused_result’ [-Werror=unused-result]
      141 |                                 selinux_restorecon(path,
          |                                 ^~~~~~~~~~~~~~~~~~~~~~~~
      142 |                                                    r_opts.restorecon_flags);
          |                                                    ~~~~~~~~~~~~~~~~~~~~~~~~

Signed-off-by: Christian Göttsche <[email protected]>
As restorecond does not use the matchpathcon family but the selabel one,
via selinux_restorecon(3), drop the last unneeded call.

Signed-off-by: Christian Göttsche <[email protected]>
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.

None yet

1 participant