linux-kernel - RE: [PATCH v6 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: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <OSZPR01MB6328AE8F77449F7D6C7DE8958BD89@OSZPR01MB6328.jpnprd01.prod.outlook.com>
Date:   Wed, 8 Feb 2023 02:39:27 +0000
From:   "Shaopeng Tan (Fujitsu)" <tan.shaopeng@...itsu.com>
To:     'Ilpo Järvinen' 
        <ilpo.jarvinen@...ux.intel.com>
CC:     Fenghua Yu <fenghua.yu@...el.com>,
        Reinette Chatre <reinette.chatre@...el.com>,
        Shuah Khan <shuah@...nel.org>,
        LKML <linux-kernel@...r.kernel.org>,
        "linux-kselftest@...r.kernel.org" <linux-kselftest@...r.kernel.org>
Subject: RE: [PATCH v6 4/5] selftests/resctrl: Cleanup properly when an error
 occurs in CAT test

Hi Ilpo,

> On Tue, 31 Jan 2023, Shaopeng Tan wrote:
> 
> > After creating a child process with fork() in CAT test, if an error
> > occurs or a signal such as SIGINT is received, the parent process will
> > be terminated immediately, and therefor the child process will not be
> > killed and also resctrlfs is not unmounted.
> >
> > There is a signal handler registered in CMT/MBM/MBA tests, which kills
> > child process, unmount resctrlfs, cleanups result files, etc., if a
> > signal such as SIGINT is received.
> >
> > Commonize the signal handler registered for CMT/MBM/MBA tests and
> > reuse it in CAT too.
> >
> > To reuse the signal handler, make the child process in CAT wait to be
> > killed by parent process in any case (an error occurred or a signal
> > was received), and when killing child process use global bm_pid
> > instead of local bm_pid.
> >
> > Also, since the MBA/MBA/CMT/CAT are run in order, unregister the
> > signal handler at the end of each test so that the signal handler
> > cannot be inherited by other tests.
> >
> > Signed-off-by: Shaopeng Tan <tan.shaopeng@...fujitsu.com>
> > ---
> 
> >  	if (bm_pid == 0) {
> >  		/* Tell parent that child is ready */
> >  		close(pipefd[0]);
> >  		pipe_message = 1;
> >  		if (write(pipefd[1], &pipe_message, sizeof(pipe_message)) <
> > -		    sizeof(pipe_message)) {
> > -			close(pipefd[1]);
> > +		    sizeof(pipe_message))
> > +			/*
> > +			 * Just print the error message.
> > +			 * Let while(1) run and wait for itself to be killed.
> > +			 */
> >  			perror("# failed signaling parent process");
> 
> If the write error is ignored here, won't it just lead to parent hanging forever
> waiting for the child to send the message through the pipe which will never
> come?

If the write error is ignored here, the pipe will be closed by "close(pipefd[1]);" and child process will wait to be killed by "while(1)".
---
-			return errno;
-		}

 		close(pipefd[1]);
 		while (1)
---

If all file descriptors referring to the write end of a pipe have been closed, 
then an attempt to read(2) from the pipe will see end-of-file (read(2) will return 0).
Then, "perror("# failed reading from child process");" occurs.
---
        } else {
                /* Parent waits for child to be ready. */
                close(pipefd[1]);
                pipe_message = 0;
                while (pipe_message != 1) {
                        if (read(pipefd[0], &pipe_message,
                                 sizeof(pipe_message)) < sizeof(pipe_message)) {
                                perror("# failed reading from child process");
                                break;
                        }
                }
                close(pipefd[0]);
                kill(bm_pid, SIGKILL);
                signal_handler_unregister();
        }
---

Best regards,
Shaopeng TAN