linux-kernel - RE: [PATCH v5 1/3] Drivers: hv: vmbus: Add vmbus_requestor data structure for VMBus hardening

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: <CH2PR21MB14004CC8735841324CC4F5338B790@CH2PR21MB1400.namprd21.prod.outlook.com>
Date:   Wed, 22 Jul 2020 19:55:49 +0000
From:   Andres Beltran <t-mabelt@...rosoft.com>
To:     Michael Kelley <mikelley@...rosoft.com>,
        KY Srinivasan <kys@...rosoft.com>,
        Haiyang Zhang <haiyangz@...rosoft.com>,
        Stephen Hemminger <sthemmin@...rosoft.com>,
        "wei.liu@...nel.org" <wei.liu@...nel.org>
CC:     "linux-hyperv@...r.kernel.org" <linux-hyperv@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "parri.andrea@...il.com" <parri.andrea@...il.com>
Subject: RE: [PATCH v5 1/3] Drivers: hv: vmbus: Add vmbus_requestor data
 structure for VMBus hardening

> From: Michael Kelley <mikelley@...rosoft.com> Sent: Wednesday, July 22, 2020 3:25 PM
> I don't think the above does what you want.  The allocated
> array ends up as follows:
> 
> Slot 0 contains "2"
> Slot 1 contains "3"
> ...
> Slot size-2 contains size
> Slot size-1 contains U64_MAX
> 
> This means that allocating the next-to-last entry will go
> awry.  I think the previous version of the slot initialization
> code will actually work just fine.
> 

vmbus_next_request_id() and vmbus_request_addr() check that
the id > size, and then the array index becomes id - 1 (or size - 1 for
the next-to-last entry, which is the last slot) , so I think this works fine. 
But as you suggested below, returning current_id + 1 and decrementing 
trans_id seems cleaner to me.

> The overall scheme you are using to handle the 0 transactionID is
> a good one.  Basically the slot array is still tracking values 0 thru
> size-1, but what is presented to the calling VMbus driver is values
> in the range 1 thru size.  That way you can recognize 0 as a special case.
> So take this implementation approach:
> *  Start with the previous version of the vmbus_next_request_id()
> and vmbus_request_addr() code.
> *  In vmbus_next_request_id(), just return current_id+1 instead of
> current_id.
> * In vmbus_request_addr(), add the new code that checks trans_id
> for 0 and returns immediately.  Otherwise, decrement trans_id by 1
> and proceed with the existing code.
> 
> With this approach, none of the initialization code needs to change.
> Everything uses values in the range 0 to size-1, except that what is
> presented to the VMbus drivers is shifted higher by 1.

Yes, I'll do this instead.

Andres.