Re: [PATCH v1 2/8] s390/sclp: check sccb len before filling in data

[Cornelia Huck]

On Wed, 13 May 2020 09:43:37 +0200
Janosch Frank <address@hidden> wrote:

> On 5/12/20 6:01 PM, Cornelia Huck wrote:
> > On Mon, 11 May 2020 17:02:06 +0200
> > David Hildenbrand <address@hidden> wrote:
> >   
> >> On 11.05.20 16:50, Janosch Frank wrote:  
> >>> On 5/11/20 4:44 PM, David Hildenbrand wrote:    
> >>>> On 11.05.20 16:36, Janosch Frank wrote:    
> >>>>> On 5/9/20 1:08 AM, Collin Walling wrote:    
> >>>>>> The SCCB must be checked for a sufficient length before it is filled
> >>>>>> with any data. If the length is insufficient, then the SCLP command
> >>>>>> is suppressed and the proper response code is set in the SCCB header.
> >>>>>>
> >>>>>> Signed-off-by: Collin Walling <address@hidden>    
> >>>>>
> >>>>> Fixes tag?  
> > 
> > Probably
> > 
> > Fixes: 832be0d8a3bb ("s390x: sclp: Report insufficient SCCB length")
> > 
> > ?
> >   
> >>>>> Reviewed-by: Janosch Frank <address@hidden>    
> >>>>
> >>>> This is not a fix AFAIKs.
> >>>> sclp_service_call()/sclp_service_call_protected() always supplies a full
> >>>> SCCB of exactly 4k size.
> >>>>    
> >>>
> >>> We don't check for QEMU's 4k buffer here, but for the length that was
> >>> specified by the guest.
> >>>
> >>> It's valid for the guest to request cpu info and state that its buffer
> >>> is only 1k. We can't write everything in 1k if we have ~200 cpus, so
> >>> we'll report the insufficient length rc.
> >>>
> >>> What he fixes here is the time of the length check, it should be done
> >>> before any changes are being done to the work_sccb.    
> >>
> >> I don't have access to the spec, especially, if the guest can expect
> >> nothing else in the sccb to change in case we report an error code. So
> >> whatever you tell me, I have to trust you :)  
> > 
> > Same here. Sounds plausible, but I have to trust the folks with the
> > documentation :)
> >   
> 
> The AR states that:
> * Command validity check (has prio over length, as length is dependent
> on command)
> * boundary (if extended-length is not available)
> * Sufficient length check
> 
> are done before "any other command action is taken".
> If a test fails the command is suppressed.

Thanks, makes sense.

pgpF6cVDviDk4.pgp
Description: OpenPGP digital signature