linux-kernel - Re: [PATCH v18 08/18] s390/vfio-ap: allow assignment of unavailable AP queues to mdev device

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 for Android: free password hash cracker in your pocket

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

Message-ID: <20220309015623.41543d75.pasic@linux.ibm.com>
Date:   Wed, 9 Mar 2022 01:56:22 +0100
From:   Halil Pasic <pasic@...ux.ibm.com>
To:     "Jason J. Herne" <jjherne@...ux.ibm.com>
Cc:     Tony Krowiak <akrowiak@...ux.ibm.com>, linux-s390@...r.kernel.org,
        linux-kernel@...r.kernel.org, kvm@...r.kernel.org,
        freude@...ux.ibm.com, borntraeger@...ibm.com, cohuck@...hat.com,
        mjrosato@...ux.ibm.com, alex.williamson@...hat.com,
        kwankhede@...dia.com, fiuczy@...ux.ibm.com,
        Halil Pasic <pasic@...ux.ibm.com>
Subject: Re: [PATCH v18 08/18] s390/vfio-ap: allow assignment of unavailable
 AP queues to mdev device

On Tue, 8 Mar 2022 10:39:09 -0500
"Jason J. Herne" <jjherne@...ux.ibm.com> wrote:

> On 3/7/22 07:31, Tony Krowiak wrote:
> >>> +         * If the input apm and aqm belong to the matrix_mdev's matrix,
> >>> +         * then move on to the next.
> >>> +         */
> >>> +        if (mdev_apm == matrix_mdev->matrix.apm &&
> >>> +            mdev_aqm == matrix_mdev->matrix.aqm)
> >>>               continue;  
> >>
> >> We may have a problem here. This check seems like it exists to stop you from
> >> comparing an mdev's apm/aqm with itself. Obviously comparing an mdev's newly
> >> updated apm/aqm with itself would cause a false positive sharing check, right?
> >> If this is the case, I think the comment should be changed to reflect that.  
> > 
> > You are correct, this check is performed to prevent comparing an mdev to
> > itself, I'll improve the comment.
> >   
> >>
> >> Aside from the comment, what stops this particular series of if statements from
> >> allowing us to configure a second mdev with the exact same apm/aqm values as an
> >> existing mdev? If we do, then this check's continue will short circuit the rest
> >> of the function thereby allowing that 2nd mdev even though it should be a
> >> sharing violation.  
> > 
> > I don't see how this is possible.  
> 
> You are correct. I missed the fact that you are comparing pointers here, and not
> values. Apologies. Now that I understand the code, I agree that this is fine as is.
> 

I believe clarifying the 'belongs to' vs 'is a part of' stuff is still
worthwhile, because 'belongs to' does beg the question you asked. Thus
IMHO it is good that you raised the question.

Regards,
Halil