Received-SPF: pass (google.com: domain of linux-crypto-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
From:   "Elliott, Robert (Servers)" <elliott@hpe.com>
To:     Taehee Yoo <ap420073@gmail.com>,
        "linux-crypto@vger.kernel.org" <linux-crypto@vger.kernel.org>,
        "herbert@gondor.apana.org.au" <herbert@gondor.apana.org.au>,
        "davem@davemloft.net" <davem@davemloft.net>,
        "tglx@linutronix.de" <tglx@linutronix.de>,
        "mingo@redhat.com" <mingo@redhat.com>,
        "bp@alien8.de" <bp@alien8.de>,
        "dave.hansen@linux.intel.com" <dave.hansen@linux.intel.com>,
        "hpa@zytor.com" <hpa@zytor.com>,
        "kirill.shutemov@linux.intel.com" <kirill.shutemov@linux.intel.com>,
        "richard@nod.at" <richard@nod.at>,
        "viro@zeniv.linux.org.uk" <viro@zeniv.linux.org.uk>,
        "sathyanarayanan.kuppuswamy@linux.intel.com" 
        <sathyanarayanan.kuppuswamy@linux.intel.com>,
        "jpoimboe@kernel.org" <jpoimboe@kernel.org>,
        "x86@kernel.org" <x86@kernel.org>,
        "jussi.kivilinna@iki.fi" <jussi.kivilinna@iki.fi>
Subject: RE: [PATCH v3 2/4] crypto: aria: do not use magic number offsets of
 aria_ctx
Thread-Topic: [PATCH v3 2/4] crypto: aria: do not use magic number offsets of
 aria_ctx
Thread-Index: AQHY8e1gHlUTGybXLkCClZ3eR7ihfK43cq3g
Date:   Thu, 10 Nov 2022 03:55:40 +0000
Message-ID: <MW5PR84MB18422C6DB5DDFAE158BAF459AB019@MW5PR84MB1842.NAMPRD84.PROD.OUTLOOK.COM>
References: <20221106143627.30920-1-ap420073@gmail.com>
 <20221106143627.30920-3-ap420073@gmail.com>
In-Reply-To: <20221106143627.30920-3-ap420073@gmail.com>
Accept-Language: en-US
Content-Language: en-US
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?k1NN74SBz1+1a7HA3fIEbjFryY/QuxnCoWXV2+FAFADSsiY56X+wD8qppLRu?=
 =?us-ascii?Q?zJYjC27GRpstNU3QXV80fEAXc7doVm44I4mevkSqQLvhtR1DHHuTmaBxB/8X?=
 =?us-ascii?Q?QjNFnxh/jFydxfEd1/1Q+8tk3kyX8moaBFAzieY7KAzP+26USnFB0LNThv9o?=
 =?us-ascii?Q?ULLVExGMZv5jvM/n1dkDYc2BzQMwxuW5zfNAqq6Y6+ZUZHJlM9Oxqnj58Wtr?=
 =?us-ascii?Q?Es6LPNWiXLcSxtRvOByGYnS+KBF/mU67xUrH8ad7cU5RjQ/12gpBnx+p2ndM?=
 =?us-ascii?Q?uM1mvNap1vqe6cU2N2tgnc7Eu1/6J0slseoBZObiXmuThna63iqd8smjNOBS?=
 =?us-ascii?Q?TQjh7oxyvGFzvWL0ITW3FNbm35RX7//YegLNutL0rtIoAoG9wP9rvi/MRjkJ?=
 =?us-ascii?Q?GG0Wi1+8CJFw03bURk+sQV0iqg0r3rs+xR0TCfDzIrZ4fFJdZZjZG8R38l6c?=
 =?us-ascii?Q?SZF5dYC+PoFh5lQzR61RUU01NuG3Am4mrFXXjMUAkAn4eX/aK8rWoXfj4oix?=
 =?us-ascii?Q?fICX+aTa60hRh8fS4YpilaGBAko0Ql6DiOOUMjA5ozAAxrCm+hBJFHpHrf1/?=
 =?us-ascii?Q?HNi4xGZw9m1rkzl5TMAkc+Fe2dfHvOISCI3CdncD2vv+lM2bEqj/kAm14B/n?=
 =?us-ascii?Q?63qEX4HAe/C2wbNhHz+hIxJ6MmBhaFuyS4YEwEaFh6kpGG8Kvmq8sy6uAuGH?=
 =?us-ascii?Q?Y/jOzC7vXWAGL97hEyxlPKiH/pTEbWLiX75AWn7+nKjz6kiCxzCPsRAKx6mx?=
 =?us-ascii?Q?ytrtnDSGCbCSPIU3I/IRDqcaUMvN0WteQ/wWz+BUeONTLDhkdKgTk2pmr624?=
 =?us-ascii?Q?Xh5iBrvpZoxHLwG9oJtu620BffM14QnCVoryaWio96EIXDDgMf5nLE96oppX?=
 =?us-ascii?Q?O6jm2PkShSMINNL6cS5/URlR35TuD33Q8K9JAhau/hGlH/FCUMhhoVyo+oOM?=
 =?us-ascii?Q?JRfZE0TBk0X850AvAYIZMh5dmsiyn9rKP/qG/wlPpRW6viVpEYXlIIw0dap8?=
 =?us-ascii?Q?+Nngbwjcv6QdBTXCaZFtOk70NI5RUoYmOATQosEWVcC74dk6d7lqqlYrqLg1?=
 =?us-ascii?Q?tHlPTJkanRyPyp5FJwX6NulnYtOh7h1ZbFFf7oygvg+xNp6f6lVQG1uTI9BQ?=
 =?us-ascii?Q?SQLePsIaFVRu4SqcbYgWIvyukNqI6+m8D4oif7rYlJGM9xDliUROJXJ3VW5l?=
 =?us-ascii?Q?s9HNWDo3wM5QOsV1CVetB6wlk10lLNKuPMoQ7OCB+WfjtAqC5EaWZB3Aqpcw?=
 =?us-ascii?Q?LuX2fGIToHvSpsNe7ZTqyG10RmfZ2eHZlp9KFSMLy5oL6AOVjPwnKORBMSXS?=
 =?us-ascii?Q?PK3RtTiRBZeF73U57RDiPC0UZ1lOCY0p9HwBrq1gvc2VSgIyVzrtA/WqAZR5?=
 =?us-ascii?Q?BLPCjhGJcH7dDM22iS6IUgjE+BnquHbfCFIL1wmJ++TleCGscUQZlze5vjux?=
 =?us-ascii?Q?sMUw6+Wx+ICaSMh7w+7NeU8HCU/j9sUGzYfPnjRl2KsSVLJdu831cmm/i0wQ?=
 =?us-ascii?Q?WfmXB8dBe866fTk/wIUEPx0zeIlEtbDOb3P9fIOTSfukziohmKsvyZi/Tfkm?=
 =?us-ascii?Q?2q2tgTHkTRRZUF2XHYE=3D?=
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-AuthSource: MW5PR84MB1842.NAMPRD84.PROD.OUTLOOK.COM
X-MS-Exchange-CrossTenant-Network-Message-Id: c144279a-5331-410c-4593-08dac2cf6eeb
X-MS-Exchange-CrossTenant-originalarrivaltime: 10 Nov 2022 03:55:40.0809
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 105b2061-b669-4b31-92ac-24d304d195dc
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: 8Sr4Wh3FLATAr0npxiea6FQVQXY7B+saoxVeOsFSKMDPlyDNjuuuXi7dadL0m5ON
X-MS-Exchange-Transport-CrossTenantHeadersStamped: MW5PR84MB1570
X-OriginatorOrg: hpe.com
X-Proofpoint-ORIG-GUID: cL9aAuWvGpxa0jza8dxh_ms1GosHt4f1
X-Proofpoint-GUID: cL9aAuWvGpxa0jza8dxh_ms1GosHt4f1
X-HPE-SCL: -1
X-Proofpoint-Virus-Version: vendor=baseguard
 engine=ICAP:2.0.219,Aquarius:18.0.895,Hydra:6.0.545,FMLib:17.11.122.1
 definitions=2022-11-09_06,2022-11-09_01,2022-06-22_01
X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 impostorscore=0 mlxscore=0
 bulkscore=0 phishscore=0 clxscore=1011 priorityscore=1501 adultscore=0
 suspectscore=0 malwarescore=0 spamscore=0 lowpriorityscore=0
 mlxlogscore=999 classifier=spam adjust=0 reason=mlx scancount=1
 engine=8.12.0-2210170000 definitions=main-2211100028
X-Spam-Status: No, score=-2.8 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,
        DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_LOW,
        SPF_HELO_NONE,SPF_NONE autolearn=ham autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-crypto.vger.kernel.org>
X-Mailing-List: linux-crypto@vger.kernel.org


> -----Original Message-----
> From: Taehee Yoo <ap420073@gmail.com>
> Sent: Sunday, November 6, 2022 8:36 AM
> To: linux-crypto@vger.kernel.org; herbert@gondor.apana.org.au;
> davem@davemloft.net; tglx@linutronix.de; mingo@redhat.com; bp@alien8.de;
> dave.hansen@linux.intel.com; hpa@zytor.com;
> kirill.shutemov@linux.intel.com; richard@nod.at; viro@zeniv.linux.org.uk;
> sathyanarayanan.kuppuswamy@linux.intel.com; jpoimboe@kernel.org; Elliott,
> Robert (Servers) <elliott@hpe.com>; x86@kernel.org; jussi.kivilinna@iki.f=
i
> Cc: ap420073@gmail.com
> Subject: [PATCH v3 2/4] crypto: aria: do not use magic number offsets of
> aria_ctx
>=20
> aria-avx assembly code accesses members of aria_ctx with magic number
> offset. If the shape of struct aria_ctx is changed carelessly,
> aria-avx will not work.
> So, we need to ensure accessing members of aria_ctx with correct
> offset values, not with magic numbers.
>=20
> It adds ARIA_CTX_enc_key, ARIA_CTX_dec_key, and ARIA_CTX_rounds in the
> asm-offsets.c So, correct offset definitions will be generated.
> aria-avx assembly code can access members of aria_ctx safely with
> these definitions.
>=20
> diff --git a/arch/x86/crypto/aria-aesni-avx-asm_64.S
...
>=20
>  #include <linux/linkage.h>
>  #include <asm/frame.h>
> -
> -/* struct aria_ctx: */
> -#define enc_key 0
> -#define dec_key 272
> -#define rounds 544

That structure also has a key_length field after the rounds field.
aria_set_key() sets it, but no function ever seems to use it.
Perhaps that field should be removed?

> +#include <asm/asm-offsets.h>

That makes the offsets flexible, which is good.

The assembly code also implicitly assumes the size of each of those fields
(e.g., enc_key is 272 bytes, dec_key is 272 bytes, and rounds is 4 bytes).
A BUILD_BUG_ON confirming those assumptions might be worthwhile.

> diff --git a/arch/x86/kernel/asm-offsets.c b/arch/x86/kernel/asm-offsets.=
c
> index cb50589a7102..32192a91c65b 100644
> --- a/arch/x86/kernel/asm-offsets.c
> +++ b/arch/x86/kernel/asm-offsets.c
> @@ -7,6 +7,7 @@
>  #define COMPILE_OFFSETS
>=20
>  #include <linux/crypto.h>
> +#include <crypto/aria.h>

Is it safe to include .h files for a large number of crypto modules
in one C file? It seems like they could easily include naming conflicts.=20

However, this set does seem to compile cleanly:

+// no .h for aegis, but ctx is simple
+#include <crypto/aes.h>
+#include <crypto/aria.h>
+#include <crypto/blake2s.h>
+#include <crypto/blowfish.h>
+// no .h for camellia in crypto; it is in arch/x86/crypto
+#include <crypto/cast5.h>
+#include <crypto/cast6.h>
+#include <crypto/chacha.h>
+// no .h for crc32c, crc32, crct10dif, but no ctx structure to worry about
+#include <crypto/curve25519.h>
+#include <crypto/des.h>
+#include <crypto/ghash.h>
+#include <crypto/nhpoly1305.h>
+#include <crypto/poly1305.h>
+#include <crypto/polyval.h>
+#include <crypto/serpent.h>
+#include <crypto/sha1.h>
+#include <crypto/sha2.h>
+#include <crypto/sm3.h>
+#include <crypto/twofish.h>