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;
Date:   Fri, 21 Jul 2023 10:19:32 +0100
From:   Simon Horman <simon.horman@corigine.com>
To:     Ard Biesheuvel <ardb@kernel.org>
Cc:     linux-crypto@vger.kernel.org,
        Herbert Xu <herbert@gondor.apana.org.au>,
        Eric Biggers <ebiggers@kernel.org>,
        Kees Cook <keescook@chromium.org>,
        Haren Myneni <haren@us.ibm.com>,
        Nick Terrell <terrelln@fb.com>,
        Minchan Kim <minchan@kernel.org>,
        Sergey Senozhatsky <senozhatsky@chromium.org>,
        Jens Axboe <axboe@kernel.dk>,
        Giovanni Cabiddu <giovanni.cabiddu@intel.com>,
        Richard Weinberger <richard@nod.at>,
        David Ahern <dsahern@kernel.org>,
        Eric Dumazet <edumazet@google.com>,
        Jakub Kicinski <kuba@kernel.org>,
        Paolo Abeni <pabeni@redhat.com>,
        Steffen Klassert <steffen.klassert@secunet.com>,
        linux-kernel@vger.kernel.org, linux-block@vger.kernel.org,
        qat-linux@intel.com, linuxppc-dev@lists.ozlabs.org,
        linux-mtd@lists.infradead.org, netdev@vger.kernel.org
Subject: Re: [RFC PATCH 07/21] ubifs: Migrate to acomp compression API
Message-ID: <ZLpNpLIx5BUW97sr@corigine.com>
References: <20230718125847.3869700-1-ardb@kernel.org>
 <20230718125847.3869700-8-ardb@kernel.org>
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20230718125847.3869700-8-ardb@kernel.org>
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?JkHHdRGilrrfvnSV4ip+Xe0QbwgIDIA+BvsesGFcix9jczbiQMVuYNdL1Ar5?=
 =?us-ascii?Q?1Io+BTvwVSi2u/JIcCR95J1OFs3X7knzJtn1qrEEhERojl7ne2ED7ELJ6daF?=
 =?us-ascii?Q?nNv6CgF/ipeCcqytIfoQ3H36g0fi1/l5G472mGkE6WIAA57IpFkK3S19RokT?=
 =?us-ascii?Q?8NUXVlG8LhZvStZrW8i84zj0XDD2dNYxbrX7eAmj0nJqabtLlEPBe2coqriN?=
 =?us-ascii?Q?Oa5ThNgHjcBvDeml5ebqKgmkfmpBFU1lK6vXGIH0d15FDnKxIi8cQXR05bkY?=
 =?us-ascii?Q?FlNyc06D0ZG+b1YedelMWCG4lA0XWKmUPqyxoefLmjXeAFVo3U0PzPVE7PJs?=
 =?us-ascii?Q?Fko/N4b2Qz+K487i0Becl99RXEYOFkLeZ3C7Q632RZYMFNcT0P64+EHeTrAh?=
 =?us-ascii?Q?auAFq7nl6U29n95csMGjbaUnkHz5ZNUmcAR0/4rIIQYJSfDpm+3JuzLL5Mpn?=
 =?us-ascii?Q?Ol/GXJIl5HbVFd1Htmf/5oW1QbjYn9yznaL2jcf8+bEds+w8FciayRnf1Wza?=
 =?us-ascii?Q?2Szo0qDYNUdQMY2wy/FCdHtD8H6PLq5nOL3DIE/YZH43ozxdi+8PqDNK+Q7f?=
 =?us-ascii?Q?YAB7Vdl2FMB0BK/deg0w0jN0T5izEef/w9p7e8fEIuu3Ly9+KkCXYW9bl1fF?=
 =?us-ascii?Q?hUXhjrpIKm3D4jkCVfX4iZoIIbMrdVn+au8DzKhYi0Qz/eVKeluhB/6WnmPQ?=
 =?us-ascii?Q?W9NCk0gVwCM70babUjZJQhE3WU05Gbij78L1sYiurtwKMjo+F/CVMK6gY9Ac?=
 =?us-ascii?Q?1d1x4BTa+UyDJq7rd584KYWPyjrCMgrUGXgOoCGh8oQYHOH5WS1YYmres7/A?=
 =?us-ascii?Q?GlJEyJWFCXq3IZ72krkloj/lD8DhTwggrTf5DfGLsqVcpwPnUTidmOaGDO9K?=
 =?us-ascii?Q?RgYOiGnQIn/ovCCHp+kz4qEOSOQQxIuDhwlcVOsHGbPEzS8DCQid4gZUREen?=
 =?us-ascii?Q?bQp5CYmWuUdEpo8bi3CDoTJj26CV52LzwccGkuCfrmW0xlelvRf1dZyQvs2N?=
 =?us-ascii?Q?QYal3IT5Z8fILmPpI6XLve6QujR5IExClKYZ1Z1rMkLyvO7QJv7Fpy6t7zno?=
 =?us-ascii?Q?VrY+eudOeckRCtUezdPF3Zz1gPZ274udurhaCx7sx7Ph6ERHEzVDLpC3oNGE?=
 =?us-ascii?Q?5FSV7Vzzn69WbtytCjCeqhu0DduIsZFfw5Ym7W+oVZuarjV7EncI1/iAYjh1?=
 =?us-ascii?Q?W4K6NwdP0CYrItIOL8So4S5eSQxR3ZXmhAXDeEVGAWKMDG0CPmpS/M+X6PKN?=
 =?us-ascii?Q?Ur9zVMlO1z9t+SZxAUmZ7reKutN5jkJ/Mn4d47oo2RFRlDGeg7koAEm5tOCO?=
 =?us-ascii?Q?MipYAs8jxvoif+UPWAfb5jG5wfa9UQTyzoBC+hmAxJqkxsHMjsAPrKaVQH5p?=
 =?us-ascii?Q?xhMQ/JD90873apxeh+Urb4JpgW1PmZy9wslWQYLPpHWJbzB6kgZrbpTFLne5?=
 =?us-ascii?Q?tlOc75BZnz/fwmrschUN1d8HPTs3+zhyIZEOjvQW7aVYbhZHwU9KiEZG8FQf?=
 =?us-ascii?Q?qb85MKER+nuyLQMsoXryqLqGFs+jUp+dGR0dJD11VzA5NCFB+SZxFJeVKOON?=
 =?us-ascii?Q?8mWXSgJz3Ca7F1oNWfyHWoQT0SoP3lh18TQkJSS+E1ZWqCDqW5CM44F2ywxC?=
 =?us-ascii?Q?rg=3D=3D?=
X-OriginatorOrg: corigine.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 95dbfdd0-e082-4970-b490-08db89cb9d0e
X-MS-Exchange-CrossTenant-AuthSource: PH0PR13MB4842.namprd13.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 21 Jul 2023 09:19:41.2973
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: fe128f2c-073b-4c20-818e-7246a585940c
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: NTFAcLu2R1HiYxA8wGM8vAAv3d8qKyECBLxxWCMwbrbcSehwkWF5gIa+HtQmm0/X4yUgy/9D5zuRmAPu9kFR3FsXCRYNYPjWVr0EUJ0AaP4=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: CH3PR13MB6337
X-Spam-Status: No, score=-1.9 required=5.0 tests=BAYES_00,DKIM_SIGNED,
        DKIM_VALID,RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H2,SPF_HELO_PASS,SPF_PASS,
        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

On Tue, Jul 18, 2023 at 02:58:33PM +0200, Ard Biesheuvel wrote:
> UBIFS is one of the remaining users of the obsolete 'comp' compression
> API exposed by the crypto subsystem. Given that it operates strictly on
> contiguous buffers that are either entirely in lowmem or covered by a
> single page, the conversion to the acomp API is quite straight-forward.
> 
> Only synchronous acomp implementations are considered at the moment, and
> whether or not a future conversion to permit asynchronous ones too will
> be worth the effort remains to be seen.
> 
> Signed-off-by: Ard Biesheuvel <ardb@kernel.org>

...

> @@ -197,11 +205,24 @@ int ubifs_decompress(const struct ubifs_info *c, const void *in_buf,
>  static int __init compr_init(struct ubifs_compressor *compr)
>  {
>  	if (compr->capi_name) {
> -		compr->cc = crypto_alloc_comp(compr->capi_name, 0, 0);
> +		long ret;
> +
> +		compr->cc = crypto_alloc_acomp(compr->capi_name, 0,
> +					       CRYPTO_ALG_ASYNC);
>  		if (IS_ERR(compr->cc)) {
> +			ret = PTR_ERR(compr->cc);
> +		} else {
> +			compr->req = acomp_request_alloc(compr->cc);
> +			if (!compr->req) {
> +				crypto_free_acomp(compr->cc);
> +				ret = -ENOMEM;
> +			}
> +		}

Hi Ard,

clang-16 W=1 and Smatch flag that ret may not always be initialised here.

> +
> +		if (ret) {
>  			pr_err("UBIFS error (pid %d): cannot initialize compressor %s, error %ld",
> -			       current->pid, compr->name, PTR_ERR(compr->cc));
> -			return PTR_ERR(compr->cc);
> +			       current->pid, compr->name, ret);
> +			return ret;
>  		}
>  	}
>  

...

> diff --git a/fs/ubifs/ubifs.h b/fs/ubifs/ubifs.h
> index 4c36044140e7eba9..2225de5b8ef50f71 100644
> --- a/fs/ubifs/ubifs.h
> +++ b/fs/ubifs/ubifs.h
> @@ -32,6 +32,7 @@
>  #include <crypto/hash_info.h>
>  #include <crypto/hash.h>
>  #include <crypto/algapi.h>
> +#include <crypto/acompress.h>
>  
>  #include <linux/fscrypt.h>
>  
> @@ -849,7 +850,8 @@ struct ubifs_node_range {
>   */
>  struct ubifs_compressor {
>  	int compr_type;
> -	struct crypto_comp *cc;
> +	struct crypto_acomp *cc;
> +	struct acomp_req *req;

Please consider adding @req to the kernel doc for this structure.

>  	struct mutex *comp_mutex;
>  	struct mutex *decomp_mutex;
>  	const char *name;

...