Received-SPF: pass (google.com: domain of linux-ext4-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Thu, 29 Sep 2022 13:28:13 +0200
From:   Lukas Czerner <lczerner@redhat.com>
To:     zhanchengbin <zhanchengbin1@huawei.com>
Cc:     Theodore Ts'o <tytso@mit.edu>, linux-ext4@vger.kernel.org,
        liuzhiqiang26@huawei.com, linfeilong <linfeilong@huawei.com>
Subject: Re: [bug report] misc/fsck.c: Processes may kill other processes.
Message-ID: <20220929112813.6aqtktwaff2m7fh2@fedora>
References: <4ffe3143-fc53-7178-cf44-f3481eb96ae4@huawei.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <4ffe3143-fc53-7178-cf44-f3481eb96ae4@huawei.com>
Precedence: bulk

On Thu, Sep 29, 2022 at 03:39:57PM +0800, zhanchengbin wrote:
> Hi Tytso,
> I find a error in misc/fsck.c, if run the fsck -N command, processes
> don't execute, just show what would be done. However, the pid whose
> value is -1 is added to the instance_list list in the execute
> function,if the kill_all function is called later, kill(-1, signum)
> is executed, Signals are sent to all processes except the number one
> process and itself. Other processes will be killed if they use the
> default signal processing function.

Hi,

indeed we'd like to avoid killing the instance that was not ran because
of noexecute. Can you try the following patch?

Thanks!
-Lukas


diff --git a/misc/fsck.c b/misc/fsck.c
index 1f6ec7d9..8fae7730 100644
--- a/misc/fsck.c
+++ b/misc/fsck.c
@@ -497,9 +497,10 @@ static int execute(const char *type, const char *device, const char *mntpt,
 	}
 
 	/* Fork and execute the correct program. */
-	if (noexecute)
+	if (noexecute) {
 		pid = -1;
-	else if ((pid = fork()) < 0) {
+		inst->flags |= FLAG_DONE;
+	} else if ((pid = fork()) < 0) {
 		perror("fork");
 		free(inst);
 		return errno;
@@ -544,6 +545,9 @@ static int kill_all(int signum)
 	struct fsck_instance *inst;
 	int	n = 0;
 
+	if (noexecute)
+		return 0;
+
 	for (inst = instance_list; inst; inst = inst->next) {
 		if (inst->flags & FLAG_DONE)
 			continue;


> 
> execute:
>     if (noexecute)
>         pid = -1;
>     inst->pid = pid;
>     // Find the end of the list, so we add the instance on at the end.
> 
> kill_all:
>     for (inst = instance_list; inst; inst = inst->next) {
>         kill(inst->pid, signum);
>     }
> 
> misc/fsck.c:
> main:
>     ->PRS
>         case 'N':
>             noexecute++;
>     for (num_devices) {
>         if (cancel_requested) {
>             ->kill_all(SIGTERM);
>         }
>         ->fsck_device
>             ->execute
>     }
> 
> (gdb) bt
> #0  execute (type=<optimized out>, type@entry=0x412cd0 "ext4",
> device=0x412b00 "/root/a.img", mntpt=<optimized out>,
> interactive=interactive@entry=1) at fsck.c:523
> #1  0x0000000000404959 in fsck_device (fs=fs@entry=0x412ac0,
> interactive=interactive@entry=1) at fsck.c:727
> #2  0x0000000000402704 in main (argc=<optimized out>, argv=<optimized out>)
> at fsck.c:1333
> (gdb) p inst->pid
> $7 = -1
> 
> regards,
> Zhan Chengbin
>