Received: by 2002:a25:31c3:0:0:0:0:0 with SMTP id x186csp1847523ybx; Thu, 7 Nov 2019 18:28:41 -0800 (PST) X-Google-Smtp-Source: APXvYqwEjycRfM7d1QFh2n2NS5StoG94MSrFeQcn4XPDU9MQSSKYBT1WZb/yaCNTksfAuA/2GN3L X-Received: by 2002:a05:6402:1718:: with SMTP id y24mr7410754edu.220.1573180121739; Thu, 07 Nov 2019 18:28:41 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1573180121; cv=none; d=google.com; s=arc-20160816; b=hibL3j38rh3M1SOGhp0VG2PkA6AGTVmkbmxuDFsfKppWfOitlGC3C+0jzynp9PSBPu qGxPxZ8RLjYUNOyywe1zw3guXsgT27bBShK8hWgIAfUB7Wq2mvP+8fHWtEvWM6FH6VV+ syAFNX3mdMGi6u6whBx3bSQgjzWdEVpMSsFZu3mzpxiHVPH7YFe6PH24OwW6bEdpzzVd n2fj4GQ1v/x3mgDF3FEP5d2grK3ZBpY6dsJFI0xLiEHHMy9qONsIKsZQbxBwZc5HNfDB pvs0hijs3cYvwsUIksjLNxd6UVZvJ+77dt+Jv5HtEVFy3X9hjYyeW+2Bb3LrpH+QAu0n Dpcg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:sender:user-agent:in-reply-to :content-disposition:mime-version:references:mail-followup-to :message-id:subject:cc:to:from:date:dkim-signature; bh=Mk6vEPBW4weF0VEF2D4tL+e2MzEphRSBql+jUXf7pdM=; b=zQNqYFGSEaVIto/caVKgEJIdoHPcbsq7hFkW7h5zjtK/0yKKF8qIChEJaAgPMxlKdZ +Ll2HzsXL4WZ6e9g95IaCKmWD70BHlRQrUnXEgDCfu/wIjEdyaSPpIOaWpc0uZ95OdEB PR+i+69SXA4wwjKQafbFovHPn36feOfWPi8Wu0ARKfcrKr5mvfo11/cB75Tao+nl8Zs8 4KWn55mBQYJuqetpPhcO7ocPloQZ+OWTyFJ+9kbMMwrSl1AeTzGvCdSRrsKsMMrKFLv8 RPRYzQWX42sOL/ghFkjAJMq5urCm4nKjsqaIkrVfxKox11xL9lIBXuRN+roFudqtXkHK WJsw== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@kernel.org header.s=default header.b=u6eJJ8Fp; spf=pass (google.com: best guess record for domain of linux-crypto-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-crypto-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=kernel.org Return-Path: Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67]) by mx.google.com with ESMTP id h5si3258406edj.25.2019.11.07.18.28.13; Thu, 07 Nov 2019 18:28:41 -0800 (PST) Received-SPF: pass (google.com: best guess record for domain of linux-crypto-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67; Authentication-Results: mx.google.com; dkim=pass header.i=@kernel.org header.s=default header.b=u6eJJ8Fp; spf=pass (google.com: best guess record for domain of linux-crypto-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-crypto-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1725940AbfKHC2D (ORCPT + 99 others); Thu, 7 Nov 2019 21:28:03 -0500 Received: from mail.kernel.org ([198.145.29.99]:42744 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1725928AbfKHC2C (ORCPT ); Thu, 7 Nov 2019 21:28:02 -0500 Received: from sol.localdomain (c-24-5-143-220.hsd1.ca.comcast.net [24.5.143.220]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPSA id 2E9542084D; Fri, 8 Nov 2019 02:28:01 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1573180081; bh=EmadM+X2aXhTG5ipw+1vGj1uDpsQx+J0azDRdE/N1n8=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=u6eJJ8FpsQt7K8Hxqi9SRqDIST9r/gmbGjyImf5IHFmdYMfbjw+YhhgJRmd7iVvh3 0JFyIvzd6fiP4D7fESS93YrE4M2DFe5j0n1En7xFqhCjB/fp8YlbSJGGNbYLSrBPLV 7t71NOkw6f+iXej4O35JBOPS3GfXZaAq1mUfhlCk= Date: Thu, 7 Nov 2019 18:27:59 -0800 From: Eric Biggers To: Gilad Ben-Yossef Cc: Tero Kristo , Herbert Xu , David Miller , Linux Crypto Mailing List , Ard Biesheuvel , linux-omap@vger.kernel.org, Linux ARM Subject: Re: [PATCH 09/10] crypto: add timeout to crypto_wait_req Message-ID: <20191108022759.GB1140@sol.localdomain> Mail-Followup-To: Gilad Ben-Yossef , Tero Kristo , Herbert Xu , David Miller , Linux Crypto Mailing List , Ard Biesheuvel , linux-omap@vger.kernel.org, Linux ARM References: <20191017122549.4634-1-t-kristo@ti.com> <20191017122549.4634-10-t-kristo@ti.com> <076f0bc6-ad04-9543-db02-d7c7060db036@ti.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.12.2 (2019-09-21) Sender: linux-crypto-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-crypto@vger.kernel.org On Wed, Nov 06, 2019 at 09:33:20AM +0200, Gilad Ben-Yossef wrote: > On Wed, Nov 6, 2019 at 9:25 AM Tero Kristo wrote: > > > > On 06/11/2019 08:39, Gilad Ben-Yossef wrote: > > > Hi, > > > > > > > > > On Thu, Oct 17, 2019 at 3:26 PM Tero Kristo wrote: > > >> > > >> Currently crypto_wait_req waits indefinitely for an async crypto request > > >> to complete. This is bad as it can cause for example the crypto test > > >> manager to hang without any notification as to why it has happened. > > >> Instead of waiting indefinitely, add a 1 second timeout to the call, > > >> and provide a warning print if a timeout happens. > > > > > > While the incentive is clear and positive, this suggested solution > > > creates problems of its own. > > > In many (most?) cases where we are waiting here, we are waiting for a > > > DMA operation to finish from hardware. > > > Exiting while this pending DMA operation is not finished, even with a > > > proper error return value, is dangerous because > > > unless the calling code takes great care to not release the memory the > > > DMA is being done from/to, this can have disastrous effects. > > > > > > As Eric has already mentioned, one second might seem like a long time, > > > but we don't really know if it is enough. > > > > > > How about adding a second API (ig. crypto_wait_req_timeout) which > > > supports a calee specified timeout where > > > the calle knows how to correctly deal with timeout and port the > > > relevant call sites to use this? > > > > Yeah, that would work for me. I guess we could just swap the testmgr to > > use this timeout API, as it is quite clear it should timeout rather than > > wait indefinitely, and afaics, the data buffers it uses are limited > > size. It doesn't really matter for it whether the timeout is 1 second or > > 10 seconds, as long as it eventually times out. > > > As long as you avoid releasing the memory used on timeout, that should > work well, I think. > The memory is always going to be freed eventually, though. Although the crypto tests currently reuse the input/output buffers and the request structure from one test to the next, they're freed at the end of the tests. Also, it's unsafe for one request structure to be used for multiple requests concurrently anyway. I think crypto_wait_req_timeout() would just be fundamentally unsafe. Couldn't you just use CONFIG_DETECT_HUNG_TASK=y instead? It should report if any thread is blocked for too long. - Eric