Re: [PATCH 9/9] xen: add SAF deviation for safe cast removal.

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Thu, 14 Dec 2023, Jan Beulich wrote:
> On 14.12.2023 13:07, Simone Ballarin wrote:
> > --- a/docs/misra/safe.json
> > +++ b/docs/misra/safe.json
> > @@ -28,6 +28,14 @@
> >          },
> >          {
> >              "id": "SAF-3-safe",
> > +            "analyser": {
> > +                "eclair": "MC3R1.R11.8"
> > +            },
> > +            "name": "MC3R1.R11.8: removal of const qualifier to comply 
> > with function signature",
> > +            "text": "It is safe to cast away const qualifiers to comply 
> > with function signature if the function does not modify the pointee."
> 
> I'm not happy with this description, as it invites for all sorts of abuse.
> Yet I'm also puzzled that ...

We can improve the language but the concept would still be the same. For
instance:

A single function might or might not modify the pointee depending on
other function parameters (for instance a single function that could
either read or write depending on how it is called). It is safe to cast
away const qualifiers when passing a parameter to a function of this
type when the other parameters are triggering a read-only operation.


> > --- a/xen/arch/x86/hvm/hvm.c
> > +++ b/xen/arch/x86/hvm/hvm.c
> > @@ -3413,6 +3413,7 @@ static enum hvm_translation_result __hvm_copy(
> >  enum hvm_translation_result hvm_copy_to_guest_phys(
> >      paddr_t paddr, const void *buf, unsigned int size, struct vcpu *v)
> >  {
> > +    /* SAF-3-safe */
> >      return __hvm_copy((void *)buf /* HVMCOPY_to_guest doesn't modify */,
> >                        paddr, size, v,
> >                        HVMCOPY_to_guest | HVMCOPY_phys, 0, NULL);
> 
> ... this is the only place you then use it. Afaict some of Arm's copy_guest()
> callers ought to have a similar issue. If so, an enlarged patch should be
> discussed with a larger audience, to see how we collectively think we want to
> put this specific kind of deviation.

We have a similar problem, see xen/arch/arm/guestcopy.c
raw_copy_to_guest and raw_copy_from_guest.

I would use the SAF deviation there too.

In the case here, I think the comment "HVMCOPY_to_guest doesn't modify"
could be removed as it becomes redundant with SAF-3-safe, but I'll leave
it to you.