Re: [PATCH 8/9] s390x/css: Expand check against no-data-count CCWs

[Eric Farman <farman@linux.ibm.com>]

On 9/16/26 7:16 PM, Matthew Rosato wrote:
> On 9/16/26 4:32 PM, Eric Farman wrote:
>> The channel subsystem will reject channel programs that incorporate
>> 256 CCWs without a data transfer, but the existing check for this
>> does so against a data address of zero. Expand the check to include
>> other CCWs that are valid but would transfer no data.
>>
>> Fixes: e8601dd5d0 ("s390x/css: catch ccw sequence errors")
> 
> guest-triggerable, cc stable
> 
>> Signed-off-by: Eric Farman <farman@linux.ibm.com>
>> ---
>>   hw/s390x/css.c | 4 +++-
>>   1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/hw/s390x/css.c b/hw/s390x/css.c
>> index 9da9128d88..c38549c7be 100644
>> --- a/hw/s390x/css.c
>> +++ b/hw/s390x/css.c
>> @@ -1016,7 +1016,9 @@ static int css_interpret_ccw(SubchDev *sch, hwaddr ccw_addr,
>>   
>>       check_len = !((ccw.flags & CCW_FLAG_SLI) && !(ccw.flags & CCW_FLAG_DC));
>>   
>> -    if (!ccw.cda) {
>> +    if (!ccw.cda ||
>> +        (ccw.flags & CCW_FLAG_SKIP) ||
>> +        (ccw.cmd_code == CCW_CMD_NOOP)) {
> 
> I'm not sure if this list is complete...

Surely the list of opcodes for virtio is far smaller than, say, actual 
DASD, but I'm not sure it's missing much. Maybe ccw.count == 0; that's 
valid for format-1 CCWs (we correctly reject it earlier than here for 
format-0), but would not move data.

> 
> I wonder if we could we key off of the number of bytes transferred e.g.
> sch->cds.at_byte == 0 instead?  Would have to happen at the end of this
> function e.g. in the ret == 0 check maybe?

But wouldn't that only be looking at just a single CCW? That's totally 
fine by itself, whereas this is trying to set an upper bound of chained 
CCWs in a single program that don't do meaningful work.

> 
>>           if (sch->ccw_no_data_cnt == 255) {
>>               return -EINVAL;
>>           }
>