linux-kernel - RE: [PATCH v4 4/5] selftests/resctrl: Cleanup properly when an error occurs in CAT test

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: <OSZPR01MB632837AC9133DFBD2061637D8BE69@OSZPR01MB6328.jpnprd01.prod.outlook.com>
Date:   Fri, 16 Dec 2022 08:20:10 +0000
From:   "Shaopeng Tan (Fujitsu)" <tan.shaopeng@...itsu.com>
To:     'Reinette Chatre' <reinette.chatre@...el.com>,
        Fenghua Yu <fenghua.yu@...el.com>,
        Shuah Khan <shuah@...nel.org>
CC:     "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "linux-kselftest@...r.kernel.org" <linux-kselftest@...r.kernel.org>,
        Shuah Khan <skhan@...uxfoundation.org>
Subject: RE: [PATCH v4 4/5] selftests/resctrl: Cleanup properly when an error
 occurs in CAT test

Hi Reinette

> On 12/1/2022 12:20 AM, Shaopeng Tan (Fujitsu) wrote:
> > Hi Reinette,
> >
> >> On 11/30/2022 12:32 AM, Shaopeng Tan (Fujitsu) wrote:
> >>
> >>> Removing ctrl_handler() is only part of the fix in the next version(v5).
> >>> All fixes as follows.
> >>>
> >>> --- a/tools/testing/selftests/resctrl/cat_test.c
> >>> +++ b/tools/testing/selftests/resctrl/cat_test.c
> >>> @@ -98,12 +98,17 @@ void cat_test_cleanup(void)
> >>>         remove(RESULT_FILE_NAME2);
> >>>  }
> >>>
> >>> +static void ctrlc_handler_child(int signum, siginfo_t *info, void
> >>> +*ptr) {
> >>> +       exit(EXIT_SUCCESS);
> >>> +}
> >>> +
> >>
> >> Could you please elaborate why this is necessary?
> >
> > If enter "ctrl-c" when running "resctrl_tests -t cat", SIGINT will be
> > sent to all processes (parent&child).
> >
> > At this time, the child process receives a SIGINT signal, but does not take any
> action.
> > In this case the parent process may not call ctrlc_handler() as expected.
> 
> Apologies, but I am not able to follow. My understanding is that the ideal in
> working an failing case is for the parent to kill the child.
> Could you please elaborate why the ctrlc_handler() may not be called?

Apologies for the late replay.

The problem is that at the time of running CAT test, 
previous ctrlc_handler from MBM/MBA/CMT test will be inherited to child process.

Let me explain in detail:
In resctrl_tests,the default run order of the tests is MBM->MBA->CMT->CAT.
When running MBM, MBA, CMT, signal handler(ctrlc_handler) was set to the parent process.
After these tests, when fork() is executed in CAT, 
the signal handler set by MBM/MBA/CMT is inherited by the parent&child process of CAT.
At this time, if "ctrl+c" SIGINT is sent to parent&child process,
according to the inherited signal handler,
the child process may kill parent process before parent process kills child process.
Therefore, when running all tests(MBM->MBA->CMT->CAT),
signal handler of child process need to be overridden in CAT.

Also, when running CAT test only,
since there are no signal handler that can be inherited from other tests,
signal handler of parent process need to be set.

> > Therefore, ctrlc_handler_child() is necessary.
> >
> > It may be better to ignore the signal, then code can be simple as follows.
> > ----
> >         if (bm_pid == 0) {
> >                 param.mask = l_mask_1;
> >                 strcpy(param.ctrlgrp, "c1");
> >                 strcpy(param.mongrp, "m1");
> >                 param.span = cache_size * n / count_of_bits;
> >                 strcpy(param.filename, RESULT_FILE_NAME1);
> >                 param.num_of_runs = 0;
> >                 param.cpu_no = sibling_cpu_no;
> >                 /* Ignore the signal,and wait to be cleaned up by the parent
> process */
> >                 sigfillset(&sigact.sa_mask);
> >                 sigact.sa_handler = SIG_IGN;
> >                 //sigact.sa_sigaction = ctrlc_handler_child;  //delete
> >                 if (sigaction(SIGINT, &sigact, NULL) ||
> >                     sigaction(SIGTERM, &sigact, NULL) ||
> >                     sigaction(SIGHUP, &sigact, NULL))
> >                         perror("# sigaction");
> >         } else {

Best regards,
Shaopeng