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

Please compilers #559

Merged
merged 5 commits into from
May 4, 2023
Merged

Please compilers #559

merged 5 commits into from
May 4, 2023

Conversation

cgzones
Copy link
Contributor

@cgzones cgzones commented Feb 28, 2023

No description provided.

Comparisson of different signedness can result in unexpected results due
to implicit conversions.

    ../network.c:81:34: warning: comparison of integer expressions of different signedness: ‘__u32’ {aka ‘unsigned int’} and ‘int’ [-Wsign-compare]
       81 |           if (rheader->nlmsg_seq != seq_nr)
          |                                  ^~
    ../network.c:83:34: warning: comparison of integer expressions of different signedness: ‘__u32’ {aka ‘unsigned int’} and ‘__pid_t’ {aka ‘int’} [-Wsign-compare]
      83 |           if (rheader->nlmsg_pid != getpid ())
          |                                  ^~

    ../bind-mount.c:268:17: warning: comparison of integer expressions of different signedness: ‘int’ and ‘unsigned int’ [-Wsign-compare]
      268 |       assert (i < n_lines);
          |                 ^
    ../bind-mount.c:309:13: warning: comparison of integer expressions of different signedness: ‘int’ and ‘unsigned int’ [-Wsign-compare]
      309 |   assert (i == n_lines);
          |             ^~
    ../bind-mount.c:318:17: warning: comparison of integer expressions of different signedness: ‘int’ and ‘unsigned int’ [-Wsign-compare]
      318 |   for (i = 0; i < n_lines; i++)
          |                 ^
    ../bind-mount.c:321:17: warning: comparison of integer expressions of different signedness: ‘int’ and ‘unsigned int’ [-Wsign-compare]
      321 |   for (i = 0; i < n_lines; i++)
          |                 ^

    ../utils.c:818:19: warning: comparison of integer expressions of different signedness: ‘size_t’ {aka ‘long unsigned int’} and ‘ssize_t’ {aka ‘long int’} [-Wsign-compare]
      818 |   while (size - 2 < n);
          |                   ^

    ../bubblewrap.c:489:13: warning: comparison of integer expressions of different signedness: ‘int’ and ‘long unsigned int’ [-Wsign-compare]
      489 |   assert (j < sizeof(dont_close)/sizeof(*dont_close));
          |             ^
    ../bubblewrap.c:994:25: warning: comparison of integer expressions of different signedness: ‘int’ and ‘uid_t’ {aka ‘unsigned int’} [-Wsign-compare]
      994 |       if (setfsuid (-1) != real_uid)
          |                         ^~
    ../bubblewrap.c:1042:61: warning: comparison of integer expressions of different signedness: ‘ssize_t’ {aka ‘long int’} and ‘size_t’ {aka ‘long unsigned int’} [-Wsign-compare]
     1042 |       if (write (privileged_op_socket, buffer, buffer_size) != buffer_size)
          |                                                             ^~
    ../bubblewrap.c:1232:25: warning: comparison of integer expressions of different signedness: ‘int’ and ‘long unsigned int’ [-Wsign-compare]
     1232 |           for (i = 0; i < N_ELEMENTS (cover_proc_dirs); i++)
          |                         ^
    ../bubblewrap.c:1260:25: warning: comparison of integer expressions of different signedness: ‘int’ and ‘long unsigned int’ [-Wsign-compare]
     1260 |           for (i = 0; i < N_ELEMENTS (devnodes); i++)
          |                         ^
    ../bubblewrap.c:1272:25: warning: comparison of integer expressions of different signedness: ‘int’ and ‘long unsigned int’ [-Wsign-compare]
     1272 |           for (i = 0; i < N_ELEMENTS (stdionodes); i++)
          |                         ^
    ../bubblewrap.c: In function ‘read_priv_sec_op’:
    ../bubblewrap.c:1556:15: warning: comparison of integer expressions of different signedness: ‘ssize_t’ {aka ‘long int’} and ‘long unsigned int’ [-Wsign-compare]
     1556 |   if (rec_len < sizeof (PrivSepOp))
          |               ^
    ../bubblewrap.c:1626:28: warning: comparison of integer expressions of different signedness: ‘int’ and ‘uint32_t’ {aka ‘unsigned int’} [-Wsign-compare]
     1626 |   if (*total_parsed_argc_p > MAX_ARGS)
          |                            ^
    ../bubblewrap.c:1681:40: warning: comparison of integer expressions of different signedness: ‘int’ and ‘uint32_t’ {aka ‘unsigned int’} [-Wsign-compare]
     1681 |               if (*total_parsed_argc_p > MAX_ARGS)
          |                                        ^
    ../bubblewrap.c:2265:31: warning: comparison of integer expressions of different signedness: ‘uid_t’ {aka ‘unsigned int’} and ‘int’ [-Wsign-compare]
     2265 |           if (opt_sandbox_uid != -1)
          |                               ^~
    ../bubblewrap.c:2285:31: warning: comparison of integer expressions of different signedness: ‘gid_t’ {aka ‘unsigned int’} and ‘int’ [-Wsign-compare]
     2285 |           if (opt_sandbox_gid != -1)
          |                               ^~
    ../bubblewrap.c:2678:23: warning: comparison of integer expressions of different signedness: ‘uid_t’ {aka ‘unsigned int’} and ‘int’ [-Wsign-compare]
     2678 |   if (opt_sandbox_uid == -1)
          |                       ^~
    ../bubblewrap.c:2680:23: warning: comparison of integer expressions of different signedness: ‘gid_t’ {aka ‘unsigned int’} and ‘int’ [-Wsign-compare]
     2680 |   if (opt_sandbox_gid == -1)
          |                       ^~

Signed-off-by: Christian Göttsche <[email protected]>
Found by running under pedantic UBSAN:

    ../bubblewrap.c:968:21: runtime error: implicit conversion from type 'int' of value -1 (32-bit, signed) to type 'uid_t' (aka 'unsigned int') changed the value to 4294967295 (32-bit, unsigned)
    ../bubblewrap.c:1210:28: runtime error: implicit conversion from type 'int' of value -41 (32-bit, signed) to type 'unsigned int' changed the value to 4294967255 (32-bit, unsigned)
    ../bubblewrap.c:1215:28: runtime error: implicit conversion from type 'int' of value -41 (32-bit, signed) to type 'unsigned int' changed the value to 4294967255 (32-bit, unsigned)

Signed-off-by: Christian Göttsche <[email protected]>
Signed-off-by: Christian Göttsche <[email protected]>
The parameter mode only usage is it being passed to ensure_dir(), which
takes it as mode_t.

Signed-off-by: Christian Göttsche <[email protected]>
Signed-off-by: Christian Göttsche <[email protected]>
@smcv smcv merged commit d73a78f into containers:main May 4, 2023
@cgzones cgzones deleted the compiler branch May 5, 2023 14:19
smcv added a commit to smcv/flatpak that referenced this pull request Mar 27, 2024
* `--symlink` is now idempotent, meaning it succeeds if the
  symlink already exists and already has the desired target
  (containers/bubblewrap#549, flatpak#2387,
  flatpak#3477, flatpak#5255)
* Report a better error message if `mount(2)` fails with `ENOSPC`
  (containers/bubblewrap#615, ValveSoftware/steam-runtime#637)
* Fix a double-close on error reading from `--args`, `--seccomp` or
  `--add-seccomp-fd` argument (containers/bubblewrap#558)
* Improve memory allocation behaviour
  (containers/bubblewrap#556, containers/bubblewrap#624)
* Silence various compiler warnings (containers/bubblewrap#559)

Resolves: flatpak#2387
Resolves: flatpak#3477
Resolves: flatpak#5255
Signed-off-by: Simon McVittie <[email protected]>
smcv added a commit to flatpak/flatpak that referenced this pull request Mar 27, 2024
* `--symlink` is now idempotent, meaning it succeeds if the
  symlink already exists and already has the desired target
  (containers/bubblewrap#549, #2387,
  #3477, #5255)
* Report a better error message if `mount(2)` fails with `ENOSPC`
  (containers/bubblewrap#615, ValveSoftware/steam-runtime#637)
* Fix a double-close on error reading from `--args`, `--seccomp` or
  `--add-seccomp-fd` argument (containers/bubblewrap#558)
* Improve memory allocation behaviour
  (containers/bubblewrap#556, containers/bubblewrap#624)
* Silence various compiler warnings (containers/bubblewrap#559)

Resolves: #2387
Resolves: #3477
Resolves: #5255
Signed-off-by: Simon McVittie <[email protected]>
GeorgesStavracas pushed a commit to GeorgesStavracas/flatpak that referenced this pull request Apr 26, 2024
* `--symlink` is now idempotent, meaning it succeeds if the
  symlink already exists and already has the desired target
  (containers/bubblewrap#549, flatpak#2387,
  flatpak#3477, flatpak#5255)
* Report a better error message if `mount(2)` fails with `ENOSPC`
  (containers/bubblewrap#615, ValveSoftware/steam-runtime#637)
* Fix a double-close on error reading from `--args`, `--seccomp` or
  `--add-seccomp-fd` argument (containers/bubblewrap#558)
* Improve memory allocation behaviour
  (containers/bubblewrap#556, containers/bubblewrap#624)
* Silence various compiler warnings (containers/bubblewrap#559)

Resolves: flatpak#2387
Resolves: flatpak#3477
Resolves: flatpak#5255
Signed-off-by: Simon McVittie <[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.

2 participants