On Mon, Mar 11, 2024 at 2:27 PM James Houghton <jthoughton@xxxxxxxxxx> wrote:
>
> On Mon, Mar 11, 2024 at 12:28 PM Peter Xu <peterx@xxxxxxxxxx> wrote:
> >
> > On Mon, Mar 11, 2024 at 11:59:59AM -0700, Axel Rasmussen wrote:
> > > I'd prefer not to require root or CAP_SYS_ADMIN or similar for
> > > UFFDIO_POISON, because those control access to lots more things
> > > besides, which we don't necessarily want the process using UFFD to be
> > > able to do. :/
>
> I agree; UFFDIO_POISON should not require CAP_SYS_ADMIN.
+1.
>
> > >
> > > Ratelimiting seems fairly reasonable to me. I do see the concern about
> > > dropping some addresses though.
> >
> > Do you know how much could an admin rely on such addresses? How frequent
> > would MCE generate normally in a sane system?
>
> I'm not sure about how much admins rely on the address themselves. +cc
> Jiaqi Yan
I think admins mostly care about MCEs from **real** hardware. For
example they may choose to perform some maintenance if the number of
hardware DIMM errors, keyed by PFN, exceeds some threshold. And I
think mcelog or /sys/devices/system/node/node${X}/memory_failure are
better tools than dmesg. In the case all memory errors are emulated by
hypervisor after a live migration, these dmesgs may confuse admins to
think there is dimm error on host but actually it is not the case. In
this sense, silencing these emulated by UFFDIO_POISON makes sense (if
not too complicated to do).
SIGBUS (and logged "MCE: Killing %s:%d due to hardware memory
corruption fault at %lx\n") emit by fault handler due to UFFDIO_POISON
are less useful to admins AFAIK. They are for sure crucial to
userspace / vmm / hypervisor, but the SIGBUS sent already contains the
poisoned address (in si_addr from force_sig_mceerr).
>
> It's possible for a sane hypervisor dealing with a buggy guest / guest
> userspace to trigger lots of these pr_errs. Consider the case where a
> guest userspace uses HugeTLB-1G, finds poison (which HugeTLB used to
> ignore), and then ignores SIGBUS. It will keep getting MCEs /
> SIGBUSes.
>
> The sane hypervisor will use UFFDIO_POISON to prevent the guest from
> re-accessing *real* poison, but we will still get the pr_err, and we
> still keep injecting MCEs into the guest. We have observed scenarios
> like this before.
>
> >
> > > Perhaps we can mitigate that concern by defining our own ratelimit
> > > interval/burst configuration?
> >
> > Any details?
> >
> > > Another idea would be to only ratelimit it if !CONFIG_DEBUG_VM or
> > > similar. Not sure if that's considered valid or not. :)
> >
> > This, OTOH, sounds like an overkill..
> >
> > I just checked again on the detail of ratelimit code, where we by default
> > it has:
> >
> > #define DEFAULT_RATELIMIT_INTERVAL (5 * HZ)
> > #define DEFAULT_RATELIMIT_BURST 10
> >
> > So it allows a 10 times burst rather than 2.. IIUC it means even if
> > there're continous 10 MCEs it won't get suppressed, until the 11th came, in
> > 5 seconds interval. I think it means it's possibly even less of a concern
> > to directly use pr_err_ratelimited().
>
> I'm okay with any rate limiting everyone agrees on. IMO, silencing
> these pr_errs if they came from UFFDIO_POISON (or, perhaps, if they
> did not come from real hardware MCE events) sounds like the most
> correct thing to do, but I don't mind. Just don't make UFFDIO_POISON
> require CAP_SYS_ADMIN. :)
>
> Thanks.
