linux-kernel - Re: [RFC/REFACT] Refactoring and significantly reducing code complexity

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Mon, 16 Oct 2023 12:47:29 +0200
From:   Steffen Klassert <steffen.klassert@...unet.com>
To:     Wang Jinchao <wangjinchao@...sion.com>
CC:     Daniel Jordan <daniel.m.jordan@...cle.com>,
        <linux-crypto@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
        <stone.xulei@...sion.com>
Subject: Re: [RFC/REFACT] Refactoring and significantly reducing code
 complexity

Hi,

On Thu, Sep 28, 2023 at 04:53:38PM +0800, Wang Jinchao wrote:
> This is a refactored version with the following main changes:
> 
> - The parallel workqueue no longer uses the WQ_UNBOUND attribute
> - Removal of CPU-related logic, sysfs-related interfaces
> - removal of structures like padata_cpumask, and deletion of parallel_data
> - Using completion to maintain sequencing
> - no longer using lists
> - removing structures like padata_list and padata_serial_queue
> - Removal of padata_do_serial()
> - padata automatically ensures the calling sequence.
> 
> Testing was conducted using ltp's pcrypt_aead01, and the execution time
> comparison between the old and new versions is as follows:
> 
> Old Version:
> real 0m27.451s
> user 0m0.031s
> sys 0m0.260s
> 
> New Version:
> real 0m21.351s
> user 0m0.023s
> sys 0m0.260s
> 
> Signed-off-by: Wang Jinchao <wangjinchao@...sion.com>
> ---
>  crypto/pcrypt.c        |  34 +-
>  include/linux/padata.h |  95 +----
>  kernel/padata.c        | 815 ++---------------------------------------
>  3 files changed, 43 insertions(+), 901 deletions(-)

...

>  
> -static int pcrypt_sysfs_add(struct padata_instance *pinst, const char *name)
> -{
> -	int ret;
> -
> -	pinst->kobj.kset = pcrypt_kset;
> -	ret = kobject_add(&pinst->kobj, NULL, "%s", name);
> -	if (!ret)
> -		kobject_uevent(&pinst->kobj, KOBJ_ADD);
> -
> -	return ret;
> -}

Users might rely on the existence of that sysfs files,
so removing this might be dangerous.

>  static void padata_serial_worker(struct work_struct *serial_work)
>  {
> -	struct padata_serial_queue *squeue;
> -	struct parallel_data *pd;
> -	LIST_HEAD(local_list);
> -	int cnt;
> -
> -	local_bh_disable();

Note that the networking RX path must run with BHs off.
The call to padata_do_parallel might come with BHs off
and the serialization callback should also called with
BHs off.

Please make sure that the networking usecase continues
to work with that refactorization.

Thanks!