Re: [Xen-devel] BUG_ON() vs ASSERT()

[George Dunlap <george.dunlap@xxxxxxxxxx>]

On Tue, Sep 13, 2016 at 7:16 PM, Andrew Cooper
<andrew.cooper3@xxxxxxxxxx> wrote:
> On 12/09/16 16:23, Jan Beulich wrote:
>> All,
>>
>> in
>> https://lists.xenproject.org/archives/html/xen-devel/2016-09/msg01201.html
>> and
>> https://lists.xenproject.org/archives/html/xen-devel/2016-09/msg01210.html
>> Andrew basically suggests that we should switch away from using
>> ASSERT() and over to BUG_ON() in perhaps quite broad a set of
>> cases. And honestly I'm not convinced of this: We've been adding
>> quite a few ASSERT()s over the last years with the aim of doing
>> sanity checking in debug builds, without adding overhead to non-
>> debug builds. I can certainly see possible cases where using
>> BUG_ON() to prevent further possible damage is appropriate, but
>> I don't think we should overdo here.
>
> I am not advocating switching all ASSERT()s to BUG_ON()s.  That would be
> silly.
>
> However, ASSERT()'s as a bounds check very definitely are dangerous.  If
> there is any uncertainty about the bounds, the check must not disappear
> in a release build.  (Better yet, code which copes cleanly with
> insufficient bounds).
>
>
> For anyone reading this email who hasn't already worked out the details
> of XSA-186, the data corruption issue is here:
>
> static int hvmemul_insn_fetch(...)
> {
>     unsigned int insn_off = offset - hvmemul_ctxt->insn_buf_eip;
>     ...
>     ASSERT(insn_off + bytes <= sizeof(hvmemul_ctxt->insn_buf));
>     memcpy(&hvmemul_ctxt->insn_buf[insn_off], p_data, bytes);
>     ...
>
> It is left as an exercise to the reader to work out how to exploit this
> on a release build of Xen, but it is hopefully obvious that the ASSERT()
> isn't helpful.  A BUG_ON() in this case would have been a host DoS,
> which is substantially better than a guest escape.

This seems quite sensible, and I'm glad Andy clarified.  (Although in
a lot of these cases, a domain_crash() would be preferable to a
BUG_ON()  if possible.)

 -George

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel