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

[Collin Walling]

On 5/12/20 12: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")
>
> ?

Sounds reasonable. This patch doesn't fix any explicitly-known bugs 
AFAIK. The s390 Linux kernel is hard-coded to use a 4K size SCCB when
executing these commands.

I suppose this could introduce a bug if things change in the Linux 
kernel or if some other OS wants to use this command. That should be 
enough of a justification, right? (Just want to make sure I understand 
the use of the tag correctly).

> > > > > 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 :)

Sorry I can't be of much help here :/

--
--
Regards,
Collin

Stay safe and stay healthy