| Message ID | 2c8f3b3a-b6d1-9b8b-27c7-2df51236fe8c@huawei.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [v2] misc/fsck.c: Processes may kill other processes. | expand |
On Mon, Oct 10, 2022 at 04:56:58PM +0800, zhanchengbin wrote: > 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. Looks good thanks. > > Signed-off-by: zhanchengbin <zhanchengbin1@huawei.com> > Signed-off-by: Lukas Czerner <lczerner@redhat.com> ^^^ You can remove my Sob, but you can add Reviewed-by: Lukas Czerner <lczerner@redhat.com> Thanks! -Lukas > --- > V1->V2: > Anything <= 0 is a bug and can have unexpected consequences if > we actually call the kill(). So change inst->pid==-1 to inst->pid<=0. > > misc/fsck.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/misc/fsck.c b/misc/fsck.c > index 4efe10ec..c56d1b00 100644 > --- a/misc/fsck.c > +++ b/misc/fsck.c > @@ -546,6 +546,8 @@ static int kill_all(int signum) > for (inst = instance_list; inst; inst = inst->next) { > if (inst->flags & FLAG_DONE) > continue; > + if (inst->pid <= 0) > + continue; > kill(inst->pid, signum); > n++; > } > -- > 2.27.0 >
On Mon, Oct 10, 2022 at 04:56:58PM +0800, zhanchengbin wrote: > 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. > > Signed-off-by: zhanchengbin <zhanchengbin1@huawei.com> > Signed-off-by: Lukas Czerner <lczerner@redhat.com> Looks good to me, Reviewed-by: Darrick J. Wong <djwong@kernel.org> Ted: Perhaps it's time to retire fsck.c in the e2fsprogs distribution? --D > --- > V1->V2: > Anything <= 0 is a bug and can have unexpected consequences if > we actually call the kill(). So change inst->pid==-1 to inst->pid<=0. > > misc/fsck.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/misc/fsck.c b/misc/fsck.c > index 4efe10ec..c56d1b00 100644 > --- a/misc/fsck.c > +++ b/misc/fsck.c > @@ -546,6 +546,8 @@ static int kill_all(int signum) > for (inst = instance_list; inst; inst = inst->next) { > if (inst->flags & FLAG_DONE) > continue; > + if (inst->pid <= 0) > + continue; > kill(inst->pid, signum); > n++; > } > -- > 2.27.0 >
On 2022/10/10 16:56, zhanchengbin wrote: > 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. > Looks good to me, Reviewed-by: Zhiqiang Liu <liuzhiqiang26@huawei.com> > Signed-off-by: zhanchengbin <zhanchengbin1@huawei.com> > Signed-off-by: Lukas Czerner <lczerner@redhat.com> > --- > V1->V2: > Anything <= 0 is a bug and can have unexpected consequences if > we actually call the kill(). So change inst->pid==-1 to inst->pid<=0. > > misc/fsck.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/misc/fsck.c b/misc/fsck.c > index 4efe10ec..c56d1b00 100644 > --- a/misc/fsck.c > +++ b/misc/fsck.c > @@ -546,6 +546,8 @@ static int kill_all(int signum) > for (inst = instance_list; inst; inst = inst->next) { > if (inst->flags & FLAG_DONE) > continue; > + if (inst->pid <= 0) > + continue; > kill(inst->pid, signum); > n++; > }
friendly ping, thanks. On 2022/10/10 16:56, zhanchengbin wrote: > 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. > > Signed-off-by: zhanchengbin <zhanchengbin1@huawei.com> > Signed-off-by: Lukas Czerner <lczerner@redhat.com> > --- > V1->V2: > Anything <= 0 is a bug and can have unexpected consequences if > we actually call the kill(). So change inst->pid==-1 to inst->pid<=0. > > misc/fsck.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/misc/fsck.c b/misc/fsck.c > index 4efe10ec..c56d1b00 100644 > --- a/misc/fsck.c > +++ b/misc/fsck.c > @@ -546,6 +546,8 @@ static int kill_all(int signum) > for (inst = instance_list; inst; inst = inst->next) { > if (inst->flags & FLAG_DONE) > continue; > + if (inst->pid <= 0) > + continue; > kill(inst->pid, signum); > n++; > }
ping... On 2022/10/10 16:56, zhanchengbin wrote: > 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. > > Signed-off-by: zhanchengbin <zhanchengbin1@huawei.com> > Signed-off-by: Lukas Czerner <lczerner@redhat.com> > --- > V1->V2: > Anything <= 0 is a bug and can have unexpected consequences if > we actually call the kill(). So change inst->pid==-1 to inst->pid<=0. > > misc/fsck.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/misc/fsck.c b/misc/fsck.c > index 4efe10ec..c56d1b00 100644 > --- a/misc/fsck.c > +++ b/misc/fsck.c > @@ -546,6 +546,8 @@ static int kill_all(int signum) > for (inst = instance_list; inst; inst = inst->next) { > if (inst->flags & FLAG_DONE) > continue; > + if (inst->pid <= 0) > + continue; > kill(inst->pid, signum); > n++; > }
On Mon, 10 Oct 2022 16:56:58 +0800, zhanchengbin wrote: > 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. > > [...] Applied, thanks! [1/1] misc/fsck.c: Processes may kill other processes. commit: d08ef863feae14e4710bf2026404e6c6e06db2be Best regards,
diff --git a/misc/fsck.c b/misc/fsck.c index 4efe10ec..c56d1b00 100644 --- a/misc/fsck.c +++ b/misc/fsck.c @@ -546,6 +546,8 @@ static int kill_all(int signum) for (inst = instance_list; inst; inst = inst->next) { if (inst->flags & FLAG_DONE) continue; + if (inst->pid <= 0) + continue; kill(inst->pid, signum); n++; }