lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite for Android: free password hash cracker in your pocket
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAKwvOdm9HuXyw64ECVVhROxc3R9LL-FT3KbO_e5XzfE4=Q-UoA@mail.gmail.com>
Date:   Thu, 18 Nov 2021 10:41:23 -0800
From:   Nick Desaulniers <ndesaulniers@...gle.com>
To:     Anders Roxell <anders.roxell@...aro.org>
Cc:     shuah@...nel.org, fenghua.yu@...el.com, reinette.chatre@...el.com,
        john.stultz@...aro.org, tglx@...utronix.de,
        akpm@...ux-foundation.org, nathan@...nel.org,
        linux-kernel@...r.kernel.org, linux-kselftest@...r.kernel.org,
        linux-mm@...ck.org, llvm@...ts.linux.dev, christian@...uner.io
Subject: Re: [PATCH v3 1/2] selftests: kselftest.h: mark functions with 'noreturn'

On Thu, Nov 18, 2021 at 10:40 AM Nick Desaulniers
<ndesaulniers@...gle.com> wrote:
>
> On Thu, Nov 18, 2021 at 1:58 AM Anders Roxell <anders.roxell@...aro.org> wrote:
> >
> > When building kselftests/capabilities the following warning shows up:
> >
> > clang -O2 -g -std=gnu99 -Wall    test_execve.c -lcap-ng -lrt -ldl -o test_execve
> > test_execve.c:121:13: warning: variable 'have_outer_privilege' is used uninitialized whenever 'if' condition is false [-Wsometimes-uninitialized]
> >         } else if (unshare(CLONE_NEWUSER | CLONE_NEWNS) == 0) {
> >                    ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > test_execve.c:136:9: note: uninitialized use occurs here
> >         return have_outer_privilege;
> >                ^~~~~~~~~~~~~~~~~~~~
> > test_execve.c:121:9: note: remove the 'if' if its condition is always true
> >         } else if (unshare(CLONE_NEWUSER | CLONE_NEWNS) == 0) {
> >                ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > test_execve.c:94:27: note: initialize the variable 'have_outer_privilege' to silence this warning
> >         bool have_outer_privilege;
> >                                  ^
> >                                   = false
> >
> > Rework so all the ksft_exit_*() functions have attribue
> > '__attribute__((noreturn))' so the compiler knows that there wont be
> > any return from the function. That said, without
> > '__attribute__((noreturn))' the compiler warns about the above issue
> > since it thinks that it will get back from the ksft_exit_skip()
> > function, which it wont.
> > Cleaning up the callers that rely on ksft_exit_*() return code, since
> > the functions ksft_exit_*() have never returned anything.
> >
> > Signed-off-by: Anders Roxell <anders.roxell@...aro.org>
> > Reviewed-by: Nick Desaulniers <ndesaulniers@...gle.com>
> > ---
> >  tools/testing/selftests/clone3/clone3.c              |  4 +++-
> >  .../testing/selftests/clone3/clone3_clear_sighand.c  |  2 +-
> >  tools/testing/selftests/clone3/clone3_set_tid.c      |  4 +++-
> >  tools/testing/selftests/ipc/msgque.c                 | 10 +++++-----
> >  tools/testing/selftests/kcmp/kcmp_test.c             |  2 +-
> >  tools/testing/selftests/kselftest.h                  | 12 ++++++------
> >  .../membarrier/membarrier_test_multi_thread.c        |  2 +-
> >  .../membarrier/membarrier_test_single_thread.c       |  2 +-
> >  tools/testing/selftests/pidfd/pidfd_fdinfo_test.c    |  2 +-
> >  tools/testing/selftests/pidfd/pidfd_open_test.c      |  4 +++-
> >  tools/testing/selftests/pidfd/pidfd_poll_test.c      |  2 +-
> >  tools/testing/selftests/pidfd/pidfd_test.c           |  2 +-
> >  tools/testing/selftests/resctrl/resctrl_tests.c      |  6 +++---
> >  tools/testing/selftests/sync/sync_test.c             |  2 +-
> >  tools/testing/selftests/timers/adjtick.c             |  4 ++--
> >  tools/testing/selftests/timers/alarmtimer-suspend.c  |  4 ++--
> >  tools/testing/selftests/timers/change_skew.c         |  4 ++--
> >  tools/testing/selftests/timers/clocksource-switch.c  |  4 ++--
> >  tools/testing/selftests/timers/freq-step.c           |  4 ++--
> >  tools/testing/selftests/timers/inconsistency-check.c |  4 ++--
> >  tools/testing/selftests/timers/leap-a-day.c          | 10 +++++-----
> >  tools/testing/selftests/timers/leapcrash.c           |  4 ++--
> >  tools/testing/selftests/timers/mqueue-lat.c          |  4 ++--
> >  tools/testing/selftests/timers/nanosleep.c           |  4 ++--
> >  tools/testing/selftests/timers/nsleep-lat.c          |  4 ++--
> >  tools/testing/selftests/timers/posix_timers.c        | 12 ++++++------
> >  tools/testing/selftests/timers/raw_skew.c            |  6 +++---
> >  tools/testing/selftests/timers/set-2038.c            |  4 ++--
> >  tools/testing/selftests/timers/set-tai.c             |  4 ++--
> >  tools/testing/selftests/timers/set-timer-lat.c       |  4 ++--
> >  tools/testing/selftests/timers/set-tz.c              |  4 ++--
> >  tools/testing/selftests/timers/skew_consistency.c    |  4 ++--
> >  tools/testing/selftests/timers/threadtest.c          |  2 +-
> >  tools/testing/selftests/timers/valid-adjtimex.c      |  6 +++---
> >  tools/testing/selftests/vm/madv_populate.c           |  2 +-
> >  35 files changed, 80 insertions(+), 74 deletions(-)
> >
> > diff --git a/tools/testing/selftests/clone3/clone3.c b/tools/testing/selftests/clone3/clone3.c
> > index 42be3b925830..ede5da0c67b4 100644
> > --- a/tools/testing/selftests/clone3/clone3.c
> > +++ b/tools/testing/selftests/clone3/clone3.c
> > @@ -191,5 +191,7 @@ int main(int argc, char *argv[])
> >         test_clone3(CLONE_NEWPID, getpagesize() + 8, -E2BIG,
> >                         CLONE3_ARGS_NO_TEST);
> >
> > -       return !ksft_get_fail_cnt() ? ksft_exit_pass() : ksft_exit_fail();
> > +       if (ksft_get_fail_cnt())
> > +               ksft_exit_fail();
> > +       ksft_exit_pass();
> >  }
> > diff --git a/tools/testing/selftests/clone3/clone3_clear_sighand.c b/tools/testing/selftests/clone3/clone3_clear_sighand.c
> > index 47a8c0fc3676..dcd9448eaeec 100644
> > --- a/tools/testing/selftests/clone3/clone3_clear_sighand.c
> > +++ b/tools/testing/selftests/clone3/clone3_clear_sighand.c
> > @@ -124,5 +124,5 @@ int main(int argc, char **argv)
> >
> >         test_clone3_clear_sighand();
> >
> > -       return ksft_exit_pass();
> > +       ksft_exit_pass();
> >  }
> > diff --git a/tools/testing/selftests/clone3/clone3_set_tid.c b/tools/testing/selftests/clone3/clone3_set_tid.c
> > index 0229e9ebb995..a755fcd3af89 100644
> > --- a/tools/testing/selftests/clone3/clone3_set_tid.c
> > +++ b/tools/testing/selftests/clone3/clone3_set_tid.c
> > @@ -393,5 +393,7 @@ int main(int argc, char *argv[])
> >  out:
> >         ret = 0;
> >
> > -       return !ret ? ksft_exit_pass() : ksft_exit_fail();
> > +       if (ret)
> > +               ksft_exit_fail();
> > +       ksft_exit_pass();
>
> err...did you update clone3_set_tid as Shuah requested in v3?
> Otherwise what changed from v2 -> v3?

Ah, this was part of a series. Sorry I missed that; gmail keeps the
original subject line when --in-reply-to is used even when the subject
changes.

-- 
Thanks,
~Nick Desaulniers

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ