Re: [Xen-devel] [PATCH] credit2: avoid NULL deref in csched2_res_pick() when tracing

[Dario Faggioli <dfaggioli@xxxxxxxx>]

On Mon, 2020-03-02 at 17:59 +0100, Jürgen Groß wrote:
> On 02.03.20 17:49, Dario Faggioli wrote:
> > On Mon, 2020-03-02 at 09:58 +0100, Jan Beulich wrote:
> > > 
> > >     @@ -2360,6 +2360,8 @@
> > >                          unit->cpu_soft_affinity);
> > >              cpumask_and(cpumask_scratch_cpu(cpu),
> > > cpumask_scratch_cpu(cpu),
> > >                          &min_s_rqd->active);
> > >     +
> > >     +        BUG_ON(!min_rqd);
> > >          }
> > >          else if ( min_rqd )
> > >          {
> > > 
> > > possibly accompanied by a comment. Thoughts?
> > > 
> > Yes, I think this is a good idea.
> > 
> > Personally, I'd put the BUG_ON() outside of the "if {} else if {}
> > else
> > {}" block (i.e., just above the cpumask_cycle().
> 
> I don't think so.
> 
> Otherwise the "else if ( min_rqd )" wouldn't make sense.
> 
Why wouldn't it? 

I mean, what I was saying is that I think it would be nice to have,
just before this:

 new_cpu = cpumask_cycle(min_rqd->pick_bias, cpumask_scratch_cpu(cpu));
 min_rqd->pick_bias = new_cpu;

A BUG_ON(!min_rqd), with a comment above it explainint that, no matter
how we got here, at this point we are sure that min_rqd is valid,
either because it already was before the if{} (in which case we took
either the first or the second branch of the if itself), or because we
did setup it in the if{} itself (in the third branch).

I see why one may want for something like that to be in the first
branch of the if{}... I was just saying that I personally like it
better for something like this to be close to where the pointer is
dereferenced...

Regards
-- 
Dario Faggioli, Ph.D
http://about.me/dario.faggioli
Virtualization Software Engineer
SUSE Labs, SUSE https://www.suse.com/
-------------------------------------------------------------------
<<This happens because _I_ choose it to happen!>> (Raistlin Majere)

Attachment: signature.asc
Description: This is a digitally signed message part

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel