Re: [Xen-devel] [PATCH] mm: make opt_bootscrub non-init

[Julien Grall <julien.grall@xxxxxxx>]

Hi Jan,

On 26/11/2018 12:18, Jan Beulich wrote:
> > > > On 26.11.18 at 13:04, <roger.pau@xxxxxxxxxx> wrote:
> > On Mon, Nov 26, 2018 at 03:06:16AM -0700, Jan Beulich wrote:
> > > > > > On 23.11.18 at 15:30, <roger.pau@xxxxxxxxxx> wrote:
> > > > LLVM code generation can attempt to load from a variable in the next
> > > > condition of an expression under certain circumstances, thus turning
> > > > the following condition:
> > > >
> > > > if ( system_state < SYS_STATE_active && opt_bootscrub == BOOTSCRUB_IDLE )
> > > >
> > > > Into:
> > > >
> > > > 0xffff82d080223967 <+103>: cmpl   $0x3,0x37b032(%rip) # 0xffff82d08059e9a0
> > <system_state>
> > > > 0xffff82d08022396e <+110>: setb   -0x29(%rbp)
> > > > 0xffff82d080223972 <+114>: cmpl   $0x2,0x228a8b(%rip) # 0xffff82d08044c404
> > <opt_bootscrub>
> > > > Such code will trigger a page fault if system_state >=
> > > > SYS_STATE_active because opt_bootscrub will be unmapped.
> > > >
> > > > Fix this by making opt_bootscrub non-init, thus preventing the page
> > > > fault. The LLVM bug with the discussion about this issue can be found
> > > > at:
> > > >
> > > > https://bugs.llvm.org/show_bug.cgi?id=39707
> > > >
> > > > I haven't been able to find any other instances of such conditional
> > > > expression that uses system_state together with an init variable or
> > > > function.
> > > >
> > > > Signed-off-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>
> > > I can accept this as a band-aid, so I'm not going to nack it, but
> > > I don't view this as a feasible solution to the problem. That's in
> > > particular because nothing is done at all to prevent future
> > > similar issues. Even worse, ...
> > I'm not sure what's the best way to prevent future issues. Should this
> > be mentioned in the coding style? That doesn't seems like the best
> > place, but I'm not sure where else could this be documented.
> There was some vaguely similar discussion a little while ago, and
> there iirc we had also agreed that the point there (which I don't
> recall) is not a style thing. Same here: We're talking about a
> correctness issue, not a stylistic one. Hence indeed a separate
> document would be needed, but none of the existing ones looks
> to be a good fit.
>
> Furthermore I doubt writing this down would help, because for
> such apparently simple things no-one goes hunt for related
> documentation. I think the only future proof course of action
> would be to port Linux'es section mismatch handling and stop
> allowing problematic cross references. That approach has
> downsides though, which is why I'm not going to advocate it.
May I ask what are the downsides? Do you expect a lot of false positive?

Cheers,

--
Julien Grall

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