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:     Eric Biggers <ebiggers@kernel.org>,
        Herbert Xu <herbert@gondor.apana.org.au>
CC:     "tim.c.chen@linux.intel.com" <tim.c.chen@linux.intel.com>,
        "davem@davemloft.net" <davem@davemloft.net>,
        "linux-crypto@vger.kernel.org" <linux-crypto@vger.kernel.org>,
        "Kani, Toshi" <toshi.kani@hpe.com>,
        "Wright, Randy (HPE Servers Linux)" <rwright@hpe.com>
Subject: RE: [PATCH] crypto: testmgr - don't generate WARN for missing modules
Thread-Topic: [PATCH] crypto: testmgr - don't generate WARN for missing
 modules
Thread-Index: AQHYr2qKvFfENN2UYkqyv18Pmvb4G622GGiAgADL2ACAAAvpsA==
Date:   Sat, 20 Aug 2022 00:15:41 +0000
Message-ID: <MW5PR84MB1842130E7FA4D064D50DC1EDAB6F9@MW5PR84MB1842.NAMPRD84.PROD.OUTLOOK.COM>
References: <20220813231443.2706-1-elliott@hpe.com>
 <Yv9uhQY7UAPN7QDE@gondor.apana.org.au> <YwAZhFLCrlHXegr9@sol.localdomain>
In-Reply-To: <YwAZhFLCrlHXegr9@sol.localdomain>
Accept-Language: en-US
Content-Language: en-US
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?HNgLlOPgVyKB+0dP2JZPBpav1TuHeWgHWRwUQLSQ0jbDjXEmPxIIfNsfs5vU?=
 =?us-ascii?Q?ju1zsPLn8u+WsSerxGVOxqAngc0zKx3X6oeWZu77gn9d+Zdb7G3oL8yn6D1J?=
 =?us-ascii?Q?nqotxJi0kTcZlNgeGwa0slFBfiFMYsXwbTB5SZqP2i0oshBifrrEqdulQ9+Y?=
 =?us-ascii?Q?2a2Ds0du0ivxDJn+jw23h/tOLKAvuMloSBYfVoG55y9C8pQoVG/i4kyVerKa?=
 =?us-ascii?Q?ySfQ3LxD4VKHNcmiwrtuFUmiyBBg5WXPARESx/7g8uMolv+ig8qvKnVo4CNU?=
 =?us-ascii?Q?u9+DfQvtVCO6UbMUk6kJFaufG8t+K5rI+NzOkoqKx2vdjWMEMJ234LURLUid?=
 =?us-ascii?Q?ljErsFVOwPx8xhrF3SfnCgr2O12m/RAtzjY1Br50k8KdRkKeiaXFL3Bzs1EZ?=
 =?us-ascii?Q?BROH1c2bzOky1RTWpB7rQZ5CWozXvsoJDb95N4YupXsWnEf7fMCavfwTnIti?=
 =?us-ascii?Q?sXEYgJTfR32b+kButYgMhIAYSP4gNNvJZLA2nT7P1K4dUERO5gpXrzSI9Dbi?=
 =?us-ascii?Q?gBV2EWFlGHTknk3Jt0YFanE49Kj/jXXDaIaxeVn3aGOSr/W8GuBCGaef8HwO?=
 =?us-ascii?Q?x9gdPoVLa5slPMl4yt5PEz7961AB5/5DSV6usOfiWSnsKX2lPTRPsKQnq4dc?=
 =?us-ascii?Q?dWd3NE0Yrokae6MGyfY2egN4ku1SmQTSvrEajpHFnOaxk7hCJZSDoqn6pImd?=
 =?us-ascii?Q?kVK9HFDPLJdTqyvhkG+WjyYQLy8EFV32zW64tfSFacKcVym3UeDs1pq0REkL?=
 =?us-ascii?Q?cfmlyX49K7EqF2CG1QPm2/VcV1z86kDKasn5a2XZ7cXQ0OajfxSEIWKCq1NP?=
 =?us-ascii?Q?NVJMyLpb+KoJ3SiRgocrDYFnwV6vqHqGTBrtevsfYjHUV02uMt9T9aE7ohPx?=
 =?us-ascii?Q?MTK89YiSAzVt8NbUZtJBc24nwxwuq73aJssUJJK8P6b0ojtDxp+7GUNt4Qzb?=
 =?us-ascii?Q?qIwa5UFXD1zkHmh9my45h2rM0UbjBM/oyoJjlZB4+lbOjNrfr7NVa4B1REdZ?=
 =?us-ascii?Q?yb2GRjR0oavLW+aLBC5rWkdpYGtrvpjlaTfmHymdppvH49daSosDCboppxjL?=
 =?us-ascii?Q?osnjpDhGGEHAYuYAAbKLt/Svek5hSDKg4VT5FYDa3KYfCvDmVkGD+FVLxm98?=
 =?us-ascii?Q?spEYhs3IqtrjHKqjFolwiosnY8jnVB/8kL/z1VMWtmjkCPwBRflxZX8im8cM?=
 =?us-ascii?Q?ID+Y7yQOHgmyDT/bT2OOP4T/8+WHwN5Aj8lzwq2VCVhdpFgbedC8c/wBTwOH?=
 =?us-ascii?Q?sOseiWAj4P6ctKoXN/6BPHVF3c0bxKOJWt+zjD9qrQHkLHXIbs4fmSLpV5B/?=
 =?us-ascii?Q?SPgoyj6qXlsXMIJ5fAalwxLRBtuMAaHNZ/DeGOITuNu3WA0eaNWhERleptTS?=
 =?us-ascii?Q?FZitKW64dy41lTLcIuvvwFXwD04caGJKsdcUvmNiIyr4ZA0tR3K860tbuO1P?=
 =?us-ascii?Q?wkpE13nxQKo6tzfIEU/AsVkmxWc8Cz5v8GvwyP6+3rm/FL7FkHW+p/rHMY0V?=
 =?us-ascii?Q?LJsMuzFOXhWxErO8u1Ng1eWQacxEYZ5480QYmKjL5f2FqPlp0q/kPQXK6I34?=
 =?us-ascii?Q?x72TDnweReHrHO8zm80=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: ab248840-64be-4771-e57d-08da82411dfe
X-MS-Exchange-CrossTenant-originalarrivaltime: 20 Aug 2022 00:15:41.3763
 (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: ure1+DnjmpMNa7ZgSLkeiRmkY3mnnJxvCr0PMT3oJGs0DTZbIR+3/JoR41prHnzf
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM4PR84MB1544
X-OriginatorOrg: hpe.com
X-Proofpoint-GUID: 1YQFHIgpsGEP50Wd6mS7bxAaJbxuxG2U
X-Proofpoint-ORIG-GUID: 1YQFHIgpsGEP50Wd6mS7bxAaJbxuxG2U
X-HPE-SCL: -1
X-Proofpoint-Virus-Version: vendor=baseguard
 engine=ICAP:2.0.205,Aquarius:18.0.895,Hydra:6.0.517,FMLib:17.11.122.1
 definitions=2022-08-19_13,2022-08-18_01,2022-06-22_01
X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 clxscore=1015 adultscore=0
 suspectscore=0 malwarescore=0 mlxscore=0 spamscore=0 mlxlogscore=999
 impostorscore=0 phishscore=0 lowpriorityscore=0 priorityscore=1501
 bulkscore=0 classifier=spam adjust=0 reason=mlx scancount=1
 engine=8.12.0-2207270000 definitions=main-2208190091
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,
        RCVD_IN_MSPIKE_H2,SPF_HELO_NONE,SPF_NONE,T_SCC_BODY_TEXT_LINE
        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: Eric Biggers <ebiggers@kernel.org>
> Sent: Friday, August 19, 2022 6:15 PM
> To: Herbert Xu <herbert@gondor.apana.org.au>
> Cc: Elliott, Robert (Servers) <elliott@hpe.com>;
> tim.c.chen@linux.intel.com; davem@davemloft.net; linux-
> crypto@vger.kernel.org; Kani, Toshi <toshi.kani@hpe.com>; Wright, Randy
> (HPE Servers Linux) <rwright@hpe.com>
> Subject: Re: [PATCH] crypto: testmgr - don't generate WARN for missing
> modules
>=20
> On Fri, Aug 19, 2022 at 07:05:41PM +0800, Herbert Xu wrote:
> > Robert Elliott <elliott@hpe.com> wrote:
> > > This userspace command:
> > >    modprobe tcrypt
> > > or
> > >    modprobe tcrypt mode=3D0
> > >
> > > runs all the tcrypt test cases numbered <200 (i.e., all the
> > > test cases calling tcrypt_test() and returning return values).
> > >
> > > Tests are sparsely numbered from 0 to 1000. For example:
> > >    modprobe tcrypt mode=3D12
> > > tests sha512, and
> > >    modprobe tcrypt mode=3D152
> > > tests rfc4543(gcm(aes))) - AES-GCM as GMAC
> > >
> > > The test manager generates WARNING crashdumps every time it
> attempts
> > > a test using an algorithm that is not available (not built-in to
> the
> > > kernel or available as a module):
> > >
> > >    alg: skcipher: failed to allocate transform for ecb(arc4): -2
> > >    ------------[ cut here ]-----------
> > >    alg: self-tests for ecb(arc4) (ecb(arc4)) failed (rc=3D-2)
> > >    WARNING: CPU: 9 PID: 4618 at crypto/testmgr.c:5777
> > > alg_test+0x30b/0x510
> > >    [50 more lines....]
> > >
> > >    ---[ end trace 0000000000000000 ]---
> > >
> > > If the kernel is compiled with CRYPTO_USER_API_ENABLE_OBSOLETE
> > > disabled (the default), then these algorithms are not compiled into
> > > the kernel or made into modules and trigger WARNINGs:
> > >    arc4 tea xtea khazad anubis xeta seed
> > >
> > > Additionally, any other algorithms that are not enabled in .config
> > > will generate WARNINGs. In RHEL 9.0, for example, the default
> > > selection of algorithms leads to 16 WARNING dumps.
> > >
> > > One attempt to fix this was by modifying tcrypt_test() to check
> > > crypto_has_alg() and immediately return 0 if crypto_has_alg()
> fails,
> > > rather than proceed and return a non-zero error value that causes
> > > the caller (alg_test() in crypto/testmgr.c) to invoke WARN().
> > > That knocks out too many algorithms, though; some combinations
> > > like ctr(des3_ede) would work.
> > >
> > > Instead, change the condition on the WARN to ignore a return
> > > value is ENOENT, which is the value returned when the algorithm
> > > or combination of algorithms doesn't exist. Add a pr_warn to
> > > communicate that information in case the WARN is skipped.
> > >
> > > This approach allows algorithm tests to work that are combinations,
> > > not provided by one driver, like ctr(blowfish).
> > >
> > > Result - no more WARNINGs:
> > > modprobe tcrypt
> > > [  115.541765] tcrypt: testing md5
> > > [  115.556415] tcrypt: testing sha1
> > > [  115.570463] tcrypt: testing ecb(des)
> > > [  115.585303] cryptomgr: alg: skcipher: failed to allocate
> transform for ecb(des): -2
> > > [  115.593037] cryptomgr: alg: self-tests for ecb(des) using
> ecb(des) failed (rc=3D-2)
> > > [  115.593038] tcrypt: testing cbc(des)
> > > [  115.610641] cryptomgr: alg: skcipher: failed to allocate
> transform for cbc(des): -2
> > > [  115.618359] cryptomgr: alg: self-tests for cbc(des) using
> cbc(des) failed (rc=3D-2)
> > > ...
> > >
> > > Signed-off-by: Robert Elliott <elliott@hpe.com>
> > > ---
> > > crypto/testmgr.c | 7 +++++--
> > > 1 file changed, 5 insertions(+), 2 deletions(-)
> >
> > Patch applied.  Thanks.
>=20
> I thought the conclusion from the discussion was that this should
> instead be solved by a tcrypt change?  Either dropping the enumerative
> testing support from tcrypt, or making tcrypt just try to allocate the
> algorithms (relying on the registration-time self-tests) rather than
> call alg_test() directly.
>=20
> - Eric

Per Stephan, it sounds like this was a hacky way to get some/most of
the modules loaded.

It'd be good if there was a way to run all registered tests on all
available modules, not just the ones that someone remembered to put
in tcrypt.c.

I do worry this WARN() isn't really helpful even for real self-test
failures - it's dumping the call trace to alg_test(), not the
trace to whatever crypto function alg_test called that is failing.=20
With Linus always expressing concern with too many BUG and WARN
calls, it might be better as just pr_warn() or pr_err().