diff mbox series

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

Message ID 20200508230823.22956-3-walling@linux.ibm.com
State New
Headers show
Series s390: Extended-Length SCCB & DIAGNOSE 0x318 | expand

Commit Message

Collin Walling May 8, 2020, 11:08 p.m. UTC
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 <walling@linux.ibm.com>
---
 hw/s390x/sclp.c | 22 ++++++++++------------
 smp.max_cpus    |  0
 2 files changed, 10 insertions(+), 12 deletions(-)
 create mode 100644 smp.max_cpus

Comments

Janosch Frank May 11, 2020, 2:36 p.m. UTC | #1
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 <walling@linux.ibm.com>

Fixes tag?
Reviewed-by: Janosch Frank <frankja@linux.ibm.com>

> ---
>  hw/s390x/sclp.c | 22 ++++++++++------------
>  smp.max_cpus    |  0
>  2 files changed, 10 insertions(+), 12 deletions(-)
>  create mode 100644 smp.max_cpus
> 
> diff --git a/hw/s390x/sclp.c b/hw/s390x/sclp.c
> index 156ffe3223..d08a291e40 100644
> --- a/hw/s390x/sclp.c
> +++ b/hw/s390x/sclp.c
> @@ -76,6 +76,11 @@ static void read_SCP_info(SCLPDevice *sclp, SCCB *sccb)
>      int rnsize, rnmax;
>      IplParameterBlock *ipib = s390_ipl_get_iplb();
>  
> +    if (be16_to_cpu(sccb->h.length) < (sizeof(ReadInfo) + cpu_count * sizeof(CPUEntry))) {
> +        sccb->h.response_code = cpu_to_be16(SCLP_RC_INSUFFICIENT_SCCB_LENGTH);
> +        return;
> +    }
> +
>      /* CPU information */
>      prepare_cpu_entries(read_info->entries, &cpu_count);
>      read_info->entries_cpu = cpu_to_be16(cpu_count);
> @@ -84,12 +89,6 @@ static void read_SCP_info(SCLPDevice *sclp, SCCB *sccb)
>  
>      read_info->ibc_val = cpu_to_be32(s390_get_ibc_val());
>  
> -    if (be16_to_cpu(sccb->h.length) <
> -            (sizeof(ReadInfo) + cpu_count * sizeof(CPUEntry))) {
> -        sccb->h.response_code = cpu_to_be16(SCLP_RC_INSUFFICIENT_SCCB_LENGTH);
> -        return;
> -    }
> -
>      /* Configuration Characteristic (Extension) */
>      s390_get_feat_block(S390_FEAT_TYPE_SCLP_CONF_CHAR,
>                           read_info->conf_char);
> @@ -135,17 +134,16 @@ static void sclp_read_cpu_info(SCLPDevice *sclp, SCCB *sccb)
>      ReadCpuInfo *cpu_info = (ReadCpuInfo *) sccb;
>      int cpu_count;
>  
> +    if (be16_to_cpu(sccb->h.length) < (sizeof(ReadCpuInfo) + cpu_count * sizeof(CPUEntry))) {
> +        sccb->h.response_code = cpu_to_be16(SCLP_RC_INSUFFICIENT_SCCB_LENGTH);
> +        return;
> +    }
> +
>      prepare_cpu_entries(cpu_info->entries, &cpu_count);
>      cpu_info->nr_configured = cpu_to_be16(cpu_count);
>      cpu_info->offset_configured = cpu_to_be16(offsetof(ReadCpuInfo, entries));
>      cpu_info->nr_standby = cpu_to_be16(0);
>  
> -    if (be16_to_cpu(sccb->h.length) <
> -            (sizeof(ReadCpuInfo) + cpu_count * sizeof(CPUEntry))) {
> -        sccb->h.response_code = cpu_to_be16(SCLP_RC_INSUFFICIENT_SCCB_LENGTH);
> -        return;
> -    }
> -
>      /* The standby offset is 16-byte for each CPU */
>      cpu_info->offset_standby = cpu_to_be16(cpu_info->offset_configured
>          + cpu_info->nr_configured*sizeof(CPUEntry));
> diff --git a/smp.max_cpus b/smp.max_cpus
> new file mode 100644
> index 0000000000..e69de29bb2
>
David Hildenbrand May 11, 2020, 2:44 p.m. UTC | #2
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 <walling@linux.ibm.com>
> 
> Fixes tag?
> Reviewed-by: Janosch Frank <frankja@linux.ibm.com>

This is not a fix AFAIKs.
sclp_service_call()/sclp_service_call_protected() always supplies a full
SCCB of exactly 4k size.
Collin Walling May 11, 2020, 2:47 p.m. UTC | #3
On 5/11/20 10:44 AM, 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 <walling@linux.ibm.com>
>>
>> Fixes tag?
>> Reviewed-by: Janosch Frank <frankja@linux.ibm.com>
> 
> This is not a fix AFAIKs.
> sclp_service_call()/sclp_service_call_protected() always supplies a full
> SCCB of exactly 4k size.
> 
> 

Right. This is more-or-less a preparation patch for the extended-length
SCCB stuff which allows an SCCB > 4k.

The Linux kernel always provides a 4k SSCB today, so this patch
more-or-less makes the code AR compliant.
Janosch Frank May 11, 2020, 2:50 p.m. UTC | #4
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 <walling@linux.ibm.com>
>>
>> Fixes tag?
>> Reviewed-by: Janosch Frank <frankja@linux.ibm.com>
> 
> 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.
David Hildenbrand May 11, 2020, 3:02 p.m. UTC | #5
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 <walling@linux.ibm.com>
>>>
>>> Fixes tag?
>>> Reviewed-by: Janosch Frank <frankja@linux.ibm.com>
>>
>> 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 :)
Cornelia Huck May 12, 2020, 4:01 p.m. UTC | #6
On Mon, 11 May 2020 17:02:06 +0200
David Hildenbrand <david@redhat.com> 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 <walling@linux.ibm.com>  
> >>>
> >>> Fixes tag?

Probably

Fixes: 832be0d8a3bb ("s390x: sclp: Report insufficient SCCB length")

?

> >>> Reviewed-by: Janosch Frank <frankja@linux.ibm.com>  
> >>
> >> 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 :)
Collin Walling May 12, 2020, 4:16 p.m. UTC | #7
On 5/12/20 12:01 PM, Cornelia Huck wrote:
> On Mon, 11 May 2020 17:02:06 +0200
> David Hildenbrand <david@redhat.com> 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 <walling@linux.ibm.com>
>>>>>
>>>>> 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 <frankja@linux.ibm.com>
>>>>
>>>> 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 :/
Cornelia Huck May 12, 2020, 4:24 p.m. UTC | #8
On Tue, 12 May 2020 12:16:45 -0400
Collin Walling <walling@linux.ibm.com> wrote:

> On 5/12/20 12:01 PM, Cornelia Huck wrote:
> > On Mon, 11 May 2020 17:02:06 +0200
> > David Hildenbrand <david@redhat.com> 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 <walling@linux.ibm.com>  
> >>>>>
> >>>>> 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).

Yes; from the description of how this is supposed to work it fixes
architectural conformance, not a bug that is triggered by today's guest
systems.

[Usage of the Fixes: tag in QEMU is not quite as essential as in Linux,
as we don't do the numerous, big stable updates here; I don't think
this is stable material, but we can certainly record where this was
introduced.]
Collin Walling May 12, 2020, 4:25 p.m. UTC | #9
On 5/12/20 12:24 PM, Cornelia Huck wrote:
> On Tue, 12 May 2020 12:16:45 -0400
> Collin Walling <walling@linux.ibm.com> wrote:
> 
>> On 5/12/20 12:01 PM, Cornelia Huck wrote:
>>> On Mon, 11 May 2020 17:02:06 +0200
>>> David Hildenbrand <david@redhat.com> 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 <walling@linux.ibm.com>
>>>>>>>
>>>>>>> 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).
> 
> Yes; from the description of how this is supposed to work it fixes
> architectural conformance, not a bug that is triggered by today's guest
> systems.
> 
> [Usage of the Fixes: tag in QEMU is not quite as essential as in Linux,
> as we don't do the numerous, big stable updates here; I don't think
> this is stable material, but we can certainly record where this was
> introduced.]
> 
> 

Makes sense to me, thanks for the info and commit ID! I'll add it to the
message for next round.
Janosch Frank May 13, 2020, 7:43 a.m. UTC | #10
On 5/12/20 6:01 PM, Cornelia Huck wrote:
> On Mon, 11 May 2020 17:02:06 +0200
> David Hildenbrand <david@redhat.com> 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 <walling@linux.ibm.com>  
>>>>>
>>>>> Fixes tag?
> 
> Probably
> 
> Fixes: 832be0d8a3bb ("s390x: sclp: Report insufficient SCCB length")
> 
> ?
> 
>>>>> Reviewed-by: Janosch Frank <frankja@linux.ibm.com>  
>>>>
>>>> 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.
Cornelia Huck May 13, 2020, 8:16 a.m. UTC | #11
On Wed, 13 May 2020 09:43:37 +0200
Janosch Frank <frankja@linux.ibm.com> wrote:

> On 5/12/20 6:01 PM, Cornelia Huck wrote:
> > On Mon, 11 May 2020 17:02:06 +0200
> > David Hildenbrand <david@redhat.com> 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 <walling@linux.ibm.com>    
> >>>>>
> >>>>> Fixes tag?  
> > 
> > Probably
> > 
> > Fixes: 832be0d8a3bb ("s390x: sclp: Report insufficient SCCB length")
> > 
> > ?
> >   
> >>>>> Reviewed-by: Janosch Frank <frankja@linux.ibm.com>    
> >>>>
> >>>> 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.
Collin Walling May 14, 2020, 5:22 p.m. UTC | #12
On 5/13/20 4:16 AM, Cornelia Huck wrote:
> On Wed, 13 May 2020 09:43:37 +0200
> Janosch Frank <frankja@linux.ibm.com> wrote:
> 
>> On 5/12/20 6:01 PM, Cornelia Huck wrote:
>>> On Mon, 11 May 2020 17:02:06 +0200
>>> David Hildenbrand <david@redhat.com> 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 <walling@linux.ibm.com>    
>>>>>>>
>>>>>>> Fixes tag?  
>>>
>>> Probably
>>>
>>> Fixes: 832be0d8a3bb ("s390x: sclp: Report insufficient SCCB length")
>>>
>>> ?
>>>   
>>>>>>> Reviewed-by: Janosch Frank <frankja@linux.ibm.com>    
>>>>>>
>>>>>> 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.
> 

Thanks, Janosch! (I suppose I could've said the same as well. Sorry
about that).
David Hildenbrand May 18, 2020, 11:43 a.m. UTC | #13
On 09.05.20 01:08, 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 <walling@linux.ibm.com>
> ---
>  hw/s390x/sclp.c | 22 ++++++++++------------
>  smp.max_cpus    |  0
>  2 files changed, 10 insertions(+), 12 deletions(-)
>  create mode 100644 smp.max_cpus
> 
> diff --git a/hw/s390x/sclp.c b/hw/s390x/sclp.c
> index 156ffe3223..d08a291e40 100644
> --- a/hw/s390x/sclp.c
> +++ b/hw/s390x/sclp.c
> @@ -76,6 +76,11 @@ static void read_SCP_info(SCLPDevice *sclp, SCCB *sccb)
>      int rnsize, rnmax;
>      IplParameterBlock *ipib = s390_ipl_get_iplb();
>  
> +    if (be16_to_cpu(sccb->h.length) < (sizeof(ReadInfo) + cpu_count * sizeof(CPUEntry))) {
> +        sccb->h.response_code = cpu_to_be16(SCLP_RC_INSUFFICIENT_SCCB_LENGTH);
> +        return;
> +    }
> +

Lines too long.

Please run scripts/checkpatch.pl before submitting.
diff mbox series

Patch

diff --git a/hw/s390x/sclp.c b/hw/s390x/sclp.c
index 156ffe3223..d08a291e40 100644
--- a/hw/s390x/sclp.c
+++ b/hw/s390x/sclp.c
@@ -76,6 +76,11 @@  static void read_SCP_info(SCLPDevice *sclp, SCCB *sccb)
     int rnsize, rnmax;
     IplParameterBlock *ipib = s390_ipl_get_iplb();
 
+    if (be16_to_cpu(sccb->h.length) < (sizeof(ReadInfo) + cpu_count * sizeof(CPUEntry))) {
+        sccb->h.response_code = cpu_to_be16(SCLP_RC_INSUFFICIENT_SCCB_LENGTH);
+        return;
+    }
+
     /* CPU information */
     prepare_cpu_entries(read_info->entries, &cpu_count);
     read_info->entries_cpu = cpu_to_be16(cpu_count);
@@ -84,12 +89,6 @@  static void read_SCP_info(SCLPDevice *sclp, SCCB *sccb)
 
     read_info->ibc_val = cpu_to_be32(s390_get_ibc_val());
 
-    if (be16_to_cpu(sccb->h.length) <
-            (sizeof(ReadInfo) + cpu_count * sizeof(CPUEntry))) {
-        sccb->h.response_code = cpu_to_be16(SCLP_RC_INSUFFICIENT_SCCB_LENGTH);
-        return;
-    }
-
     /* Configuration Characteristic (Extension) */
     s390_get_feat_block(S390_FEAT_TYPE_SCLP_CONF_CHAR,
                          read_info->conf_char);
@@ -135,17 +134,16 @@  static void sclp_read_cpu_info(SCLPDevice *sclp, SCCB *sccb)
     ReadCpuInfo *cpu_info = (ReadCpuInfo *) sccb;
     int cpu_count;
 
+    if (be16_to_cpu(sccb->h.length) < (sizeof(ReadCpuInfo) + cpu_count * sizeof(CPUEntry))) {
+        sccb->h.response_code = cpu_to_be16(SCLP_RC_INSUFFICIENT_SCCB_LENGTH);
+        return;
+    }
+
     prepare_cpu_entries(cpu_info->entries, &cpu_count);
     cpu_info->nr_configured = cpu_to_be16(cpu_count);
     cpu_info->offset_configured = cpu_to_be16(offsetof(ReadCpuInfo, entries));
     cpu_info->nr_standby = cpu_to_be16(0);
 
-    if (be16_to_cpu(sccb->h.length) <
-            (sizeof(ReadCpuInfo) + cpu_count * sizeof(CPUEntry))) {
-        sccb->h.response_code = cpu_to_be16(SCLP_RC_INSUFFICIENT_SCCB_LENGTH);
-        return;
-    }
-
     /* The standby offset is 16-byte for each CPU */
     cpu_info->offset_standby = cpu_to_be16(cpu_info->offset_configured
         + cpu_info->nr_configured*sizeof(CPUEntry));
diff --git a/smp.max_cpus b/smp.max_cpus
new file mode 100644
index 0000000000..e69de29bb2