Re: [RFC PATCH 07/21] ubifs: Migrate to acomp compression API

[Simon Horman]

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@xxxxxxxxxx>

...

> @@ -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;

...