All of lore.kernel.org
 help / color / mirror / Atom feed
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.
Date: Fri, 30 Sep 2022 09:20:42 +0200	[thread overview]
Message-ID: <20220930072042.dwakvbnefctk2jyd@fedora> (raw)
In-Reply-To: <470ea2ee-54fd-3dd5-2500-36fb82665c11@huawei.com>

On Fri, Sep 30, 2022 at 09:42:52AM +0800, zhanchengbin wrote:
> 
> 
> On 2022/9/29 19:28, Lukas Czerner wrote:
> > 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
> 
> Yes, you're right, I think we can fix it in this way.
> 
> diff --git a/misc/fsck.c b/misc/fsck.c
> index 1f6ec7d9..91edbf17 100644
> --- a/misc/fsck.c
> +++ b/misc/fsck.c
> @@ -547,6 +547,8 @@ static int kill_all(int signum)
>         for (inst = instance_list; inst; inst = inst->next) {
>                 if (inst->flags & FLAG_DONE)
>                         continue;
> +               if (inst->pid == -1)
> +                       continue;

Yeah, that works as well although I find the "if (noexecute)" condition
more obvious. We can do both. Also rather than checking for -1 we can
check for <= 0 since anything other than real pid at this point is a bug.

Feel free to send a proper patch.

Thanks!
-Lukas

>                 kill(inst->pid, signum);
>                 n++;
>         }
> > 
> > 
> > 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;
> regards,
> Zhan Chengbin
> 


  reply	other threads:[~2022-09-30  7:20 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-09-29  7:39 [bug report] misc/fsck.c: Processes may kill other processes zhanchengbin
2022-09-29 11:28 ` Lukas Czerner
2022-09-30  1:42   ` zhanchengbin
2022-09-30  7:20     ` Lukas Czerner [this message]
2022-10-04 22:18       ` Darrick J. Wong
2022-10-10  8:09       ` Karel Zak
2022-10-10  9:26         ` zhanchengbin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20220930072042.dwakvbnefctk2jyd@fedora \
    --to=lczerner@redhat.com \
    --cc=linfeilong@huawei.com \
    --cc=linux-ext4@vger.kernel.org \
    --cc=liuzhiqiang26@huawei.com \
    --cc=tytso@mit.edu \
    --cc=zhanchengbin1@huawei.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.