linux-kernel - [PATCH 1/2] VMCI: drop condition with no effect

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-next>] [day] [month] [year] [list]

Date:   Sun,  6 Nov 2016 13:53:33 +0100
From:   Nicholas Mc Guire <hofrat@...dl.org>
To:     "Kirill A. Shutemov" <kirill.shutemov@...ux.intel.com>
Cc:     Michal Hocko <mhocko@...e.com>,
        George Zhang <georgezhang@...are.com>,
        linux-kernel@...r.kernel.org, Nicholas Mc Guire <hofrat@...dl.org>
Subject: [PATCH 1/2] VMCI: drop condition with no effect

The condition calls the same code, further the code called does not 
differentiate between old/new-style VMX anyway so the if/else here 
is simply not needed.

Fixes: 06164d2b72aa ("VMCI: queue pairs implementation.")
Signed-off-by: Nicholas Mc Guire <hofrat@...dl.org>
---

problem found by coccinelle

drivers/misc/vmw_vmci/vmci_queue_pair.c:vmci_qp_broker_detach()
2232        if (entry->vmci_page_files)
2233                qp_host_unregister_user_memory(entry->produce_q,
2234                                               entry->
2235                                               consume_q);
2236        else
2237                qp_host_unregister_user_memory(entry->produce_q,
2238                                               entry->
2239                                               consume_q);

The original driver code was calling:
               if (entry->vmciPageFiles) {
                       VMCIHost_ReleaseUserMemory(entry->produceQ,
                                                  entry->consumeQ);
               } else {
                       VMCIHost_UnregisterUserMemory(entry->produceQ,
                                                     entry->consumeQ);
               }
but the VMCIHost_ReleaseUserMemory equivalent function seems to not
have ever existed in the mainline Linus version of the driver so the 
differentiation no longer is needed.

The difference handled by setting vmci_page_files is old-style and
new-style VMX but the qp_host_unregister_user_memory and the there
called qp_release_pages() are not differentiating between old and
new style VMX so it does not seem to be a missing parameter and 
from the documentation it seems that the unregister function is to
be called after unmapping - so this if/else here seems to make no
sense - from code review I suspect it can be dropped but this would 
need confirmation by someone that understands the details of the 
driver.

The documentation at the top of the file also seems to be a bit out of
date/inconsistent with respect to old-style/new-style VMX (or Im just 
not understanding it).

Patch was compile checked with: x86_64_defconfig + CONFIG_VMWARE_VMCI=m

Patch is against 4.9.0-rc2 (localversion-next is next-20161028)

 drivers/misc/vmw_vmci/vmci_queue_pair.c | 10 ++--------
 1 file changed, 2 insertions(+), 8 deletions(-)

diff --git a/drivers/misc/vmw_vmci/vmci_queue_pair.c b/drivers/misc/vmw_vmci/vmci_queue_pair.c
index f84a427..6f6069c 100644
--- a/drivers/misc/vmw_vmci/vmci_queue_pair.c
+++ b/drivers/misc/vmw_vmci/vmci_queue_pair.c
@@ -2229,14 +2229,8 @@ int vmci_qp_broker_detach(struct vmci_handle handle, struct vmci_ctx *context)
 					handle.context, handle.resource,
 					result);
 
-			if (entry->vmci_page_files)
-				qp_host_unregister_user_memory(entry->produce_q,
-							       entry->
-							       consume_q);
-			else
-				qp_host_unregister_user_memory(entry->produce_q,
-							       entry->
-							       consume_q);
+			qp_host_unregister_user_memory(entry->produce_q,
+						       entry->consume_q);
 
 		}
 
-- 
2.1.4