| 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
| ||
|
Message-ID: <f93ef357-3336-fdc0-55e9-b09beeb2da72@csgroup.eu>
Date: Fri, 17 Jun 2022 10:57:36 +0000
From: Christophe Leroy <christophe.leroy@...roup.eu>
To: Liang He <windhl@....com>,
"Conor.Dooley@...rochip.com" <Conor.Dooley@...rochip.com>
CC: "oss@...error.net" <oss@...error.net>,
"mpe@...erman.id.au" <mpe@...erman.id.au>,
"benh@...nel.crashing.org" <benh@...nel.crashing.org>,
"paulus@...ba.org" <paulus@...ba.org>,
"linuxppc-dev@...ts.ozlabs.org" <linuxppc-dev@...ts.ozlabs.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v4] powerpc:85xx: Add missing of_node_put() in sgy_cst1000
Le 17/06/2022 à 12:47, Liang He a écrit :
>
>
>
> At 2022-06-17 16:27:03, Conor.Dooley@...rochip.com wrote:
>> On 17/06/2022 09:17, Liang He wrote:
>>>
>>>
>>>
>>> At 2022-06-17 14:53:13, "Christophe Leroy" <christophe.leroy@...roup.eu> wrote:
>>>>
>>>>
>>>> Le 17/06/2022 à 08:45, Liang He a écrit :
>>>>>
>>>>>
>>>>>
>>>>> At 2022-06-17 14:28:56, "Christophe Leroy" <christophe.leroy@...roup.eu> wrote:
>>>>>>
>>>>>>
>>>>>> Le 17/06/2022 à 08:08, Liang He a écrit :
>>>>>>> In gpio_halt_probe(), of_find_matching_node() will return a node
>>>>>>> pointer with refcount incremented. We should use of_node_put() in
>>>>>>> fail path or when it is not used anymore.
>>>>>>>
>>>>>>> Signed-off-by: Liang He <windhl@....com>
>>>>>>> ---
>>>>>>> changelog:
>>>>>>> v4: reuse exist 'err' and use a simple code style, advised by CJ
>>>>>>> v3: use local 'child_node' advised by Michael.
>>>>>>> v2: use goto-label patch style advised by Christophe Leroy.
>>>>>>> v1: add of_node_put() before each exit.
>>>>>>>
>>>>>>> arch/powerpc/platforms/85xx/sgy_cts1000.c | 35 ++++++++++++++---------
>>>>>>> 1 file changed, 22 insertions(+), 13 deletions(-)
>>>>>>>
>>>>>>> diff --git a/arch/powerpc/platforms/85xx/sgy_cts1000.c b/arch/powerpc/platforms/85xx/sgy_cts1000.c
>>>>>>> index 98ae64075193..e4588943fe7e 100644
>>>>>>> --- a/arch/powerpc/platforms/85xx/sgy_cts1000.c
>>>>>>> +++ b/arch/powerpc/platforms/85xx/sgy_cts1000.c
>>>>>>> @@ -71,6 +71,7 @@ static int gpio_halt_probe(struct platform_device *pdev)
>>>>>>> {
>>>>>>> enum of_gpio_flags flags;
>>>>>>> struct device_node *node = pdev->dev.of_node;
>>>>>>> + struct device_node *child_node;
>>>>>>> int gpio, err, irq;
>>>>>>> int trigger;
>>>>>>>
>>>>>>> @@ -78,26 +79,29 @@ static int gpio_halt_probe(struct platform_device *pdev)
>>>>>>> return -ENODEV;
>>>>>>>
>>>>>>> /* If there's no matching child, this isn't really an error */
>>>>>>> - halt_node = of_find_matching_node(node, child_match);
>>>>>>> - if (!halt_node)
>>>>>>> + child_node = of_find_matching_node(node, child_match);
>>>>>>> + if (!child_node)
>>>>>>> return 0;
>>>>>>>
>>>>>>> /* Technically we could just read the first one, but punish
>>>>>>> * DT writers for invalid form. */
>>>>>>> - if (of_gpio_count(halt_node) != 1)
>>>>>>> - return -EINVAL;
>>>>>>> + if (of_gpio_count(child_node) != 1) {
>>>>>>> + err = -EINVAL;
>>>>>>> + goto err_put;
>>>>>>> + }
>>>>>>>
>>>>>>> /* Get the gpio number relative to the dynamic base. */
>>>>>>> - gpio = of_get_gpio_flags(halt_node, 0, &flags);
>>>>>>> - if (!gpio_is_valid(gpio))
>>>>>>> - return -EINVAL;
>>>>>>> + gpio = of_get_gpio_flags(child_node, 0, &flags);
>>>>>>> + if (!gpio_is_valid(gpio)) {
>>>>>>> + err = -EINVAL;
>>>>>>> + gotot err_put;
>>>>>>
>>>>>> Did you test the build ?
>>>>>
>>>>> Sorry for this fault.
>>>>>
>>>>> In fact, I am still finding an efficient way to building different arch source code as I only have x86-64.
>>>>>
>>>>> Now I am try using QEMU.
>>>>>
>>>>> Anyway, sorry for this fault.
>>>>
>>>> You can find cross compilers for most architectures for x86-64 here :
>>>> https://mirrors.edge.kernel.org/pub/tools/crosstool/
>>>>
>>>> Christophe
>>>
>>> Hi, Christophe and Conor.
>>>
>>> Sorry to trouble you again.
>>>
>>> Now I only know how to quickly identify the refcounting bugs, but I cannot efficiently give a build test.
>>>
>>> For example, I use the cross compilers 'powerpc-linux-gnu-gcc' to compile 'arch/powerpc/platforms/85xx/sgy_cts1000.c' with -fsyntax-only flag.
>>> But I meet too many header file missing errors. Even if I add some 'include' pathes, e.g., ./arch/powerpc/include, ./include,
>>> there are still too many other errors.
>>>
>>> So if there is any efficient way to check my patch code to avoid 'gotot' error again.
>>
>> idk anything about powerpc, but what I find is a nice way to get a compiler
>> for an arch I don't use is to search on lore.kernel.org for a 0day robot
>> build error since it gives instructions for building on that arch.
>> For example:
>> https://lore.kernel.org/linuxppc-dev/202206060910.rYNTFqdI-lkp@intel.com/
>>
>>
>> In this case, your bug seems obvious? You typed "gotot" instead of "goto".
>>
>> Hope that helps,
>> Conor.
>>
>>>
>>> Thanks again, Christophe and Conor.
>>>
>>> Liang
>
> Thanks, Conor and Christophe.
>
> I finally figure out an efficient way in which I can use cross-compiler to check my single patched file as follow:
>
> powerpc64le-linux-gnu-gcc -Wp,-MMD,arch/powerpc/kernel/.io.o.d -nostdinc -I./arch/powerpc/include -I./arch/powerpc/include/generated -I./include -I./arch/powerpc/include/uapi -I./arch/powerpc/include/generated/uapi -I./include/uapi -I./include/generated/uapi -include ./include/linux/compiler-version.h -include ./include/linux/kconfig.h -include ./include/linux/compiler_types.h -D__KERNEL__ -I ./arch/powerpc -fmacro-prefix-map=./= -Wall -Wundef -Werror=strict-prototypes -Wno-trigraphs -fno-strict-aliasing -fno-common -fshort-wchar -fno-PIE -Werror=implicit-function-declaration -Werror=implicit-int -Werror=return-type -Wno-format-security -std=gnu11 -mcpu=powerpc -mcpu=powerpc -m32 -msoft-float -pipe -ffixed-r2 -mno-readonly-in-sdata -mno-altivec -mno-vsx -fno-asynchronous-unwind-tables -mno-string -mbig-endian -mstack-protector-guard=tls -mstack-protector-guard-reg=r2 -fno-delete-null-pointer-checks -Wno-frame-address -Wno-format-truncation -Wno-format-overflow -Wno-address-of-packed-member -O2 --param=allow-store-data-races=0 -Wframe-larger-than=1024 -fstack-protector-strong -Wimplicit-fallthrough=5 -Wno-main -Wno-unused-but-set-variable -Wno-unused-const-variable -fomit-frame-pointer -fno-stack-clash-protection -Wdeclaration-after-statement -Wvla -Wno-pointer-sign -Wcast-function-type -Wno-stringop-truncation -Wno-stringop-overflow -Wno-restrict -Wno-maybe-uninitialized -Wno-alloc-size-larger-than -fno-strict-overflow -fno-stack-check -fconserve-stack -Werror=date-time -Werror=incompatible-pointer-types -Werror=designated-init -Wno-packed-not-aligned -mstack-protector-guard-offset=576 -Werror -DKBUILD_MODNAME='"85xx"' -DKBUILD_MODFILE='"arch/powerpc/platforms/85xx/sgy_cts1000.c"' -fsyntax-only ./arch/powerpc/platforms/85xx/sgy_cts1000.c
>
Easiest way is:
- Download cross compiler and extract it somewhere on your PC and ensure
it is in the PATH.
- Then:
run "make CROSS_COMPILE=powerpc64-linux-gnu ARCH=powerpc
corenet32_smp_defconfig"
modify .config to activate CONFIG_SGY_CTS1000
run "make CROSS_COMPILE=powerpc64-linux-gnu ARCH=powerpc
arch/powerpc/platforms/85xx/sgy_cts1000.o"
Powered by blists - more mailing lists