From patchwork Thu Sep 27 13:48:52 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Peter Maydell X-Patchwork-Id: 975735 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (mailfrom) smtp.mailfrom=nongnu.org (client-ip=2001:4830:134:3::11; helo=lists.gnu.org; envelope-from=qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org; receiver=) Authentication-Results: ozlabs.org; dmarc=fail (p=none dis=none) header.from=linaro.org Received: from lists.gnu.org (lists.gnu.org [IPv6:2001:4830:134:3::11]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 42Lbms4djKz9s3Z for ; Thu, 27 Sep 2018 23:50:52 +1000 (AEST) Received: from localhost ([::1]:36026 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1g5Wgr-0003jt-DJ for incoming@patchwork.ozlabs.org; Thu, 27 Sep 2018 09:50:49 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:47183) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1g5WfK-000310-OW for qemu-devel@nongnu.org; Thu, 27 Sep 2018 09:49:16 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1g5WfB-0001qn-9I for qemu-devel@nongnu.org; Thu, 27 Sep 2018 09:49:11 -0400 Received: from orth.archaic.org.uk ([2001:8b0:1d0::2]:48662) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1g5WfA-0001lp-UB for qemu-devel@nongnu.org; Thu, 27 Sep 2018 09:49:05 -0400 Received: from pm215 by orth.archaic.org.uk with local (Exim 4.89) (envelope-from ) id 1g5Wf2-0002pv-5I; Thu, 27 Sep 2018 14:48:56 +0100 From: Peter Maydell To: qemu-devel@nongnu.org Date: Thu, 27 Sep 2018 14:48:52 +0100 Message-Id: <20180927134852.21490-1-peter.maydell@linaro.org> X-Mailer: git-send-email 2.19.0 MIME-Version: 1.0 X-detected-operating-system: by eggs.gnu.org: Genre and OS details not recognized. X-Received-From: 2001:8b0:1d0::2 Subject: [Qemu-devel] [PATCH] hw/scsi/mptendian: Avoid taking address of fields in packed structs X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Paolo Bonzini , Fam Zheng , patches@linaro.org Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: "Qemu-devel" Taking the address of a field in a packed struct is a bad idea, because it might not be actually aligned enough for that pointer type (and thus cause a crash on dereference on some host architectures). Newer versions of clang warn about this. Avoid the bug by not using the "modify in place" byte swapping functions. This patch was produced with the following simple spatch script: @@ expression E; @@ -le16_to_cpus(&E); +E = le16_to_cpu(E); @@ expression E; @@ -le32_to_cpus(&E); +E = le32_to_cpu(E); @@ expression E; @@ -le64_to_cpus(&E); +E = le64_to_cpu(E); @@ expression E; @@ -cpu_to_le16s(&E); +E = cpu_to_le16(E); @@ expression E; @@ -cpu_to_le32s(&E); +E = cpu_to_le32(E); @@ expression E; @@ -cpu_to_le64s(&E); +E = cpu_to_le64(E); followed by some minor tidying of overlong lines and bad indent. Signed-off-by: Peter Maydell Reviewed-by: Fam Zheng Reviewed-by: Philippe Mathieu-Daudé --- NB: tested with qemu-system-x86_64 -cpu host -enable-kvm -m 4096 -device mptsas1068 -drive id=mydisk,if=none,file=harddisk.qcow2 -device scsi-disk,drive=mydisk which behaves no differently before or after the patch, though the guest I have (an x86 ubuntu) seems to detect the controller but not find any disks attached to it. Maybe my command line is wrong? hw/scsi/mptendian.c | 163 ++++++++++++++++++++++---------------------- 1 file changed, 83 insertions(+), 80 deletions(-) diff --git a/hw/scsi/mptendian.c b/hw/scsi/mptendian.c index 8ae39a76f42..79f99734d21 100644 --- a/hw/scsi/mptendian.c +++ b/hw/scsi/mptendian.c @@ -35,152 +35,155 @@ static void mptsas_fix_sgentry_endianness(MPISGEntry *sge) { - le32_to_cpus(&sge->FlagsLength); + sge->FlagsLength = le32_to_cpu(sge->FlagsLength); if (sge->FlagsLength & MPI_SGE_FLAGS_64_BIT_ADDRESSING) { - le64_to_cpus(&sge->u.Address64); + sge->u.Address64 = le64_to_cpu(sge->u.Address64); } else { - le32_to_cpus(&sge->u.Address32); + sge->u.Address32 = le32_to_cpu(sge->u.Address32); } } static void mptsas_fix_sgentry_endianness_reply(MPISGEntry *sge) { if (sge->FlagsLength & MPI_SGE_FLAGS_64_BIT_ADDRESSING) { - cpu_to_le64s(&sge->u.Address64); + sge->u.Address64 = cpu_to_le64(sge->u.Address64); } else { - cpu_to_le32s(&sge->u.Address32); + sge->u.Address32 = cpu_to_le32(sge->u.Address32); } - cpu_to_le32s(&sge->FlagsLength); + sge->FlagsLength = cpu_to_le32(sge->FlagsLength); } void mptsas_fix_scsi_io_endianness(MPIMsgSCSIIORequest *req) { - le32_to_cpus(&req->MsgContext); - le32_to_cpus(&req->Control); - le32_to_cpus(&req->DataLength); - le32_to_cpus(&req->SenseBufferLowAddr); + req->MsgContext = le32_to_cpu(req->MsgContext); + req->Control = le32_to_cpu(req->Control); + req->DataLength = le32_to_cpu(req->DataLength); + req->SenseBufferLowAddr = le32_to_cpu(req->SenseBufferLowAddr); } void mptsas_fix_scsi_io_reply_endianness(MPIMsgSCSIIOReply *reply) { - cpu_to_le32s(&reply->MsgContext); - cpu_to_le16s(&reply->IOCStatus); - cpu_to_le32s(&reply->IOCLogInfo); - cpu_to_le32s(&reply->TransferCount); - cpu_to_le32s(&reply->SenseCount); - cpu_to_le32s(&reply->ResponseInfo); - cpu_to_le16s(&reply->TaskTag); + reply->MsgContext = cpu_to_le32(reply->MsgContext); + reply->IOCStatus = cpu_to_le16(reply->IOCStatus); + reply->IOCLogInfo = cpu_to_le32(reply->IOCLogInfo); + reply->TransferCount = cpu_to_le32(reply->TransferCount); + reply->SenseCount = cpu_to_le32(reply->SenseCount); + reply->ResponseInfo = cpu_to_le32(reply->ResponseInfo); + reply->TaskTag = cpu_to_le16(reply->TaskTag); } void mptsas_fix_scsi_task_mgmt_endianness(MPIMsgSCSITaskMgmt *req) { - le32_to_cpus(&req->MsgContext); - le32_to_cpus(&req->TaskMsgContext); + req->MsgContext = le32_to_cpu(req->MsgContext); + req->TaskMsgContext = le32_to_cpu(req->TaskMsgContext); } void mptsas_fix_scsi_task_mgmt_reply_endianness(MPIMsgSCSITaskMgmtReply *reply) { - cpu_to_le32s(&reply->MsgContext); - cpu_to_le16s(&reply->IOCStatus); - cpu_to_le32s(&reply->IOCLogInfo); - cpu_to_le32s(&reply->TerminationCount); + reply->MsgContext = cpu_to_le32(reply->MsgContext); + reply->IOCStatus = cpu_to_le16(reply->IOCStatus); + reply->IOCLogInfo = cpu_to_le32(reply->IOCLogInfo); + reply->TerminationCount = cpu_to_le32(reply->TerminationCount); } void mptsas_fix_ioc_init_endianness(MPIMsgIOCInit *req) { - le32_to_cpus(&req->MsgContext); - le16_to_cpus(&req->ReplyFrameSize); - le32_to_cpus(&req->HostMfaHighAddr); - le32_to_cpus(&req->SenseBufferHighAddr); - le32_to_cpus(&req->ReplyFifoHostSignalingAddr); + req->MsgContext = le32_to_cpu(req->MsgContext); + req->ReplyFrameSize = le16_to_cpu(req->ReplyFrameSize); + req->HostMfaHighAddr = le32_to_cpu(req->HostMfaHighAddr); + req->SenseBufferHighAddr = le32_to_cpu(req->SenseBufferHighAddr); + req->ReplyFifoHostSignalingAddr = + le32_to_cpu(req->ReplyFifoHostSignalingAddr); mptsas_fix_sgentry_endianness(&req->HostPageBufferSGE); - le16_to_cpus(&req->MsgVersion); - le16_to_cpus(&req->HeaderVersion); + req->MsgVersion = le16_to_cpu(req->MsgVersion); + req->HeaderVersion = le16_to_cpu(req->HeaderVersion); } void mptsas_fix_ioc_init_reply_endianness(MPIMsgIOCInitReply *reply) { - cpu_to_le32s(&reply->MsgContext); - cpu_to_le16s(&reply->IOCStatus); - cpu_to_le32s(&reply->IOCLogInfo); + reply->MsgContext = cpu_to_le32(reply->MsgContext); + reply->IOCStatus = cpu_to_le16(reply->IOCStatus); + reply->IOCLogInfo = cpu_to_le32(reply->IOCLogInfo); } void mptsas_fix_ioc_facts_endianness(MPIMsgIOCFacts *req) { - le32_to_cpus(&req->MsgContext); + req->MsgContext = le32_to_cpu(req->MsgContext); } void mptsas_fix_ioc_facts_reply_endianness(MPIMsgIOCFactsReply *reply) { - cpu_to_le16s(&reply->MsgVersion); - cpu_to_le16s(&reply->HeaderVersion); - cpu_to_le32s(&reply->MsgContext); - cpu_to_le16s(&reply->IOCExceptions); - cpu_to_le16s(&reply->IOCStatus); - cpu_to_le32s(&reply->IOCLogInfo); - cpu_to_le16s(&reply->ReplyQueueDepth); - cpu_to_le16s(&reply->RequestFrameSize); - cpu_to_le16s(&reply->ProductID); - cpu_to_le32s(&reply->CurrentHostMfaHighAddr); - cpu_to_le16s(&reply->GlobalCredits); - cpu_to_le32s(&reply->CurrentSenseBufferHighAddr); - cpu_to_le16s(&reply->CurReplyFrameSize); - cpu_to_le32s(&reply->FWImageSize); - cpu_to_le32s(&reply->IOCCapabilities); - cpu_to_le16s(&reply->HighPriorityQueueDepth); + reply->MsgVersion = cpu_to_le16(reply->MsgVersion); + reply->HeaderVersion = cpu_to_le16(reply->HeaderVersion); + reply->MsgContext = cpu_to_le32(reply->MsgContext); + reply->IOCExceptions = cpu_to_le16(reply->IOCExceptions); + reply->IOCStatus = cpu_to_le16(reply->IOCStatus); + reply->IOCLogInfo = cpu_to_le32(reply->IOCLogInfo); + reply->ReplyQueueDepth = cpu_to_le16(reply->ReplyQueueDepth); + reply->RequestFrameSize = cpu_to_le16(reply->RequestFrameSize); + reply->ProductID = cpu_to_le16(reply->ProductID); + reply->CurrentHostMfaHighAddr = cpu_to_le32(reply->CurrentHostMfaHighAddr); + reply->GlobalCredits = cpu_to_le16(reply->GlobalCredits); + reply->CurrentSenseBufferHighAddr = + cpu_to_le32(reply->CurrentSenseBufferHighAddr); + reply->CurReplyFrameSize = cpu_to_le16(reply->CurReplyFrameSize); + reply->FWImageSize = cpu_to_le32(reply->FWImageSize); + reply->IOCCapabilities = cpu_to_le32(reply->IOCCapabilities); + reply->HighPriorityQueueDepth = cpu_to_le16(reply->HighPriorityQueueDepth); mptsas_fix_sgentry_endianness_reply(&reply->HostPageBufferSGE); - cpu_to_le32s(&reply->ReplyFifoHostSignalingAddr); + reply->ReplyFifoHostSignalingAddr = + cpu_to_le32(reply->ReplyFifoHostSignalingAddr); } void mptsas_fix_config_endianness(MPIMsgConfig *req) { - le16_to_cpus(&req->ExtPageLength); - le32_to_cpus(&req->MsgContext); - le32_to_cpus(&req->PageAddress); + req->ExtPageLength = le16_to_cpu(req->ExtPageLength); + req->MsgContext = le32_to_cpu(req->MsgContext); + req->PageAddress = le32_to_cpu(req->PageAddress); mptsas_fix_sgentry_endianness(&req->PageBufferSGE); } void mptsas_fix_config_reply_endianness(MPIMsgConfigReply *reply) { - cpu_to_le16s(&reply->ExtPageLength); - cpu_to_le32s(&reply->MsgContext); - cpu_to_le16s(&reply->IOCStatus); - cpu_to_le32s(&reply->IOCLogInfo); + reply->ExtPageLength = cpu_to_le16(reply->ExtPageLength); + reply->MsgContext = cpu_to_le32(reply->MsgContext); + reply->IOCStatus = cpu_to_le16(reply->IOCStatus); + reply->IOCLogInfo = cpu_to_le32(reply->IOCLogInfo); } void mptsas_fix_port_facts_endianness(MPIMsgPortFacts *req) { - le32_to_cpus(&req->MsgContext); + req->MsgContext = le32_to_cpu(req->MsgContext); } void mptsas_fix_port_facts_reply_endianness(MPIMsgPortFactsReply *reply) { - cpu_to_le32s(&reply->MsgContext); - cpu_to_le16s(&reply->IOCStatus); - cpu_to_le32s(&reply->IOCLogInfo); - cpu_to_le16s(&reply->MaxDevices); - cpu_to_le16s(&reply->PortSCSIID); - cpu_to_le16s(&reply->ProtocolFlags); - cpu_to_le16s(&reply->MaxPostedCmdBuffers); - cpu_to_le16s(&reply->MaxPersistentIDs); - cpu_to_le16s(&reply->MaxLanBuckets); + reply->MsgContext = cpu_to_le32(reply->MsgContext); + reply->IOCStatus = cpu_to_le16(reply->IOCStatus); + reply->IOCLogInfo = cpu_to_le32(reply->IOCLogInfo); + reply->MaxDevices = cpu_to_le16(reply->MaxDevices); + reply->PortSCSIID = cpu_to_le16(reply->PortSCSIID); + reply->ProtocolFlags = cpu_to_le16(reply->ProtocolFlags); + reply->MaxPostedCmdBuffers = cpu_to_le16(reply->MaxPostedCmdBuffers); + reply->MaxPersistentIDs = cpu_to_le16(reply->MaxPersistentIDs); + reply->MaxLanBuckets = cpu_to_le16(reply->MaxLanBuckets); } void mptsas_fix_port_enable_endianness(MPIMsgPortEnable *req) { - le32_to_cpus(&req->MsgContext); + req->MsgContext = le32_to_cpu(req->MsgContext); } void mptsas_fix_port_enable_reply_endianness(MPIMsgPortEnableReply *reply) { - cpu_to_le32s(&reply->MsgContext); - cpu_to_le16s(&reply->IOCStatus); - cpu_to_le32s(&reply->IOCLogInfo); + reply->MsgContext = cpu_to_le32(reply->MsgContext); + reply->IOCStatus = cpu_to_le16(reply->IOCStatus); + reply->IOCLogInfo = cpu_to_le32(reply->IOCLogInfo); } void mptsas_fix_event_notification_endianness(MPIMsgEventNotify *req) { - le32_to_cpus(&req->MsgContext); + req->MsgContext = le32_to_cpu(req->MsgContext); } void mptsas_fix_event_notification_reply_endianness(MPIMsgEventNotifyReply *reply) @@ -188,16 +191,16 @@ void mptsas_fix_event_notification_reply_endianness(MPIMsgEventNotifyReply *repl int length = reply->EventDataLength; int i; - cpu_to_le16s(&reply->EventDataLength); - cpu_to_le32s(&reply->MsgContext); - cpu_to_le16s(&reply->IOCStatus); - cpu_to_le32s(&reply->IOCLogInfo); - cpu_to_le32s(&reply->Event); - cpu_to_le32s(&reply->EventContext); + reply->EventDataLength = cpu_to_le16(reply->EventDataLength); + reply->MsgContext = cpu_to_le32(reply->MsgContext); + reply->IOCStatus = cpu_to_le16(reply->IOCStatus); + reply->IOCLogInfo = cpu_to_le32(reply->IOCLogInfo); + reply->Event = cpu_to_le32(reply->Event); + reply->EventContext = cpu_to_le32(reply->EventContext); /* Really depends on the event kind. This will do for now. */ for (i = 0; i < length; i++) { - cpu_to_le32s(&reply->Data[i]); + reply->Data[i] = cpu_to_le32(reply->Data[i]); } }