linux-kernel - Re: [PATCH] nitro_enclaves: Fix implicit type conversion

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] [day] [month] [year] [list]

Message-ID: <cc1abeaf-4ea1-0e89-4449-dd87cd321cc7@amazon.com>
Date:   Wed, 27 Oct 2021 19:27:59 +0300
From:   "Paraschiv, Andra-Irina" <andraprs@...zon.com>
To:     Jiasheng Jiang <jiasheng@...as.ac.cn>
CC:     <linux-kernel@...r.kernel.org>, <lexnv@...zon.com>,
        <alcioa@...zon.com>, Greg KH <gregkh@...uxfoundation.org>,
        Kamal Mostafa <kamal@...onical.com>,
        Paolo Bonzini <pbonzini@...hat.com>,
        Stefano Garzarella <sgarzare@...hat.com>,
        Stefan Hajnoczi <stefanha@...hat.com>,
        "Vitaly Kuznetsov" <vkuznets@...hat.com>,
        "ne-devel-upstream@...zon.com" <ne-devel-upstream@...zon.com>,
        "kvm@...r.kernel.org" <kvm@...r.kernel.org>
Subject: Re: [PATCH] nitro_enclaves: Fix implicit type conversion



On 26/10/2021 12:45, Jiasheng Jiang wrote:
> 
> The variable 'cpu' is defined as unsigned int.
> However in the for_each_cpu, its value is assigned to -1.
> That doesn't make sense and in the cpumask_next() it is implicitly
> type conversed to int.
> It is universally accepted that the implicit type conversion is
> terrible.
> Also, having the good programming custom will set an example for
> others.
> Thus, it might be better to change the definition of 'cpu' from
> unsigned int to int.
> 
> Fixes: ff8a4d3 ("nitro_enclaves: Add logic for setting an enclave vCPU")
> Signed-off-by: Jiasheng Jiang <jiasheng@...as.ac.cn>
> ---
>   drivers/virt/nitro_enclaves/ne_misc_dev.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/virt/nitro_enclaves/ne_misc_dev.c b/drivers/virt/nitro_enclaves/ne_misc_dev.c
> index e21e1e8..38d1fd9 100644
> --- a/drivers/virt/nitro_enclaves/ne_misc_dev.c
> +++ b/drivers/virt/nitro_enclaves/ne_misc_dev.c
> @@ -168,7 +168,7 @@ static bool ne_check_enclaves_created(void)
>   static int ne_setup_cpu_pool(const char *ne_cpu_list)
>   {
>          int core_id = -1;
> -       unsigned int cpu = 0;
> +       int cpu = 0;
>          cpumask_var_t cpu_pool;
>          unsigned int cpu_sibling = 0;
>          unsigned int i = 0;
> --
> 2.7.4
> 

Thank you for the patch. Can you please update the other occurrences in 
the same file e.g. when a local variable is used for the "for_each_cpu" 
loop.

Thanks,
Andra

[1] 
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/virt/nitro_enclaves/ne_misc_dev.c#n171
[2] 
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/virt/nitro_enclaves/ne_misc_dev.c#n173
[3] 
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/virt/nitro_enclaves/ne_misc_dev.c#n377
[4] 
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/virt/nitro_enclaves/ne_misc_dev.c#n519
[5] 
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/virt/nitro_enclaves/ne_misc_dev.c#n565
[6] 
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/virt/nitro_enclaves/ne_misc_dev.c#n1020
[7] 
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/virt/nitro_enclaves/ne_misc_dev.c#n1363



Amazon Development Center (Romania) S.R.L. registered office: 27A Sf. Lazar Street, UBC5, floor 2, Iasi, Iasi County, 700045, Romania. Registered in Romania. Registration number J22/2621/2005.