linux-kernel - Re: [PATCH 1/1] s390: vfio-ccw: push down unsupported IDA check

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20180514170135.2cab4f7d.cohuck@redhat.com>
Date:   Mon, 14 May 2018 17:01:35 +0200
From:   Cornelia Huck <cohuck@...hat.com>
To:     Halil Pasic <pasic@...ux.ibm.com>
Cc:     Dong Jia Shi <bjsdjshi@...ux.ibm.com>,
        linux-kernel@...r.kernel.org, kvm@...r.kernel.org,
        linux-s390@...r.kernel.org
Subject: Re: [PATCH 1/1] s390: vfio-ccw: push down unsupported IDA check

On Mon, 14 May 2018 16:44:29 +0200
Halil Pasic <pasic@...ux.ibm.com> wrote:

> On 05/14/2018 04:00 PM, Cornelia Huck wrote:
> > On Mon, 14 May 2018 15:37:17 +0200
> > Halil Pasic <pasic@...ux.ibm.com> wrote:
> >   
> >> On 05/14/2018 01:55 PM, Cornelia Huck wrote:  
> >>> On Wed,  9 May 2018 19:36:47 +0200
> >>> Halil Pasic <pasic@...ux.ibm.com> wrote:
> >>>      
> >>>> +		/*
> >>>> +		 * 2k byte block IDAWs (fmt1 or fmt2) are not yet supported.
> >>>> +		 * There are however CPs that don't use IDA at all, and can
> >>>> +		 * benefit from not failing until failure is eminent.  
> >>>
> >>> What about:
> >>>
> >>> "As we don't want to fail direct addressing even if the orb specified
> >>> one of the unsupported formats, we defer checking for IDAWs in
> >>> unsupported formats to here."  
> >>
> >> Was the second sentence only confusing because of CP? I'm not perfectly
> >> satisfied with your version either:
> >> * 'fail direct addressing even if the orb specified one of the unsupported formats'
> >>      I wanted to say: 'hey it does not matter what format for IDA the orb implies
> >>      if the channel program does not use any IDA at all'. That could be paraphrased
> >>      as channel programs using direct addressing exclusively. But failing the direct
> >>      addressing does not fit for me.  
> > 
> > But that's effectively what happens now, no? We reject the orb out of
> > hand due to unsupported flags that do not have any relevance for the
> > channel program in that case.  
> 
> Yes, that's what happens now, except that we make the whole channel program fail,
> and not the direct addressing. But the comment should describe what happens
> with the patch applied.

Even more, it should describe _why_ it is done that way (the reason
being "we don't want to fail..."). That's where I've been coming from.

> >>> The patch looks sane, I have only issues with the description/comments.
> >>>      
> >>
> >> Thanks for having a look. Please give me short feedback about the one
> >> open point and I will respin with the requested changes.  
> > 
> > Does anybody else have feedback?
> >   
> 
> Will wait a day or so. Dong Jia and Jason have already seen the patch, and
> they only complained about the text. Since that spin was mainly for the
> tested-by tags, and I stated that any substantial discussion should happen
> upstream, I ignored those complaints.
> 
> So yes I will wait a bit so everybody can chime in.

Sounds good.