Re: [PATCH] get_signal: simplify/cleanup the the usage of ksig->info

[Wen Yang]

On 2024/2/26 17:52, Oleg Nesterov wrote:
> OK, lets forget about "no need to initialize ksig->info" part for now.
>
> Yes, PF_USER_WORKER's never dereference ksig, but I agree this doesn't
> look clean without other cleanups. I'll probably return to this later
> when other pending problems with PF_USER_WORKER are fixed.
>
> Wen, thanks again for your review.
>
> Oleg.


Thank you very much, Oleg,

We greatly appreciate your comments and the discussion has given us a 
precious opportunity to understand the linux signal.

--
Best wishes,
Wen


> On 02/23, Oleg Nesterov wrote:
> > On 02/23, Oleg Nesterov wrote:
> > > On 02/23, Oleg Nesterov wrote:
> > > > On 02/23, Wen Yang wrote:
> > > > > On 2024/2/23 18:50, Oleg Nesterov wrote:
> > > > > > +			/*
> > > > > > +			 * implies do_group_exit(), no need to initialize
> > > > > > +			 * ksig->info
> > > > > > +			 */
> > > > > >   			goto fatal;
> > > > >
> > > > > There is little question:
> > > > > If the this conditions is met:
> > > > >     current->flags & PF_USER_WORKER
> > > > > It may execute “goto out“ instead of do_group_exit().
> > > >
> > > > Yes, but vhost/io workers do not use ksig at all.
> > >
> > > Ah, wait...
> > >
> > > I forgot about the ->sa_flags check before hide_si_addr_tag_bits().
> > >
> > > OK, thanks... I'll send V2.
> >
> > Tomorrow.
> >
> > I'll recheck, but I think we need the patch below as 1/2 regadless of
> > this change
> >
> > Oleg.
> > ---
> >
> > --- a/kernel/signal.c
> > +++ b/kernel/signal.c
> > @@ -2897,13 +2897,13 @@ bool get_signal(struct ksignal *ksig)
> >   		/* NOTREACHED */
> >   	}
> >   	spin_unlock_irq(&sighand->siglock);
> > -out:
> > +
> >   	ksig->sig = signr;
> >
> >   	if (!(ksig->ka.sa.sa_flags & SA_EXPOSE_TAGBITS))
> >   		hide_si_addr_tag_bits(ksig);
> > -
> > -	return ksig->sig > 0;
> > +out:
> > +	return signr > 0;
> >   }
> >
> >   /**