From patchwork Wed Oct 2 09:44:56 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Juerg Haefliger X-Patchwork-Id: 1991914 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@legolas.ozlabs.org Authentication-Results: legolas.ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=lists.ubuntu.com (client-ip=185.125.189.65; helo=lists.ubuntu.com; envelope-from=kernel-team-bounces@lists.ubuntu.com; receiver=patchwork.ozlabs.org) Received: from lists.ubuntu.com (lists.ubuntu.com [185.125.189.65]) (using TLSv1.2 with cipher ECDHE-ECDSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by legolas.ozlabs.org (Postfix) with ESMTPS id 4XJVKh6YKMz1xtq for ; Wed, 2 Oct 2024 19:45:28 +1000 (AEST) Received: from localhost ([127.0.0.1] helo=lists.ubuntu.com) by lists.ubuntu.com with esmtp (Exim 4.86_2) (envelope-from ) id 1svvv6-0006Ul-HN; Wed, 02 Oct 2024 09:45:20 +0000 Received: from smtp-relay-internal-1.internal ([10.131.114.114] helo=smtp-relay-internal-1.canonical.com) by lists.ubuntu.com with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.86_2) (envelope-from ) id 1svvuv-0006N6-7d for kernel-team@lists.ubuntu.com; Wed, 02 Oct 2024 09:45:09 +0000 Received: from mail-wr1-f69.google.com (mail-wr1-f69.google.com [209.85.221.69]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by smtp-relay-internal-1.canonical.com (Postfix) with ESMTPS id A77253F1AF for ; Wed, 2 Oct 2024 09:45:08 +0000 (UTC) Received: by mail-wr1-f69.google.com with SMTP id ffacd0b85a97d-37ccc188d6aso4441264f8f.2 for ; Wed, 02 Oct 2024 02:45:08 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1727862308; x=1728467108; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=GW4ZCYL8q6gHFcl3XSZmKhwO8ew6peFUsI6v/lzBUg8=; b=PkzDsyzTjf28Pa4Xh3YSklgie4+HJds4311WXgrECUEXLsHKFa+AQDlmLcUh2wkCZH KR1RfBfPXQq56Zal6pdihdyhLMM5fgybyaTADf37uixA/NtjYA8fgdVEk85NFiX0JA+2 nXQKdrKQegNSTXPFVFTE7Jb/VX2X9zc5x9n1nl4QeC9MuvgUZ4r2fkNbfVkGnofIgCLI qiB7xoi1r8LBCw9W3z85DFnn2ExIbDIDI6vlGI8MERxRONU6fjahoukuo8orMYG2L8f6 CPqMAuaBfSjkFU/IL7if+ChBYO6fZsBylM4jvUVh3kcTdGTe42AsRnVfSFRTmzrBGoQM 02Uw== X-Gm-Message-State: AOJu0YyrVl/pirTaNcr0TxsEAwAxqcPlyU9UMH0KLTCYGTQxhr+l43gF cHu7ZKfe7ConJIxXDdN01w4kvZFqA+wX0hmcTXosL3l+VCGAegcPaWxA+jdLJP8C7h+BkV9CB7y U+9ZdcKfJ0+2XPUtsGeTQ5TMuWVrxMtLxdGLqRQ3oqcRUGB4mqRbT8GreW3E83rF4CFydFSQYJM MfXPkhQfvdcg== X-Received: by 2002:adf:ee84:0:b0:37c:d18e:6463 with SMTP id ffacd0b85a97d-37cfba0a68amr2010265f8f.47.1727862308070; Wed, 02 Oct 2024 02:45:08 -0700 (PDT) X-Google-Smtp-Source: AGHT+IFKdDERtqDii0ocuE2Gy0VynF4QyxQD8V4lOjB5cTCUfevJu+w4C2SovfpkbSUswGYInnvy3g== X-Received: by 2002:adf:ee84:0:b0:37c:d18e:6463 with SMTP id ffacd0b85a97d-37cfba0a68amr2010248f8f.47.1727862307646; Wed, 02 Oct 2024 02:45:07 -0700 (PDT) Received: from localhost ([81.221.247.52]) by smtp.gmail.com with ESMTPSA id 5b1f17b1804b1-42f79fc9032sm13837005e9.25.2024.10.02.02.45.06 for (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 02 Oct 2024 02:45:07 -0700 (PDT) From: Juerg Haefliger To: kernel-team@lists.ubuntu.com Subject: [SRU][N][PATCH 5/6] scsi: aacraid: Rearrange order of struct aac_srb_unit Date: Wed, 2 Oct 2024 11:44:56 +0200 Message-ID: <20241002094457.1777904-6-juerg.haefliger@canonical.com> X-Mailer: git-send-email 2.43.0 In-Reply-To: <20241002094457.1777904-1-juerg.haefliger@canonical.com> References: <20241002094457.1777904-1-juerg.haefliger@canonical.com> MIME-Version: 1.0 X-BeenThere: kernel-team@lists.ubuntu.com X-Mailman-Version: 2.1.20 Precedence: list List-Id: Kernel team discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: kernel-team-bounces@lists.ubuntu.com Sender: "kernel-team" From: Kees Cook BugLink: https://bugs.launchpad.net/bugs/2078038 struct aac_srb_unit contains struct aac_srb, which contains struct sgmap, which ends in a (currently) "fake" (1-element) flexible array. Converting this to a flexible array is needed so that runtime bounds checking won't think the array is fixed size (i.e. under CONFIG_FORTIFY_SOURCE=y and/or CONFIG_UBSAN_BOUNDS=y), as other parts of aacraid use struct sgmap as a flexible array. It is not legal to have a flexible array in the middle of a structure, so it either needs to be split up or rearranged so that it is at the end of the structure. Luckily, struct aac_srb_unit, which is exclusively consumed/updated by aac_send_safw_bmic_cmd(), does not depend on member ordering. The values set in the on-stack struct aac_srb_unit instance "srbu" by the only two callers, aac_issue_safw_bmic_identify() and aac_get_safw_ciss_luns(), do not contain anything in srbu.srb.sgmap.sg, and they both implicitly initialize srbu.srb.sgmap.count to 0 during memset(). For example: memset(&srbu, 0, sizeof(struct aac_srb_unit)); srbcmd = &srbu.srb; srbcmd->flags = cpu_to_le32(SRB_DataIn); srbcmd->cdb[0] = CISS_REPORT_PHYSICAL_LUNS; srbcmd->cdb[1] = 2; /* extended reporting */ srbcmd->cdb[8] = (u8)(datasize >> 8); srbcmd->cdb[9] = (u8)(datasize); rcode = aac_send_safw_bmic_cmd(dev, &srbu, phys_luns, datasize); During aac_send_safw_bmic_cmd(), a separate srb is mapped into DMA, and has srbu.srb copied into it: srb = fib_data(fibptr); memcpy(srb, &srbu->srb, sizeof(struct aac_srb)); Only then is srb.sgmap.count written and srb->sg populated: srb->count = cpu_to_le32(xfer_len); sg64 = (struct sgmap64 *)&srb->sg; sg64->count = cpu_to_le32(1); sg64->sg[0].addr[1] = cpu_to_le32(upper_32_bits(addr)); sg64->sg[0].addr[0] = cpu_to_le32(lower_32_bits(addr)); sg64->sg[0].count = cpu_to_le32(xfer_len); But this is happening in the DMA memory, not in srbu.srb. An attempt to copy the changes back to srbu does happen: /* * Copy the updated data for other dumping or other usage if * needed */ memcpy(&srbu->srb, srb, sizeof(struct aac_srb)); But this was never correct: the sg64 (3 u32s) overlap of srb.sg (2 u32s) always meant that srbu.srb would have held truncated information and any attempt to walk srbu.srb.sg.sg based on the value of srbu.srb.sg.count would result in attempting to parse past the end of srbu.srb.sg.sg[0] into srbu.srb_reply. After getting a reply from hardware, the reply is copied into srbu.srb_reply: srb_reply = (struct aac_srb_reply *)fib_data(fibptr); memcpy(&srbu->srb_reply, srb_reply, sizeof(struct aac_srb_reply)); This has always been fixed-size, so there's no issue here. It is worth noting that the two callers _never check_ srbu contents -- neither srbu.srb nor srbu.srb_reply is examined. (They depend on the mapped xfer_buf instead.) Therefore, the ordering of members in struct aac_srb_unit does not matter, and the flexible array member can moved to the end. (Additionally, the two memcpy()s that update srbu could be entirely removed as they are never consumed, but I left that as-is.) Signed-off-by: Kees Cook Link: https://lore.kernel.org/r/20240711215739.208776-1-kees@kernel.org Signed-off-by: Martin K. Petersen (cherry picked from commit 6e5860b0ad4934baee8c7a202c02033b2631bb44) Signed-off-by: Juerg Haefliger --- drivers/scsi/aacraid/aacraid.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/scsi/aacraid/aacraid.h b/drivers/scsi/aacraid/aacraid.h index 6f0417f6f8a1..8e7a0a5cb7aa 100644 --- a/drivers/scsi/aacraid/aacraid.h +++ b/drivers/scsi/aacraid/aacraid.h @@ -2029,8 +2029,8 @@ struct aac_srb_reply }; struct aac_srb_unit { - struct aac_srb srb; struct aac_srb_reply srb_reply; + struct aac_srb srb; }; /*